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: Remove fgRenumberBlocks calls in loop opt phases #110227

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

amanasifkhalid
Copy link
Member

Follow-up to #110026. This required a small refactor of FlowGraphNaturalLoop::VisitLoopBlocksLexical to remove its bbNum checks.

@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 Nov 27, 2024
Copy link
Contributor

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

@@ -5145,37 +5145,18 @@ BasicBlockVisit FlowGraphNaturalLoop::VisitLoopBlocks(TFunc func)
template <typename TFunc>
BasicBlockVisit FlowGraphNaturalLoop::VisitLoopBlocksLexical(TFunc func)
Copy link
Member

Choose a reason for hiding this comment

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

We should likely just remove it. The usages are just for duplicating loop blocks, and those probably don't need to be duplicated lexically anymore.

No need to make the change here if you don't want to, I can do it separately.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it, I saw #110020 is introducing new call sites for it, so I decided to keep it around to avoid conflicts with that PR. I can remove it in one go once that PR is in.

@amanasifkhalid
Copy link
Member Author

Small diffs from the bbNum dependency in fgUpdateFlowGraph, for methods that don't have loops and thus won't trigger block renumbering in optSetBlockWeights.

@amanasifkhalid amanasifkhalid merged commit c6bebf0 into dotnet:main Nov 28, 2024
105 of 108 checks passed
@amanasifkhalid amanasifkhalid deleted the more-renumbering-removal branch November 28, 2024 16:26
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this pull request Dec 10, 2024
Follow-up to dotnet#110026. This required a small refactor of FlowGraphNaturalLoop::VisitLoopBlocksLexical to remove its bbNum checks.
amanasifkhalid added a commit that referenced this pull request Dec 12, 2024
Follow-up to #110227. In the few places where we still visit loop blocks in lexical order, just visit them in RPO instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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