search

python / cpython

The Python programming language
https://www.python.org/
Other
60.91k stars 29.41k forks source link

MSVC pragma to turn compiler optimization off around the _PyEval_EvalFrameDefault is no longer necessary #121338

Open mdboom opened 5 days ago

mdboom commented 5 days ago

Bug report

Bug description:

When the Tier 2 interpreter loop was first merged with the Tier 1 interpreter loop, the MSVC compiler would crash when building for PGO, presumably due to the very large size of the function. #111786 introduced a #pragma optimize("off") to turn off optimization around _PyEval_EvalFrameDefault which seems to have side-stepped the crash.

This crash was reported to the MSVC compiler team, but there hasn't been resolution around that.

In the intervening time, the interpreter loop has changed significantly again, and it seems that this #pragma hack is no longer necessary, either for default builds (which no longer include a Tier 2 interpreter) and --enable-experimental-jit builds (which do). I propose we remove it for the sake of removing what should have been a temporary hack and to give the compiler the fairest shot at doing its best work. Benchmarking shows no significant change for either a default build or a JIT build.

I'm not sure whether this should also be backported to 3.13. It's not really a bugfix, but I would certainly feel better shipping 3.13.0 without a weird compiler workaround (which would be harder to remove after the rc).

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

Linked PRs

zooba commented 5 days ago

I'd support backporting.

It's not clear that it has any effect on PGO'd code (which usually ignores optimisation preferences like /Ot and /Os), but it's still a really bad look to have contributors complaining about MSVC perf when we also explicitly disable optimisations. Let's pretend we never turned it off :)