Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler crash on MSVC when building with JIT and PGO #125217

Closed
mdboom opened this issue Oct 9, 2024 · 4 comments
Closed

Compiler crash on MSVC when building with JIT and PGO #125217

mdboom opened this issue Oct 9, 2024 · 4 comments
Assignees
Labels
OS-windows type-bug An unexpected behavior, bug, or error

Comments

@mdboom
Copy link
Contributor

mdboom commented Oct 9, 2024

Bug report

Bug description:

Building with the JIT and PGO simultaneously causes MSVC 1941 to crash:

Merging D:\a\cpython\cpython\PCbuild\amd64\python314!1.pgc
  D:\a\cpython\cpython\PCbuild\amd64\python314!1.pgc: Used 31.3% (19698312 / 62844928) of total space reserved.  0.0% of the counts were dropped due to overflow.
    Reading PGD file 1: D:\a\cpython\cpython\PCbuild\amd64\python314.pgd
     Creating library D:\a\cpython\cpython\PCbuild\amd64\python314.lib and object D:\a\cpython\cpython\PCbuild\amd64\python314.exp
  Generating code
  
  0 of 0 ( 0.0%) original invalid call sites were matched.
  0 new call sites were added.
  291 of 16259 (  1.79%) profiled functions will be compiled for speed, and the rest of the functions will be compiled for size
D:\a\cpython\cpython\Python\ceval.c(766): fatal error C1001: Internal compiler error. [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]
  (compiler file 'D:\a\_work\1\s\src\vctools\Compiler\Utc\src\p2\main.c', line 247)
   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()+0xd94ce
    link!InvokeCompilerPass()+0xd94ce
    link!InvokeCompilerPass()+0xd922d
    link!InvokeCompilerPass()+0xd610c
    link!InvokeCompilerPass()+0xd6011
    link!CloseTypeServerPDB()+0x12904
    link!InvokeCompilerPassW()+0x65cd9
    link!InvokeCompilerPassW()+0x65c9c
    link!time32()+0x83
    link!BaseThreadInitThunk()+0x19
    link!RtlGetFullPathName_UEx()+0xad
    link!RtlGetFullPathName_UEx()+0x7b

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

Linked PRs

@mdboom mdboom added type-bug An unexpected behavior, bug, or error OS-windows labels Oct 9, 2024
@mdboom mdboom self-assigned this Oct 9, 2024
mdboom added a commit to mdboom/cpython that referenced this issue Oct 9, 2024
@mdboom
Copy link
Contributor Author

mdboom commented Oct 10, 2024

Perhaps this is the same root cause as the crash with the free-threading build as well (just extra complexity in _PyEval_EvalFrameDefault #123153.

@mdboom
Copy link
Contributor Author

mdboom commented Oct 11, 2024

This problem is proving tricky to solve. In the past when we've seen this crash, it has been enough to disable speed optimizations on the single _PyEval_EvalFrameDefault function using #pragma optimize("t", off), but that's not working in this case. I have also tried turning off all optimizations on that function to no avail.

Just to confirm why PGO is so important, I measured its effect on the last known-working commit (04c837d) and PGO improves speed by about 14% on the pyperformance suite. So "just not using PGO on Windows" isn't a viable option.

git bisect tells me that the first commit to cause the compiler to crash with the JIT/PGO combination is da071fa #124392, which at least seems plausible. Maybe there is something we can do with those changes to workaround the compiler crash (Cc: @markshannon) as a stopgap solution.

In the meantime, now that I have a set of changes to point to, I can file a bug against MSVC.

@mdboom
Copy link
Contributor Author

mdboom commented Oct 11, 2024

@neonene
Copy link
Contributor

neonene commented Oct 14, 2024

Leaving a few possible alternatives just in case:

_PyEval_EvalFrameDefault(...)
{
 ...
-    uint8_t opcode;    /* Current opcode */
+    uint16_t opcode;   /* Current opcode */
 ...
 #if USE_COMPUTED_GOTOS
         _unknown_opcode:
 #else
-        EXTRA_CASES  // From pycore_opcode_metadata.h, ...
+        default:
 #endif

Adding an extra opcode case more than 299, which loses the advantage of EXTRA_CASES:

_PyEval_EvalFrameDefault(...)
{
 ...

 #if USE_COMPUTED_GOTOS
         _unknown_opcode:
 #else
         EXTRA_CASES  // From pycore_opcode_metadata.h, ...
+        case 500:
 #endif

I guess MSVC might want to merge the jump tables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants