gh-133371: Don't optimize LOAD_FAST instructions whose local is killed by DELETE_FAST#133383
gh-133371: Don't optimize LOAD_FAST instructions whose local is killed by DELETE_FAST#133383mpage merged 4 commits intopython:mainfrom
LOAD_FAST instructions whose local is killed by DELETE_FAST#133383Conversation
…_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.
LOAD_FAST instructions whose local is killed by DEL_FASTLOAD_FAST instructions whose local is killed by DELETE_FAST
| case DELETE_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
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: |
|
@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. |
|
@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 |
|
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. |
|
I think the hypothesis failure is unrelated to this PR. It looks like |
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?)
Yes. This optimization happens statically during bytecode compilation. Maybe you are thinking of the specializing interpreter, which optimizes the compiled bytecode at runtime? |
|
Go ahead and merge then! |
|
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.
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.