-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Improve TryTransformStoreObjAsStoreInd
optimization.
#55727
Conversation
/azp run runtime-coreclr jitstress, runtime-coreclr outerloop, runtime-coreclr jitstressregs |
Azure Pipelines successfully started running 3 pipeline(s). |
GitHub does not show the checks for me but they were started: |
PTAL @dotnet/jit-contrib, I would like to sneak 1 more change into Prev7 if somebody has time to review this simple change. It actually fixes a wrong usage of |
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.
Great catch!
outerloop failures are related to #55657. |
It was failing on DevDiv_280120 arm32 linux, did not repro with an altjit.
I looked at the failures and most were unrelated but one was hitting the new assert that I added here. I have disabled the assert and will open an issue to check why it was failing on that test. The assert was about catching missed optimization opportunities, not about correctness so it is safe not to add it. |
With #55558 in the main we can now start using structs in regs potential, this is a change to convert more
STORE_BLK(dst, LCL_VAR)
asSTORE_IND(dst, LCL_VAR)
.The second representation is better because
STORE_IND
allowsLCL_VAR
to be enregistered.There was also a bug that the optimization was enabled for MinOpts and disabled for MaxOpts, fixed as well. It was not providing much value without enregistration so that was staying hidden for quite a while (since 5.0).
Some SPMI highlights:
asp and benchmarks are showing improvements as well.
Full diffs
The regressions are small and expected, they are:
The ratio of improvements/regressions in benchmarks was concerning so I manually checked that all regressions are in one of the first 2 categories.
contributes to #43867