Skip to content

JIT: Skip moving BBJ_COND jump target if fallthrough target is equally likely#105084

Merged
amanasifkhalid merged 1 commit intodotnet:mainfrom
amanasifkhalid:loop-exit-layout
Jul 18, 2024
Merged

JIT: Skip moving BBJ_COND jump target if fallthrough target is equally likely#105084
amanasifkhalid merged 1 commit intodotnet:mainfrom
amanasifkhalid:loop-exit-layout

Conversation

@amanasifkhalid
Copy link
Contributor

@amanasifkhalid amanasifkhalid commented Jul 18, 2024

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.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 18, 2024
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jul 18, 2024

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

@amanasifkhalid amanasifkhalid mentioned this pull request Jul 18, 2024
Closed
@build-analysis build-analysis bot mentioned this pull request Jul 18, 2024
Closed
@amanasifkhalid
Copy link
Contributor Author

amanasifkhalid commented Jul 18, 2024 •
edited
Loading

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.

AndyAyersMS
AndyAyersMS approved these changes Jul 18, 2024
View reviewed changes
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
Contributor Author

amanasifkhalid commented Jul 18, 2024

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
Contributor Author

amanasifkhalid commented Jul 18, 2024

/ba-g NativeAOT failures look like #104500

@amanasifkhalid amanasifkhalid merged commit c707913 into dotnet:main Jul 18, 2024
@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.

Reviewers

@jakobbotsch jakobbotsch jakobbotsch approved these changes

@AndyAyersMS AndyAyersMS AndyAyersMS approved these changes

Assignees

@amanasifkhalid amanasifkhalid

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

JIT: Suboptimal block layout with loops exiting to returns

3 participants

@amanasifkhalid @jakobbotsch @AndyAyersMS