gh-133371: Don't optimize LOAD_FAST instructions whose local is killed by DELETE_FAST#133383
Uh oh!
There was an error while loading. Please reload this page.
Conversation
…_FAST` In certain cases it's possible for locals loaded by `LOAD_FAST` instructions to be on the stack when the local is killed by `DEL_FAST`. These `LOAD_FAST` instructions should not be optimized into `LOAD_FAST_BORROW` as the strong reference in the frame is killed while there is still a reference on the stack.
mpage changed the title LOAD_FAST instructions whose local is killed by DEL_FASTLOAD_FAST instructions whose local is killed by DELETE_FAST
| caseDELETE_FAST:{ | ||
| kill_local(instr_flags, &refs, oparg); | ||
| break; | ||
| } | ||
There was a problem hiding this comment.
FWIW: This change by itself is not enough to make the crash in test_asyncio go away.
gvanrossum left a comment
gvanrossum left a comment There was a problem hiding this comment.
Doesn't seem to fix the test_asyncio crash.
When you're done making the requested changes, leave the comment: |
mpage commented May 5, 2025
@gvanrossum - This fixes both of the repros in the linked issue. Even if this doesn't fix the original issue, it's worth merging by itself. |
mpage commented May 5, 2025
@gvanrossum - I forgot to bump the magic number. I suspect that you have some cached bytecode. I've bumped the magic number, double checked that this fixes both of the repros on the linked issue, and applied this on top of the affected PR and verified that it fixes the crash in |
mpage commented May 5, 2025
I have made the requested changes; please review again |
Thanks for making the requested changes! @Yhg1s, @gvanrossum, @Fidget-Spinner: please review the changes made to this pull request. |
mpage commented May 5, 2025
I think the hypothesis failure is unrelated to this PR. It looks like |
Uh oh!
There was an error while loading. Please reload this page.
gvanrossum left a comment
gvanrossum left a comment There was a problem hiding this comment.
I can no longer repro the crash (probably because I am now using a different machine) but your hypothesis that I kept seeing it because of the magic number makes sense. (A little confused still -- does optimized bytecode now end up in .pyc files?)
mpage commented May 5, 2025
Yes. This optimization happens statically during bytecode compilation. Maybe you are thinking of the specializing interpreter, which optimizes the compiled bytecode at runtime? |
gvanrossum commented May 5, 2025
Go ahead and merge then! |
mpage merged commit 78adb63 into python:mainUh oh!
There was an error while loading. Please reload this page.
gvanrossum commented May 5, 2025
Thanks! |
…is killed by `DELETE_FAST` (python#133383) In certain cases it's possible for locals loaded by `LOAD_FAST` instructions to be on the stack when the local is killed by `DEL_FAST`. These `LOAD_FAST` instructions should not be optimized into `LOAD_FAST_BORROW` as the strong reference in the frame is killed while there is still a reference on the stack.
6 participants
In certain cases it's possible for locals loaded by
LOAD_FASTinstructions to be on the stack when the local is killed byDELETE_FAST. TheseLOAD_FASTinstructions should not be optimized intoLOAD_FAST_BORROWas the strong reference in the frame is killed while there is still a reference on the stack.