JIT: skip loop inversion for loops that are already bottom-tested#128459
JIT: skip loop inversion for loops that are already bottom-tested#128459AndyAyersMS wants to merge 2 commits into
Conversation
In optTryInvertWhileLoop, bail out when the loop's back-edge already
controls loop continuation. Two shapes count:
* a BBJ_COND latch that exits the loop
* a BBJ_ALWAYS latch (installed by optCanonicalizeBackedges for loops
that originally had multiple backedges) with an in-loop predecessor
that is a BBJ_COND exiting the loop
In either case the candidate condBlock is some other early-exit test;
inverting it just adds a redundant test above the loop and splits the
body into two branches per iteration where there was one.
The existing 'already inverted' checks only catch single-block self-
loops and loops whose latch test was recognized as the IV test by
AnalyzeIteration. Loops with an unrecognized bottom test plus a
separate early-exit BBJ_COND in the body fell through and got
inverted on top of an already bottom-tested shape.
PR dotnet#116104 made this more visible by always computing loop iteration
estimates from profile data, which pushes more loops past the size
budget threshold where inversion is attempted.
Fixes dotnet#118494.
github-actions
Bot
added
the
area-CodeGen-coreclr
|
(meant to create as draft, but fine this way too) |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
Another idea for multi-exit loops is to see if the exit blocks dominate one another. If so they divide the loop up into segments: [head, exit 1), [exit 1, exit 2), ... We can try and rotate so that there is no computation after the last exit (bottom test) and that the first segment has the best hoisting opportunities. |
There was a problem hiding this comment.
Pull request overview
This PR updates CoreCLR JIT loop inversion (optTryInvertWhileLoop) to skip loop inversion when the loop is already bottom-tested by its backedge structure. The goal is to avoid introducing a redundant preheader test and extra per-iteration control flow when the loop’s continuation is already governed by a bottom test (including the canonicalized multi-backedge “BBJ_ALWAYS latch” shape).
Changes:
- Add an early bail-out in
optTryInvertWhileLoopwhen a backedge source is an exitingBBJ_CONDlatch. - Extend the bail-out to cover the canonical multi-backedge case where a
BBJ_ALWAYSlatch’s in-loop predecessor is an exitingBBJ_COND.
|
Fairly extensive code size and tp reduction. Suggests we are perhaps being too aggressive with our current approach? I will do some targeted benchmarking here, but it's hard to envision how to assess this as a whole without actually enabling it and then seeing what happens. |
|
@EgorBot -arm -linux_arm64 -intel -linux_x64 "System.Text.Tests.Perf_Encoding" |
@AndyAyersMS I haven't enabled the smart mode yet in the bot (to use AI to parse arguments), but you're expected to use BDN's arguments after bot specific ones, so it should be |
Yeah operator error, it was there initially. |
|
@EgorBot -arm -linux_arm64 -intel -linux_x64 --filter "System.Text.Tests.Perf_Encoding" |
|
Also it is eating the wildcards, so probably needs more tweaking. |
|
@EgorBot -arm -linux_arm64 -intel -linux_x64 --filter "System.Text.Tests.Perf_Encoding*" |
it should be passed to the bot correctly, it's just github renders them as italic |
| // If the loop is already bottom-tested (has a BBJ_COND latch that exits the loop), | ||
| // there is no need to invert. Also handle the canonical multi-backedge case, where | ||
| // optCanonicalizeBackedges has installed a BBJ_ALWAYS latch whose in-loop predecessors | ||
| // are the original bottom tests. | ||
| // |
There was a problem hiding this comment.
I wonder if this more or less subsumes the AnalyzeIteration handling above. Does the above case get cases this one doesn't?
There was a problem hiding this comment.
Seems likely that the initial check is now redundant, I'll take a look.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In optTryInvertWhileLoop, bail out when the loop's back-edge already controls loop continuation. Two shapes count:
In either case the candidate condBlock is some other early-exit test; inverting it just adds a redundant test above the loop and splits the body into two branches per iteration where there was one.
The existing 'already inverted' checks only catch single-block self- loops and loops whose latch test was recognized as the IV test by AnalyzeIteration. Loops with an unrecognized bottom test plus a separate early-exit BBJ_COND in the body fell through and got inverted on top of an already bottom-tested shape.
PR #116104 made this more visible by always computing loop iteration estimates from profile data, which pushes more loops past the size budget threshold where inversion is attempted.
Fixes #118494.