-
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
Fix missing relocations in NativeAOT x86 output #98740
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThis bug was introduced in #97413. Output without the fix:
Output with the fix:
|
Diff results for #98740Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Throughput diffs for windows/arm64 ran on windows/x64MinOpts (-0.00% to +0.01%)
Details here |
cc @dotnet/jit-contrib |
@kunalspathak could you have a look? Looks like this was broken by your recent TLS change. |
/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.
LGTM, I don't recall why I to do this change during linux/x64 support, probably because there was a case where TLS related constant that was wrongly getting IMAGE_REL_BASED_MOFFSET
reloc and should have not get it. I am running outer loop test to make sure.
@filipnavara - where was this issue hitting you? I am surprised why our outerloop didn't catch from past few weeks. |
win-x86 target which I am bringing up. It’s not part of the tests yet. |
This bug was introduced in #97413.
Output without the fix:
Output with the fix: