-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NativeAOT/Arm64: Do not overwrite gcinfo tracking registers for TLS #112469
Conversation
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (1)
- src/coreclr/jit/emitarm64.cpp: Language not supported
@dotnet/jit-contrib |
@jkotas - I think this should be backported to net9.0? |
Yes, could you please backport this to net9.0? |
/backport to release/9.0-staging |
Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/13321667412 |
Fields
reg1
andreg2
ofinstrDesc
were repurposed for saving live gc registers. However, this field was getting overwritten when we populate the register for theblr
instruction that is needed for TLS access pattern. Since the pattern of the instructions including register requirements for them is very specific and has to be like following:runtime/src/coreclr/jit/codegenarmarch.cpp
Lines 3578 to 3591 in 07f8ebd
In LSRA, we already ensure that those requirements are met.
runtime/src/coreclr/jit/lsraarmarch.cpp
Lines 265 to 267 in 07f8ebd
As such, there is no need to track the target handle register separately in a field but can just be used directly in codegen/emitter logic. Also, I noticed that the display was not using the right register, so fixed that as well.
Fixes: #112351