PGO build broken on Windows

[mdboom]

Bug report

Bug description:

Building with --pgo on Windows seems to be broken on main (853b4b5).

My hunch, since this is failing in ceval.c, is that the recent merge of the Tier 1 and Tier 2 interpreters may have made the interpreter loop function too large for MSVC to handle.

Build command:

PCbuild\build.bat --pgo -c Release

Error:

Merging C:\actions-runner\_work\benchmarking\benchmarking\cpython\PCbuild\amd64\python313!1.pgc
  C:\actions-runner\_work\benchmarking\benchmarking\cpython\PCbuild\amd64\python313!1.pgc: Used 25.9% (15643384 / 60370944) of total space reserved.  0.0% of the counts were dropped due to overflow.
    Reading PGD file 1: C:\actions-runner\_work\benchmarking\benchmarking\cpython\PCbuild\amd64\python313.pgd
     Creating library C:\actions-runner\_work\benchmarking\benchmarking\cpython\PCbuild\amd64\python313.lib and object C:\actions-runner\_work\benchmarking\benchmarking\cpython\PCbuild\amd64\python313.exp
  Generating code
  
  0 of 0 ( 0.0%) original invalid call sites were matched.
  0 new call sites were added.
  257 of 14721 (  1.75%) profiled functions will be compiled for speed, and the rest of the functions will be compiled for size
C:\actions-runner\_work\benchmarking\benchmarking\cpython\Python\ceval.c(669): fatal error C1001: Internal compiler error. [C:\actions-runner\_work\benchmarking\benchmarking\cpython\PCbuild\pythoncore.vcxproj]
  (compiler file 'D:\a\_work\1\s\src\vctools\Compiler\Utc\src\p2\main.c', line 224)
   To work around this problem, try simplifying or changing the program near the locations listed above.
  If possible please provide a repro here: https://developercommunity.visualstudio.com/ 
  Please choose the Technical Support command on the Visual C++ 
   Help menu, or open the Technical Support help file for more information
    link!InvokeCompilerPass()+0x10e636
    link!InvokeCompilerPass()+0x10e636
    link!InvokeCompilerPass()+0x10e373
    link!InvokeCompilerPass()+0x10b310
    link!InvokeCompilerPass()+0x10b215
    link!InvokeCompilerPass()+0x102cea
  

Build FAILED.

Complete build log

We don't currently have a buildbot that builds with --pgo on Windows -- probably something to fix as well.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

Linked issue

Microsoft Developer Community MSVC bug report

Linked PRs

• gh-111786: Turn off optimize for speed for _PyEval_EvalFrameDefault on MSVC #111794
• gh-111786: Use separate opcode vars for Tier 1 and Tier 2 #112289

[vstinner]

fatal error C1001: Internal compiler error.

That sounds like a MSVC bug.

To work around this problem, try simplifying or changing the program near the locations listed above.
If possible please provide a repro here: https://developercommunity.visualstudio.com/

Can you try to report the bug to MSVC support?

[zooba]

Yeah, we'll need a link repro for this, but it should be easy enough to get them to diagnose it. Reporting through dev community is still the best option, even if you work there 😉

Hopefully there's a resolution that doesn't require a specific compiler version. Our infrastructure needs a revamp to handle that situation...

[mdboom]

My plan is to explore configuration/pragma options to see if anything can be worked around (and benchmark the effect of that so we know what the performance impact of not doing certain optimizations on certain functions is). Barring that we may need to revert or redesign #111428.

Reporting the bug to MSVC also makes sense, and I plan to do that. But I don't think we can count on it to solve our issue -- there will always be a need to compile on a range of versions for a variety of reasons, including what @zooba already said.

[mdboom]

Reading the description of error C1001, it's clear this is an error in the optimizer. This lead me to a description of MSVC's optimization pragmas as a way to control the optimization of just this one very large function (rather than spending time on anything more global).

The two main optimization flags are size ("s") and time ("t"). If you disable them both, you create a new problem, which is that the stack frames get larger, and it exceeds the C stack space during some unit tests (test_pickle being one of them). If you disable only size, the internal compiler error remains. However, if you just disable time ("t"), it successfully compiles.

It would seems that disabling "optimize for speed" would make things slower, however, running on our benchmarking machine, it is about 3% faster (by the HPT method, which is more stable between runs) or 6% faster by the geometric mean method.

So it seems like disabling the "t" optimization for just _PyEval_EvalFrameDefault has no downside.

I still plan to report this issue to MSVC. When there is a fix in MSVC, we can add an #ifdef to remove this workaround for that newer version of MSVC, assuming it helps performance.

[zooba]

It would seems that disabling "optimize for speed" would make things slower, however, running on our benchmarking machine, it is about 3% faster

Large functions are often faster when optimised for size, as the instruction cache pressure tends to outweigh anything else.

That said, I'm fairly sure that PGO ignores all of those options and uses its own data, and generally it improves upon the /O options by 10-20%, if not more. So it may not be a suitable choice.

I've also found in the past that many of our optimiser crashes have been due to mishandled types between object file (e.g. a function argument is defined as int in one but unsigned int in the other). Might be worth looking for anywhere new that kind of thing may have happened - the compiler should never crash, but it's potentially crashing because of an error in our code that we'd want to fix anyway.