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: Skip moving BBJ_COND jump target if fallthrough target is equally likely #105084

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

amanasifkhalid
Copy link
Member

Fixes #105083. In Compiler::fgMoveHotJumps, if a BBJ_COND block falls into one of its targets, and its targets are equally likely to be taken, don't bother moving anything.

@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 Jul 18, 2024
Copy link
Contributor

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

@amanasifkhalid
Copy link
Member Author

amanasifkhalid commented Jul 18, 2024

cc @AndyAyersMS, diffs are quite big. We have about as many size improvements as regressions on x64. This seems like a layout decision we ought to do on principle, though I'm curious to hear your thoughts.

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might want to go even further ... say if one of the successors is already next then we require stronger evidence we're making the wrong choice. Or we look at the successor's preferred predecessors.

But this starts to verge onto K-opt territory...

I'm ok merging this since it's a net code size improvement and keeps loops compact, despite the churn.

@amanasifkhalid
Copy link
Member Author

But this starts to verge onto K-opt territory...

Yeah, I have a couple of ideas for improving fgMoveHotJumps that I'd like to try, but I think k-opt is doable enough that we might as well spend time on it.

@amanasifkhalid
Copy link
Member Author

/ba-g NativeAOT failures look like #104500

@amanasifkhalid amanasifkhalid merged commit c707913 into dotnet:main Jul 18, 2024
105 of 108 checks passed
@amanasifkhalid amanasifkhalid deleted the loop-exit-layout branch July 18, 2024 21:51
@github-actions github-actions bot locked and limited conversation to collaborators Aug 18, 2024
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.

JIT: Suboptimal block layout with loops exiting to returns
3 participants