From 0ffd465aec1cbdbebc1e1b370b13dca9c015029e Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 25 Jun 2026 13:15:47 -0700 Subject: [PATCH 1/2] JIT: only skip loop inversion when the IV test is the bottom test The bail-out added in #128459 fires for any BBJ_COND latch (or BBJ_COND predecessor of a canonical BBJ_ALWAYS latch) that exits the loop, but that block may be an unrelated early-exit and not the loop's continuation test. Restrict the bail-out to the case where the recognized IV test (from AnalyzeIteration) is at that position. AnalyzeIteration is called lazily so only loops that match the structural shape pay the cost. Fixes the Dictionary regression reported in #128910. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/optimizer.cpp | 49 +++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 98ee3b6aa5a9c1..5476b745caab1e 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -1903,14 +1903,16 @@ 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). 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. + // latch. So avoid inversion if the loop is already bottom-tested. + // + // We check for two cases: + // 1. The latch itself is a BBJ_COND that exits the loop, AND the latch is + // the recognized IV test (so the latch BBJ_COND determines loop + // continuation, not a body-internal early exit that happens to feed the + // back-edge). + // 2. The latch is a BBJ_ALWAYS (installed by optCanonicalizeBackedges for + // loops with originally multiple backedges) and one of its in-loop + // predecessors is the recognized IV test BBJ_COND that exits the loop. // auto isExitingCondLatch = [&](BasicBlock* block) -> bool { if ((block == condBlock) || !block->KindIs(BBJ_COND)) @@ -1927,13 +1929,34 @@ bool Compiler::optTryInvertWhileLoop(FlowGraphNaturalLoop* loop) return false; }; + // Only run AnalyzeIteration when we see a back-edge that could + // qualify for the bail-out (i.e. its source block, or an in-loop predecessor + // of a BBJ_ALWAYS canonical latch, is a candidate for being the IV test). + // + NaturalLoopIterInfo iterInfo; + bool analyzedIteration = false; + BasicBlock* ivTestBlock = nullptr; + + auto isIvTest = [&](BasicBlock* candidate) -> bool { + if (!analyzedIteration) + { + analyzedIteration = true; + if (loop->AnalyzeIteration(&iterInfo)) + { + ivTestBlock = iterInfo.TestBlock; + } + } + return (ivTestBlock != nullptr) && (candidate == ivTestBlock); + }; + for (FlowEdge* const backEdge : loop->BackEdges()) { BasicBlock* const latch = backEdge->getSourceBlock(); - if (isExitingCondLatch(latch)) + if (isExitingCondLatch(latch) && isIvTest(latch)) { - JITDUMP("No loop-inversion for " FMT_LP "; latch " FMT_BB " already makes it bottom-tested\n", + JITDUMP("No loop-inversion for " FMT_LP "; IV-test latch " FMT_BB + " already makes it bottom-tested\n", loop->GetIndex(), latch->bbNum); return false; } @@ -1943,10 +1966,10 @@ bool Compiler::optTryInvertWhileLoop(FlowGraphNaturalLoop* loop) for (FlowEdge* const predEdge : latch->PredEdges()) { BasicBlock* const pred = predEdge->getSourceBlock(); - if (loop->ContainsBlock(pred) && isExitingCondLatch(pred)) + if (loop->ContainsBlock(pred) && isExitingCondLatch(pred) && isIvTest(pred)) { - JITDUMP("No loop-inversion for " FMT_LP "; predecessor " FMT_BB " of canonical latch " FMT_BB - " already makes it bottom-tested\n", + JITDUMP("No loop-inversion for " FMT_LP "; IV-test predecessor " FMT_BB " of canonical latch " + FMT_BB " already makes it bottom-tested\n", loop->GetIndex(), pred->bbNum, latch->bbNum); return false; } From 10656cdd0be1a8121587080bb77cdcd28385b385 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 25 Jun 2026 13:44:50 -0700 Subject: [PATCH 2/2] Format Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/optimizer.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 5476b745caab1e..74e598e0ea579a 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -1955,8 +1955,7 @@ bool Compiler::optTryInvertWhileLoop(FlowGraphNaturalLoop* loop) if (isExitingCondLatch(latch) && isIvTest(latch)) { - JITDUMP("No loop-inversion for " FMT_LP "; IV-test latch " FMT_BB - " already makes it bottom-tested\n", + JITDUMP("No loop-inversion for " FMT_LP "; IV-test latch " FMT_BB " already makes it bottom-tested\n", loop->GetIndex(), latch->bbNum); return false; } @@ -1968,8 +1967,8 @@ bool Compiler::optTryInvertWhileLoop(FlowGraphNaturalLoop* loop) BasicBlock* const pred = predEdge->getSourceBlock(); if (loop->ContainsBlock(pred) && isExitingCondLatch(pred) && isIvTest(pred)) { - JITDUMP("No loop-inversion for " FMT_LP "; IV-test predecessor " FMT_BB " of canonical latch " - FMT_BB " already makes it bottom-tested\n", + JITDUMP("No loop-inversion for " FMT_LP "; IV-test predecessor " FMT_BB + " of canonical latch " FMT_BB " already makes it bottom-tested\n", loop->GetIndex(), pred->bbNum, latch->bbNum); return false; }