Skip to content

JIT: Align newly recognized loops #96756

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jan 12, 2024
Merged

JIT: Align newly recognized loops #96756

merged 5 commits into from
Jan 12, 2024

Conversation

jakobbotsch
Copy link
Member

Also remove quirk when checking for inner-most loops.
@jakobbotsch
JIT: Align newly recognized loops
e606a7f 
Also remove quirk when checking for inner-most loops.
@ghost ghost assigned jakobbotsch Jan 10, 2024
@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 10, 2024
@ghost
Copy link

ghost commented Jan 10, 2024

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

Issue Details

Also remove quirk when checking for inner-most loops.

Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr
Milestone: -
jakobbotsch
jakobbotsch commented Jan 10, 2024
View reviewed changes
src/coreclr/jit/compiler.cpp
Comment on lines -5268 to -5270
// TODO-Quirk: Remove. When removing we will likely need to add some
// form of "lexicality" heuristic here: only align loops whose blocks
// are fairly tightly packed together physically.
Copy link
Member Author

Choose a reason for hiding this comment

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

I decided not to look at this here; in particular because we have a lot of work in flight that is severely changing the flow graph, so it's hard to get a good feeling for how this heuristic should eventually look.
@build-analysis build-analysis bot mentioned this pull request Jan 10, 2024
Closed
@jakobbotsch jakobbotsch mentioned this pull request Jan 10, 2024
Closed
21 tasks
@build-analysis build-analysis bot mentioned this pull request Jan 11, 2024
Open
2 tasks
@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @BruceForstall

Diffs from a run with alignment enabled.

Aggregated stats for our win-x64 SPMI collections:

-Alignment candidates: 35898
+Alignment candidates: 38075
-Aligned: 13523
+Aligned: 14193

6.1% more candidates, 5.0% more aligned loops
@jakobbotsch jakobbotsch marked this pull request as ready for review January 11, 2024 16:01
@build-analysis build-analysis bot mentioned this pull request Jan 11, 2024
Closed
kunalspathak
kunalspathak approved these changes Jan 11, 2024
View reviewed changes
Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM
@jakobbotsch
Copy link
Member Author

superpmi-replay failures are the timeouts we see every once in a while. The linker tests had a timeout in the macOS job. Other failures are known.
@jakobbotsch jakobbotsch merged commit 602ad47 into dotnet:main Jan 12, 2024
@jakobbotsch jakobbotsch deleted the align-new-loops branch January 12, 2024 13:03
This was referenced Jan 16, 2024
Open
Open
@DrewScoggins DrewScoggins mentioned this pull request Jan 16, 2024
Closed
tmds pushed a commit to tmds/runtime that referenced this pull request Jan 23, 2024
@jakobbotsch @tmds
JIT: Align newly recognized loops (dotnet#96756)
497650f 
Also remove quirk when checking for inner-most loops.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 12, 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
3 participants
@jakobbotsch @BruceForstall @kunalspathak