From 5a6529ba70b345b8fcdf02606b5bb17c3fc24906 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 10 Jul 2024 19:32:53 +0200 Subject: [PATCH 1/2] JIT: Refine post-dominance check in strength reduction When strength reduction has to find an insertion point for the new primary IV update it needs to find a block that post-dominates all the users of the IV. This was using `optReachable` before, but that is conservative since it finds paths that may exit the loop. This implements a more precise check. --- src/coreclr/jit/compiler.h | 2 + src/coreclr/jit/flowgraph.cpp | 70 +++++++++++++++++++++++ src/coreclr/jit/inductionvariableopts.cpp | 10 +--- src/coreclr/jit/redundantbranchopts.cpp | 4 +- 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 08307977ac6920..72d89347b3eed7 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2232,6 +2232,8 @@ class FlowGraphNaturalLoop bool MayExecuteBlockMultipleTimesPerIteration(BasicBlock* block); + bool IsPostDominatedOnLoopIteration(BasicBlock* block, BasicBlock* postDominator); + #ifdef DEBUG static void Dump(FlowGraphNaturalLoop* loop); #endif // DEBUG diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index b4105197e4b7b0..659585323fa621 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -5801,6 +5801,76 @@ bool FlowGraphNaturalLoop::MayExecuteBlockMultipleTimesPerIteration(BasicBlock* return false; } +//------------------------------------------------------------------------ +// IsPostDominatedOnLoopIteration: +// Check whether control will always flow through "postDominator" if starting +// at "block" and a backedge is taken. +// +// Parameters: +// block - The basic block +// postDominator - Block to query postdominance of +// +// Returns: +// True if so. +// +bool FlowGraphNaturalLoop::IsPostDominatedOnLoopIteration(BasicBlock* block, BasicBlock* postDominator) +{ + assert(ContainsBlock(block) && ContainsBlock(postDominator)); + + unsigned index; + bool gotIndex = TryGetLoopBlockBitVecIndex(block, &index); + assert(gotIndex); + + Compiler* comp = m_dfsTree->GetCompiler(); + ArrayStack stack(comp->getAllocator(CMK_Loops)); + + BitVecTraits traits = LoopBlockTraits(); + BitVec visited(BitVecOps::MakeEmpty(&traits)); + + stack.Push(block); + BitVecOps::AddElemD(&traits, visited, index); + + auto queueSuccs = [=, &stack, &traits, &visited](BasicBlock* succ) { + if (succ == m_header) + { + // We managed to reach the header without going through "postDominator". + return BasicBlockVisit::Abort; + } + + unsigned index; + if (!TryGetLoopBlockBitVecIndex(succ, &index) || !BitVecOps::IsMember(&traits, m_blocks, index)) + { + // Block is not inside loop + return BasicBlockVisit::Continue; + } + + if (!BitVecOps::TryAddElemD(&traits, visited, index)) + { + // Block already visited + return BasicBlockVisit::Continue; + } + + stack.Push(succ); + return BasicBlockVisit::Continue; + }; + + while (stack.Height() > 0) + { + BasicBlock* block = stack.Pop(); + if (block == postDominator) + { + continue; + } + + if (block->VisitAllSuccs(comp, queueSuccs) == BasicBlockVisit::Abort) + { + return false; + } + } + + return true; +} + //------------------------------------------------------------------------ // IterConst: Get the constant with which the iterator is modified // diff --git a/src/coreclr/jit/inductionvariableopts.cpp b/src/coreclr/jit/inductionvariableopts.cpp index bdbece4cc3614e..bb4131a5c3b4c1 100644 --- a/src/coreclr/jit/inductionvariableopts.cpp +++ b/src/coreclr/jit/inductionvariableopts.cpp @@ -1853,16 +1853,8 @@ BasicBlock* StrengthReductionContext::FindUpdateInsertionPoint(ArrayStackoptReachable(cursor.Block, m_loop->GetHeader(), insertionPoint)) + if (!m_loop->IsPostDominatedOnLoopIteration(cursor.Block, insertionPoint)) { - // Header is reachable without going through the insertion - // point, meaning that the insertion point does not - // post-dominate the use of an IV we want to replace. - // - // TODO-CQ: We only need to check whether the header is - // reachable from inside the loop, which is both cheaper and - // less conservative to check. - // return nullptr; } } diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 99e3fbba31ce02..9e44637f991903 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -2224,13 +2224,11 @@ bool Compiler::optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlo return BasicBlockVisit::Abort; } - if (BitVecOps::IsMember(optReachableBitVecTraits, optReachableBitVec, succ->bbNum)) + if (!BitVecOps::TryAddElemD(optReachableBitVecTraits, optReachableBitVec, succ->bbNum)) { return BasicBlockVisit::Continue; } - BitVecOps::AddElemD(optReachableBitVecTraits, optReachableBitVec, succ->bbNum); - stack.Push(succ); return BasicBlockVisit::Continue; }); From fdbc9d4aba84b7b1b673699c05cf34f71fddf378 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 10 Jul 2024 19:44:08 +0200 Subject: [PATCH 2/2] Run jit-format --- src/coreclr/jit/flowgraph.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 659585323fa621..e1c6febdf850e9 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -5818,14 +5818,14 @@ bool FlowGraphNaturalLoop::IsPostDominatedOnLoopIteration(BasicBlock* block, Bas assert(ContainsBlock(block) && ContainsBlock(postDominator)); unsigned index; - bool gotIndex = TryGetLoopBlockBitVecIndex(block, &index); + bool gotIndex = TryGetLoopBlockBitVecIndex(block, &index); assert(gotIndex); - Compiler* comp = m_dfsTree->GetCompiler(); + Compiler* comp = m_dfsTree->GetCompiler(); ArrayStack stack(comp->getAllocator(CMK_Loops)); BitVecTraits traits = LoopBlockTraits(); - BitVec visited(BitVecOps::MakeEmpty(&traits)); + BitVec visited(BitVecOps::MakeEmpty(&traits)); stack.Push(block); BitVecOps::AddElemD(&traits, visited, index); @@ -5852,7 +5852,7 @@ bool FlowGraphNaturalLoop::IsPostDominatedOnLoopIteration(BasicBlock* block, Bas stack.Push(succ); return BasicBlockVisit::Continue; - }; + }; while (stack.Height() > 0) {