Skip to content
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

JIT: Relax address exposure check for tailcalls #111397

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jan 14, 2025

The lvHasLdAddrOp based check is conservative: address exposure (as computed in local morph) is much more precise. We can switch to that check only.

This used to be a code size regression, but that's generally expected when expanding more epilogs. Now that we have perf score I want to check how it shows up in terms of perf score.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 14, 2025
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@jakobbotsch
Copy link
Member Author

jakobbotsch commented Jan 14, 2025

cc @dotnet/jit-contrib PTAL @AndyAyersMS

I tried landing this in #65102 before, but back then we only had code size diffs and I didn't spend the time to try to argue for this being worth it. I think now with perfscore we can make the argument, in addition to other motivating factors since then, like the reliance on tailcalls to slow helpers as we convert from C++ to C#. @MichalStrehovsky has been hitting that multiple times before, see e.g. #93100 (comment), #93100 (comment), #85901.

Diffs. Large number of diffs, which are often size regressions because of fewer shared epilogs, but perf score wise usually still show up as improvements.

@jakobbotsch
Copy link
Member Author

I want to run some more pipelines, but going to wait until #111405 goes through to kick them off.

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, runtime-coreclr gcstress0x3-gcstress0xc

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch jakobbotsch merged commit d2cdcdd into dotnet:main Jan 15, 2025
146 of 150 checks passed
@jakobbotsch jakobbotsch deleted the tailcall-var-address-exposure branch January 15, 2025 09:15
@AndyAyersMS AndyAyersMS mentioned this pull request Feb 9, 2025
22 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Feb 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants