-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Set lvSingleDef for non TYP_REF locals #85398
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsSets lvSingleDef for non TYP_REF locals, currently this should only affect reuse of RET_EXPR spills. Split off from #85197.
|
b132f2d
to
6c69793
Compare
Diffs are mostly moves being shuffled around and more places being affected by #85547. |
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.
Can you write up a summary of the diffs this causes? Make sure to include some of the ASP.NET regressions, eg:
Top method regressions (percentages):
27 (18.88 % of base) : 24443.dasm - System.Runtime.CompilerServices.PoolingAsyncValueTaskMethodBuilder`1+StateMachineBox`1[int,Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpRequestStream+<ReadAsyncInternal>d__30]:RentFromCache():System.Runtime.CompilerServices.PoolingAsyncValueTaskMethodBuilder`1+StateMachineBox`1[int,Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpRequestStream+<ReadAsyncInternal>d__30]
Co-authored-by: Andy Ayers <[email protected]>
That specific one is caused by PGO marking a block as zero weight which blocks CSE, as per @SingleAccretion handling that would be tricky. |
That issue is just the reason why it looks less efficient than it could be, but what is the reason that jump threading kicks in now but not before? |
It's because inlinees now assign their returns directly to the variable in the method they're inlined to, instead of to a temp that's later assigned to the variable which I assume happend to block it. |
Jump threading can't handle cases where the block to thread has side effects, so it could be by reusing a temp we're avoiding a temp to temp copy in an unfortunate spot. |
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.
Looks good to me now. Thanks!
Sets lvSingleDef for non TYP_REF locals, currently this should only affect reuse of RET_EXPR spills.
Split off from #85197.