From c03a7586072deee4cbf04f4f140061351c2bfc9d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 7 Jun 2024 23:31:06 +0200 Subject: [PATCH 1/5] JIT: Enable downwards optimization for multi-exit loops --- src/coreclr/jit/compiler.h | 4 ++ src/coreclr/jit/inductionvariableopts.cpp | 72 ++++++++++++++++++++--- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index dd92e05f464afc..bf00e648e15111 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7554,6 +7554,10 @@ class Compiler bool optMakeLoopDownwardsCounted(ScalarEvolutionContext& scevContext, FlowGraphNaturalLoop* loop, LoopLocalOccurrences* loopLocals); + bool optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevContext, + FlowGraphNaturalLoop* loop, + BasicBlock* exiting, + LoopLocalOccurrences* loopLocals); bool optWidenPrimaryIV(FlowGraphNaturalLoop* loop, unsigned lclNum, ScevAddRec* addRec, diff --git a/src/coreclr/jit/inductionvariableopts.cpp b/src/coreclr/jit/inductionvariableopts.cpp index a3487de6757b90..740d94abea5909 100644 --- a/src/coreclr/jit/inductionvariableopts.cpp +++ b/src/coreclr/jit/inductionvariableopts.cpp @@ -879,21 +879,75 @@ bool Compiler::optMakeLoopDownwardsCounted(ScalarEvolutionContext& scevContext, { JITDUMP("Checking if we should make " FMT_LP " downwards counted\n", loop->GetIndex()); - if (loop->ExitEdges().size() != 1) + // Common case where the backedge and exit edge is the same + if ((loop->ExitEdges().size() == 1) && (loop->BackEdges().size() == 1)) { - // With multiple exits we generally can only compute an upper bound on - // the backedge count. - JITDUMP(" No; has multiple exits\n"); - return false; + BasicBlock* exiting = loop->ExitEdge(0)->getSourceBlock(); + BasicBlock* latch = loop->BackEdge(0)->getSourceBlock(); + if (exiting == latch) + { + JITDUMP(" Considering exiting block " FMT_BB "\n", exiting->bbNum); + return optMakeExitTestDownwardsCounted(scevContext, loop, exiting, loopLocals); + } } - BasicBlock* exiting = loop->ExitEdge(0)->getSourceBlock(); - if (!exiting->KindIs(BBJ_COND)) + if (m_domTree == nullptr) { - JITDUMP(" No; exit is not BBJ_COND\n"); - return false; + m_domTree = FlowGraphDominatorTree::Build(m_dfsTree); } + BasicBlock* dominates = nullptr; + + for (FlowEdge* backEdge : loop->BackEdges()) + { + if (dominates == nullptr) + { + dominates = backEdge->getSourceBlock(); + } + else + { + dominates = m_domTree->Intersect(dominates, backEdge->getSourceBlock()); + } + } + + bool changed = false; + while ((dominates != nullptr) && loop->ContainsBlock(dominates)) + { + if (dominates->KindIs(BBJ_COND) && + (!loop->ContainsBlock(dominates->GetTrueTarget()) || !loop->ContainsBlock(dominates->GetFalseTarget()))) + { + JITDUMP(" Considering exiting block " FMT_BB "\n", dominates->bbNum); + // 'dominates' is an exiting block that dominates all backedges. + changed |= optMakeExitTestDownwardsCounted(scevContext, loop, dominates, loopLocals); + } + + dominates = dominates->bbIDom; + } + + return changed; +} + +//------------------------------------------------------------------------ +// optMakeExitTestDownwardsCounted: +// Try to modify the condition of a specific BBJ_COND exiting block to be on +// a downwards counted IV if profitable. +// +// Parameters: +// scevContext - SCEV context +// loop - The specific loop +// exiting - Exiting block +// loopLocals - Data structure tracking local uses +// +// Returns: +// True if any modification was made. +// +bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevContext, + FlowGraphNaturalLoop* loop, + BasicBlock* exiting, + LoopLocalOccurrences* loopLocals) +{ + assert(exiting->KindIs(BBJ_COND)); + Statement* jtrueStmt = exiting->lastStmt(); GenTree* jtrue = jtrueStmt->GetRootNode(); assert(jtrue->OperIs(GT_JTRUE)); From 228e9f0c7901c33142c5cb2603aecef9eb8400a6 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 8 Jun 2024 00:53:16 +0200 Subject: [PATCH 2/5] Fix assert --- src/coreclr/jit/inductionvariableopts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/inductionvariableopts.cpp b/src/coreclr/jit/inductionvariableopts.cpp index 740d94abea5909..5dc81d4b255735 100644 --- a/src/coreclr/jit/inductionvariableopts.cpp +++ b/src/coreclr/jit/inductionvariableopts.cpp @@ -884,7 +884,7 @@ bool Compiler::optMakeLoopDownwardsCounted(ScalarEvolutionContext& scevContext, { BasicBlock* exiting = loop->ExitEdge(0)->getSourceBlock(); BasicBlock* latch = loop->BackEdge(0)->getSourceBlock(); - if (exiting == latch) + if ((exiting == latch) && exiting->KindIs(BBJ_COND)) { JITDUMP(" Considering exiting block " FMT_BB "\n", exiting->bbNum); return optMakeExitTestDownwardsCounted(scevContext, loop, exiting, loopLocals); From be5b5cf2489d443dd59133bf0e26f50e4ecc99fb Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 19 Jun 2024 16:06:12 +0200 Subject: [PATCH 3/5] Simplify at minor cost of TP --- src/coreclr/jit/inductionvariableopts.cpp | 55 +++-------------------- 1 file changed, 6 insertions(+), 49 deletions(-) diff --git a/src/coreclr/jit/inductionvariableopts.cpp b/src/coreclr/jit/inductionvariableopts.cpp index 5dc81d4b255735..90628617047390 100644 --- a/src/coreclr/jit/inductionvariableopts.cpp +++ b/src/coreclr/jit/inductionvariableopts.cpp @@ -879,23 +879,6 @@ bool Compiler::optMakeLoopDownwardsCounted(ScalarEvolutionContext& scevContext, { JITDUMP("Checking if we should make " FMT_LP " downwards counted\n", loop->GetIndex()); - // Common case where the backedge and exit edge is the same - if ((loop->ExitEdges().size() == 1) && (loop->BackEdges().size() == 1)) - { - BasicBlock* exiting = loop->ExitEdge(0)->getSourceBlock(); - BasicBlock* latch = loop->BackEdge(0)->getSourceBlock(); - if ((exiting == latch) && exiting->KindIs(BBJ_COND)) - { - JITDUMP(" Considering exiting block " FMT_BB "\n", exiting->bbNum); - return optMakeExitTestDownwardsCounted(scevContext, loop, exiting, loopLocals); - } - } - - if (m_domTree == nullptr) - { - m_domTree = FlowGraphDominatorTree::Build(m_dfsTree); - } - BasicBlock* dominates = nullptr; for (FlowEdge* backEdge : loop->BackEdges()) @@ -1070,40 +1053,13 @@ bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevConte return false; } - // Commonly there is only a single shared exit and backedge. In that case - // we do not even need to compute dominators since all backedges are - // definitely dominated by the exit. - if ((loop->BackEdges().size() == 1) && loop->BackEdge(0)->getSourceBlock() == loop->ExitEdge(0)->getSourceBlock()) - { - // Exit definitely dominates the latch since they are the same block. - // Fall through. - JITDUMP(" Loop exit is also the only backedge\n"); - } - else + for (FlowEdge* backedge : loop->BackEdges()) { - for (FlowEdge* backedge : loop->BackEdges()) + if (!m_domTree->Dominates(exiting, backedge->getSourceBlock())) { - // We know dom(x, y) => ancestor(x, y), so we can utilize the - // contrapositive !ancestor(x, y) => !dom(x, y) to avoid computing - // dominators in some cases. - if (!m_dfsTree->IsAncestor(exiting, backedge->getSourceBlock())) - { - JITDUMP(" No; exiting block " FMT_BB " is not an ancestor of backedge source " FMT_BB "\n", - exiting->bbNum, backedge->getSourceBlock()->bbNum); - return false; - } - - if (m_domTree == nullptr) - { - m_domTree = FlowGraphDominatorTree::Build(m_dfsTree); - } - - if (!m_domTree->Dominates(exiting, backedge->getSourceBlock())) - { - JITDUMP(" No; exiting block " FMT_BB " does not dominate backedge source " FMT_BB "\n", exiting->bbNum, - backedge->getSourceBlock()->bbNum); - return false; - } + JITDUMP(" No; exiting block " FMT_BB " does not dominate backedge source " FMT_BB "\n", exiting->bbNum, + backedge->getSourceBlock()->bbNum); + return false; } } @@ -1224,6 +1180,7 @@ PhaseStatus Compiler::optInductionVariables() bool changed = false; m_dfsTree = fgComputeDfs(); + m_domTree = FlowGraphDominatorTree::Build(m_dfsTree); m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); LoopLocalOccurrences loopLocals(m_loops); From 57b3f709b2beec4fdd38368fbef271bc68a9f818 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 19 Jun 2024 16:08:23 +0200 Subject: [PATCH 4/5] Run jit-format --- src/coreclr/jit/inductionvariableopts.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/inductionvariableopts.cpp b/src/coreclr/jit/inductionvariableopts.cpp index 70f43c63e7aa09..ab2808d64af680 100644 --- a/src/coreclr/jit/inductionvariableopts.cpp +++ b/src/coreclr/jit/inductionvariableopts.cpp @@ -1180,9 +1180,9 @@ PhaseStatus Compiler::optInductionVariables() bool changed = false; optReachableBitVecTraits = nullptr; - m_dfsTree = fgComputeDfs(); - m_domTree = FlowGraphDominatorTree::Build(m_dfsTree); - m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); + m_dfsTree = fgComputeDfs(); + m_domTree = FlowGraphDominatorTree::Build(m_dfsTree); + m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); LoopLocalOccurrences loopLocals(m_loops); From 3774e42f2b77b86b0db4c943b007c66421f13db2 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 19 Jun 2024 21:14:19 +0200 Subject: [PATCH 5/5] Clean up --- src/coreclr/jit/inductionvariableopts.cpp | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/coreclr/jit/inductionvariableopts.cpp b/src/coreclr/jit/inductionvariableopts.cpp index ab2808d64af680..030d5d95950eba 100644 --- a/src/coreclr/jit/inductionvariableopts.cpp +++ b/src/coreclr/jit/inductionvariableopts.cpp @@ -1053,16 +1053,6 @@ bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevConte return false; } - for (FlowEdge* backedge : loop->BackEdges()) - { - if (!m_domTree->Dominates(exiting, backedge->getSourceBlock())) - { - JITDUMP(" No; exiting block " FMT_BB " does not dominate backedge source " FMT_BB "\n", exiting->bbNum, - backedge->getSourceBlock()->bbNum); - return false; - } - } - // At this point we know that the single exit dominates all backedges. JITDUMP(" All backedges are dominated by exiting block " FMT_BB "\n", exiting->bbNum);