Skip to content

JIT: Make loop inversion graph based #109346

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

Closed
wants to merge 13 commits into from

Conversation

jakobbotsch
Copy link
Member

Rewrite loop inversion to be graph based and to use the new loop representation.

Contributes to #107749
Contributes to #108913

Currently has large size regressions that I haven't dug into yet.

Rewrite loop inversion to be graph based and to use the new loop
representation.
@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 Oct 29, 2024
@amanasifkhalid
Copy link
Member

Currently has large size regressions that I haven't dug into yet.

I suspect you're unblocking loop cloning in quite a few more instances -- when I prototyped this locally, I had similar size increases.

@jakobbotsch
Copy link
Member Author

I suspect you're unblocking loop cloning in quite a few more instances -- when I prototyped this locally, I had similar size increases.

Doesn't appear to be loop cloning -- there are similar with loop cloning disabled in both the base/diff.

Copy link
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@amanasifkhalid
Copy link
Member

amanasifkhalid commented Mar 19, 2025

@jakobbotsch I hope you don't mind me reviving this, but I looked at the diffs from this change on top of main to see where we stand now. With loop cloning and unrolling disabled, I see some instances where lexical loop inversion would do a transformation that fgOptimizeBranch would have done in the next phase, whereas in this PR, fgOptimizeBranch runs before graph-based loop inversion, and the latter while consider shapes that lexical inversion doesn't recognize. Thus, we can end up with extraneous cloning.

I think fgOptimizeBranch should only run right before layout where we stand to benefit most from it, since it can mess with loop recognition/opts; I can try removing the early pass as a prerequisite to this PR. However, with loop cloning/unrolling and early fgOptimizeBranch disabled, the size regressions don't come down all that much, since we're still recognizing and inverting more loops now. I'm not sure if the diffs I'm seeing are as extreme as the ones you started out with; I'll push my changes here for visibility. (I don't have permission to push to your branch; I'll open a follow-up PR). All I needed to change was add some profile maintenance code in since asserts are now enabled (a nice side effect of your implementation is loop inversion will no longer introduce profile inconsistency).

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