JIT: Move loop inversion to after loop recognition

[amanasifkhalid]

Prerequisite to #113709. I expect diffs to go both ways: In some cases, loop canonicalization unlocks pattern-based loop inversion, whereas in other cases, we now recognize fewer loops due to loop inversion no longer introducing new cycles pre-canonicalization.

[dotnet-policy-service]

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

[Copilot]

Pull Request Overview

This PR moves the loop inversion phase to after loop recognition, adds immediate block compaction/removal for newly altered test blocks, and triggers a DFS rebuild with fresh loop analysis when any loops were inverted.

• Add single-predecessor block compaction/removal in optInvertWhileLoop
• Recompute the DFS tree and re-run loop finding after any loop inversions
• Relocate the PHASE_INVERT_LOOPS call in the compilation pipeline

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
optimizer.cpp	Inserted block compaction/removal and DFS invalidation
compiler.cpp	Moved the loop inversion phase to a later point in compCompile

Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.cpp:4668

• Add targeted tests that verify the new phase ordering and ensure that both block compaction and removal occur as expected after loop inversion.

DoPhase(this, PHASE_INVERT_LOOPS, &Compiler::optInvertLoops);

[amanasifkhalid]

The diffs will be hard to parse for this, so I'm looking more at metrics. Here are some metric diffs for aspnet on win-x64:

Base:

• Loops found: 29252
• Loops inverted: 10694
• Loops cloned: 1885
• Loops unrolled: 12
• Loops IV widened: 3047
• Widened IVs: 3047
• Unused IVs removed: 4539
• Loops downward counted: 1873
• Loops strength reduced: 1702
• RBO: 30708
• Jump threadings: 9735

Diff:

• Loops found: 29085 (-167)
• Loops inverted: 9074 (-1620)
• Loops cloned: 3596 (+1711)
• Loops unrolled: 12
• Loops IV widened: 2999 (-48)
• Widened IVs: 2999 (-48)
• Unused IVs removed: 4498 (-41)
• Loops downward counted: 1863 (-10)
• Loops strength reduced: 1693 (-9)
• RBO: 33859 (+3151)
• Jump threadings: 9774 (+39)

We can see from the metrics that we're inverting fewer loops overall, but there are plenty of cases where we invert new loops, thus unblocking other loop opts -- in particular, we're doing a lot more cloning. Fewer loops found overall is due to loop inversion no longer introducing new cycles before loop recognition runs.

PerfScore diffs are overwhelmingly negative in non-PGO collections. This might be heuristic-derived profile weights for cloned loops inflating PerfScores, and/or something else...

[AndyAyersMS]

Diffs

Assuming the diffs are largely cloning related, it appears that extra cloning is pretty costly. It is hard to know how much of it is really beneficial. I wish we had better heuristics.

[amanasifkhalid]

It is hard to know how much of it is really beneficial.

Right, because of this, I've decided to flip my ordering and enable graph-based loop inversion with the existing phase ordering. Locally, the diffs are slightly easier to triage. Once that's in, hopefully it'll be easier to triage the diffs on this PR and see if there's anything actionable.