Skip to content

JIT: skip loop inversion for loops that are already bottom-tested #128459

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

Open
AndyAyersMS wants to merge 2 commits into
base: main
Choose a base branch
from
+47 −7
Open

JIT: skip loop inversion for loops that are already bottom-tested #128459

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd8ed68
JIT: skip loop inversion for loops that are already bottom-tested
AndyAyersMS May 21, 2026
c2d855e
JIT: remove now-redundant IV-tested check
AndyAyersMS May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 47 additions & 7 deletions src/coreclr/jit/optimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1840,14 +1840,54 @@ bool Compiler::optTryInvertWhileLoop(FlowGraphNaturalLoop* loop)

// There may be multiple exits, and one of the other exits may also be a
// latch. That latch could be preferable to leave (for example because it
// is an IV test).
NaturalLoopIterInfo iterInfo;
if (loop->AnalyzeIteration(&iterInfo) &&
(iterInfo.TestBlock->TrueTargetIs(loop->GetHeader()) != iterInfo.TestBlock->FalseTargetIs(loop->GetHeader())))
{
// Test block is both a latch and exit, so the loop is already inverted in a preferable way.
JITDUMP("No loop-inversion for " FMT_LP " since it is already inverted (with an IV test)\n", loop->GetIndex());
// is an IV test). The general BBJ_COND-latch-that-exits check below
// subsumes the IV-test case.

// 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.
//
Comment on lines +1846 to +1850
Copy link
Copy Markdown
Member

@jakobbotsch jakobbotsch May 22, 2026

Choose a reason for hiding this comment

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

I wonder if this more or less subsumes the AnalyzeIteration handling above. Does the above case get cases this one doesn't?

Copy link
Copy Markdown
Member Author

@AndyAyersMS AndyAyersMS May 22, 2026

Choose a reason for hiding this comment

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

Seems likely that the initial check is now redundant, I'll take a look.

auto isExitingCondLatch = [&](BasicBlock* block) -> bool {
if ((block == condBlock) || !block->KindIs(BBJ_COND))
{
return false;
}
for (FlowEdge* const exitEdge : loop->ExitEdges())
{
if (exitEdge->getSourceBlock() == block)
{
return true;
}
}
return false;
};

for (FlowEdge* const backEdge : loop->BackEdges())
{
BasicBlock* const latch = backEdge->getSourceBlock();

if (isExitingCondLatch(latch))
{
JITDUMP("No loop-inversion for " FMT_LP "; latch " FMT_BB " already makes it bottom-tested\n",
loop->GetIndex(), latch->bbNum);
return false;
}

if (latch->KindIs(BBJ_ALWAYS) && latch->isEmpty())
{
for (FlowEdge* const predEdge : latch->PredEdges())
{
BasicBlock* const pred = predEdge->getSourceBlock();
if (loop->ContainsBlock(pred) && isExitingCondLatch(pred))
{
JITDUMP("No loop-inversion for " FMT_LP "; predecessor " FMT_BB " of canonical latch " FMT_BB
" already makes it bottom-tested\n",
loop->GetIndex(), pred->bbNum, latch->bbNum);
return false;
}
}
}
}

JITDUMP("Condition in block " FMT_BB " of loop " FMT_LP " is a candidate for duplication to invert the loop\n",
Expand Down
Loading