-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Removing GT_STORE_DYN_BLK, part 1 #98733
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
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue Detailsnull
|
|
I think we can actually merge it as is? Diffs are almost none. I think it makes sense to separate function change (this) and the upcoming -1000 LOC clean up to actually remove @dotnet/jit-contrib PTAL. CI failures are unrelated (happen on all PRs now) |
src/coreclr/jit/importer.cpp
Outdated
| if (isVolatile) | ||
| { | ||
| // Wrap with memory barriers: full-barrier + call + load-barrier | ||
| impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("spilling side-effects")); |
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.
I think this needs to be called before we pop op1, op2 and op3 -- probably need to move the pops into each of the if/else.
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.
Ah, right
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.
Addressed
1703607 to
44c9c76
Compare
No description provided.