-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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 gtGetClassHandle for static fields (R2R only) #74075
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsCloses #74000 It turns out Codegen diff for sample in ttps://github.com//issues/74000: https://www.diffchecker.com/AjNP6YTx
|
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.
Is the comment you deleted no longer true?
If so, there may be other assumptions like this in the jit code.
Looks like static IFoo theFoo = new Program(); it looks like
so it's not really opaque and returns nullptr for gtGetClassHandle. Maybe it's NativeAOT specific? For this PR I am waiting for SPMI to fix itself to re-run and see if there any diffs |
FWIW, the shape of access to static fields hasn't changed for a very long time (one exception being the attachment of field sequences to outer addresses for boxed statics, which are not relevant here). Also, I believe the "is static base" check can be simply deleted if there's not a correctness (versioning resilience?) reason we cannot look at declared types of statics under R2R. The field sequence check should be sufficient. |
Odd, what you have now is more or less the way I wrote it initially, long ago, but then revised it. But I can't find any notes from back then so I don't recall why I changed things. Nothing else calls |
…ticGCBaseHelperCall
/azp list |
This comment was marked as resolved.
This comment was marked as resolved.
/azp run runtime-coreclr r2r, runtime-coreclr crossgen2, runtime-coreclr crossgen2 outerloop, runtime-extra-platforms |
Azure Pipelines successfully started running 4 pipeline(s). |
Oh, forgot about this one, will address the feedback tomorrow |
…ticGCBaseHelperCall
@AndyAyersMS PTAL, a small clean up, diffs (in crossgen2 collections): https://dev.azure.com/dnceng-public/public/_build/results?buildId=38038&view=ms.vss-build-web.run-extensions-tab |
Closes #74000
It turns out
gtGetClassHandle
used to always return nullptr for such static field loads in R2R (here)Codegen diff for sample in #74000: https://www.diffchecker.com/AjNP6YTx