-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
gh-139757: Treat call specially in JIT assembly backend optimizer on x86-64 and AArch64 #142907
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
gh-139757: Treat call specially in JIT assembly backend optimizer on x86-64 and AArch64 #142907
Conversation
Co-authored-by: Savannah Ostrowski <[email protected]>
…n into treat_call_as_jump
savannahostrowski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks for the back and forth on this!
|
I'm merging this as it's fixing an actual bug that causes malformed code in the JIT. We can refine it later if we want, but I have gone through a lot of discussion with Savannah about the correctness of this PR, and I feel confident on it. |
On x86-64 and AArch64, a call is effectively a jump to a label with some caveats. As such, the assembly flow optimizer must treat it as such, or it risks emitting invalid optimizations.
The bug triggers in the following code:
The assembly optimizer currently doesn't see
PyStackRef_CLOSEas being used, and just optimizes the whole thing away.The fix informs the assembly optimizer we might jump to
PyStackRef_CLOSE, thus the label is used and to preserve it.I verified it fixes the crash on FT builds on my system.
The reason why we didn't see this earlier is that our stencils were at -O3 and that squashed them all into one function. After we switched to -Os, bigger stencils get outlined. It just so happens the biggest stencils are on FT.