GH-109214: _SET_IP before _PUSH_FRAME (but not _POP_FRAME)
#111001
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brandtbucher
added
interpreter-core
gvanrossum
approved these changes
Oct 17, 2023
| case OPARG_SET_IP: // op==_SET_IP; oparg=next instr | ||
| case OPARG_SET_IP: // uop=_SET_IP; oparg=next_instr-1 | ||
| // The number of caches is smuggled in via offset: | ||
| assert(offset == _PyOpcode_Caches[_PyOpcode_Deopt[opcode]]); |
There was a problem hiding this comment.
If this is always it, why do we need to smuggle it in via offset? Also it's beginning to look like the offset should just be made one larger in the code generator, so we can drop the ugly + 1 on line 654 above.
|
I agree with your comments. I'll land this now to unblock tier two, and work on a follow-up PR to clean up this IP stuff further. |
Maybe wait until Irit's PR lands. It touches on all these things. |
|
Okay, I will. |
aisk
pushed a commit
to aisk/cpython
that referenced
this pull request
Feb 11, 2024
Glyphack
pushed a commit
to Glyphack/cpython
that referenced
this pull request
Sep 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#110755 inadvertently allowed the tier two optimizer to remove instruction pointer updates before frame pushes, which breaks the following return. This updates
remove_unneeded_uopsto recognize_PUSH_FRAMEas a uop that requires an up-to-date instruction pointer on the frame.While investigating this, I realized that we actually don't need to set the instruction pointer before returning from a frame (since the frame is unlinked and torn down immediately after). So I've also removed
_SAVE_CURRENT_IPfrom both of the return opcodes, which is a nice little win for tier one.Also, remove some dead code and comment/assert a non-obvious bit of code during trace projection.