[mono][interp] Improve precision of native offset estimation#103956
Merged
BrzVlad merged 1 commit intodotnet:mainfrom Jun 26, 2024
Merged
[mono][interp] Improve precision of native offset estimation#103956BrzVlad merged 1 commit intodotnet:mainfrom
BrzVlad merged 1 commit intodotnet:mainfrom
Conversation
The interpreter had always had problems with emitting branches, because the interpreter IR is actually executable. So when we generate branches we don't know if the branch will be long or short. A fix for this was to always generate long branches, and before the final codegen we compute conservative native offset estimates for instructions / bblocks (which are always larger that what would end up in the final code). Based on these, we determine if some branches can only be short, in which case, during codegen we emit a short branch. The problem arises with superinstructions doing branches which only have support for short branches. Before this pass, we need to compute the native offset estimates, in order to know if it is safe to optimize with a branch superins. The problem is that, it is hard to maintain the constraint for the native offset estimates if we are computing them so early in the optimization process. This commit fixes the estimate to account for additional moves that can be inserted by the var offset allocator. We still don't account for instructions inserted by some bblock optimizations (it is not clear if these are hit in practice). This commit is a conservative fix, addressing an existing precision issue with the computation, but a better long term fix would probably be to either deconstruct the super instruction if we detect it needs to do a long branch during final emit or to just move super instruction generation to a very late stage, after all optimizations and codgen happened, as a peephole optimization, where we can be certain whether a certain branch will be short or long.
ghost
added
the
Contributor
|
Tagging subscribers to this area: @BrzVlad, @kotlarmilos |
This was referenced Jun 25, 2024
Open
kotlarmilos
approved these changes
Jun 26, 2024
| // might end up inserting additional moves for the arguments | ||
| int *call_args = ins->info.call_info->call_args; | ||
| while (*call_args != -1) { | ||
| noe += 4; // mono_interp_oplen [MINT_MOV_VT]; |
Member
There was a problem hiding this comment.
Is it always a single MINT_MOV_VT per argument?
Member
Author
There was a problem hiding this comment.
It is either a scalar mov (size 3) or a vt move (which also has size embedded so it is size 4). We are being conservative here since it is not worth checking the signature.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The interpreter had always had problems with emitting branches, because the interpreter IR is actually executable. So when we generate branches we don't know if the branch will be long or short. A fix for this was to always generate long branches, and before the final codegen we compute conservative native offset estimates for instructions / bblocks (which are always larger that what would end up in the final code). Based on these, we determine if some branches can only be short, in which case, during codegen we emit a short branch.
The problem arises with superinstructions doing branches which only have support for short branches. Before this pass, we need to compute the native offset estimates, in order to know if it is safe to optimize with a branch superins. The problem is that, it is hard to maintain the constraint for the native offset estimates if we are computing them so early in the optimization process. This commit fixes the estimate to account for additional moves that can be inserted by the var offset allocator. We still don't account for instructions inserted by some bblock optimizations (it is not clear if these are hit in practice).
This commit is a conservative fix, addressing an existing precision issue with the computation, but a better long term fix would probably be to either deconstruct the super instruction if we detect it needs to do a long branch during final emit or to just move super instruction generation to a very late stage, after all optimizations and codgen happened, as a peephole optimization, where we can be certain whether a certain branch will be short or long.
Fixes #100753