GH-144651: Optimize the new uops added when recording values during tracing.#144948
Open
markshannon wants to merge 2 commits intopython:mainfrom
Open
GH-144651: Optimize the new uops added when recording values during tracing.#144948markshannon wants to merge 2 commits intopython:mainfrom
markshannon wants to merge 2 commits intopython:mainfrom
Conversation
* Handle dependencies in the optimizer, not the tracer * Strengthen some checks to avoid relying on optimizer for correctness
markshannon
requested review from
Fidget-Spinner,
savannahostrowski and
tomasr8
as code owners
Member
Fidget-Spinner
left a comment
Fidget-Spinner
left a comment
There was a problem hiding this comment.
This is a really good simplification. Thanks for doing it. Just two comments.
| int delta = (int)(new_stack_pointer - current_sp); | ||
| assert(delta >= 0); | ||
| if (delta) { | ||
| /* Shift existing stack elements up */ |
Member
There was a problem hiding this comment.
Hmm this is suspicious. How safe is this?
Member
Author
There was a problem hiding this comment.
I'll add a guard above that new_stack_pointer is within the limits of the array.
Member
|
CI is failing because unoptimized traces can now just execute through invalidated code objects. That's intentional right? As the guard now handles those. I think you can just add |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR adds optimizations for the new uops added in #144179 and also removes dependencies tracking from the front-end.
By splitting the optimizer stack into two parts, one for locals and one for the evaluation stack, we are able to use the stack before knowing the size of the frame. This is necessary as guards can happen after we set local or stack values, e.g in
_FOR_ITER_GEN_FRAMEAlso includes some small fixes to ensure that the code emitted by the front-end is stand-alone, meaning that it does not depend on watchers or the optimizer for correctness.
Also add a couple of missing invalidations for instrumentation.