JIT: Fix profiler enter callback init reg trash logic #100637
Merged
Conversation
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
During prolog generation we sometimes generate code to call the profiler enter callback. This may trash the "initReg" that we expect to keep zeroed during the prolog. The logic to check if the initReg was being trashed was wrong in a couple of cases: - Most backends did not take into account that the logic also trashes the registers used for arguments to the enter callback - SysV x64 thought that the enter callback trashed the parameter registers, but it does not This generally did not cause issues because `genFnPrologCalleeRegArgs` is unnecessarily conservative around whether or not it trashes `initReg`, and it comes after the profiler callback in the prolog. However, with the rewrite of the homing function that is not going to be the case anymore.
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
cc @dotnet/jit-contrib PTAL @BruceForstall |
|
No diffs -- so no test coverage? |
BruceForstall
approved these changes
Apr 4, 2024
We have test coverage in stress due to |
matouskozak
pushed a commit
to matouskozak/runtime
that referenced
this pull request
Apr 30, 2024
During prolog generation we sometimes generate code to call the profiler enter callback. This may trash the "initReg" that we expect to keep zeroed during the prolog. The logic to check if the initReg was being trashed was wrong in a couple of cases: - Most backends did not take into account that the logic also trashes the registers used for arguments to the enter callback - SysV x64 thought that the enter callback trashed the parameter registers, but it does not This generally did not cause issues because `genFnPrologCalleeRegArgs` is unnecessarily conservative around whether or not it trashes `initReg`, and it comes after the profiler callback in the prolog. However, with the rewrite of the homing function that is not going to be the case anymore.
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.
During prolog generation we sometimes generate code to call the profiler enter callback. This may trash the "initReg" that we expect to keep zeroed during the prolog. The logic to check if the initReg was being trashed was wrong in a couple of cases:
This generally did not cause issues because
genFnPrologCalleeRegArgsis unnecessarily conservative around whether or not it trashesinitReg, and it comes after the profiler callback in the prolog. However, with the rewrite of the homing function that is not going to be the case anymore.Fixes an issue I noticed by inspection over in #100572