Repository navigation
Support frame variables in the SPC without deopt - #728
Draft
matthew-mojira wants to merge 1 commit into
Draft
matthew-mojira wants to merge 1 commit into
matthew-mojira wants to merge 1 commit into
Conversation
matthew-mojira
marked this pull request as ready for review
October 15, 2026 23:30
matthew-mojira
marked this pull request as draft
October 15, 2026 23:36
Contributor
Author
|
there is still a bug for probes that are deopting to the wrong place that still warrant another PR |
titzer
reviewed
Oct 21, 2026
| // XXX does not support dynamically changing frame_var_tags in a function | ||
| def pushFrameVars(func: FuncDecl) { | ||
| if (FeatureDisable.frameVariables || func.frame_var_tags == null) return; | ||
| for (t in func.frame_var_tags) { |
Owner
There was a problem hiding this comment.
The original code just added them to the state and records them as constant 0, without writing their frame tags. The new code eagerly initializes them, and I think that is not necessary.
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
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.
So that the path profiling monitor can be compiled and use frame variables (without having to deopt) I have added support for the SPC to hold frame variables. The layout is the same as the interpreter, so that if there is actually a deopt or tierup/OSR the frame variables don't require any special handling.
One thing I noticed is that there isn't really a formal way to add/remove/change frame variables. The fast interpreter reads the
frame_var_tagsfield from the function before executing the function which is robust against changes toframe_var_tags. But in the SPC it is compiled once against theframe_var_tagsseen at compile time. So if anything ever changes theframe_var_tagsat runtime (or after it is compiled) then the SPC code will not notice.Currently, only legacy exception handling uses frame variables and that is done at validation time before the SPC runs. For my path profiling monitor I had to add a function to add frame variables to hold the path number register, but these variables are added in the
onParsemethod of the monitor which should be before the SPC compiles the function.