Skip to content

Commit

Permalink
JIT: Refine post-dominance check in strength reduction (#104687)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jakobbotsch authored Jul 12, 2024
1 parent 61f08d3 commit b8ac8b2
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 12 deletions.
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
70 changes: 70 additions & 0 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<BasicBlock*> 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
//
Expand Down
10 changes: 1 addition & 9 deletions src/coreclr/jit/inductionvariableopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1853,16 +1853,8 @@ BasicBlock* StrengthReductionContext::FindUpdateInsertionPoint(ArrayStack<Cursor
}
else
{
if (m_comp->optReachable(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;
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/jit/redundantbranchopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down

0 comments on commit b8ac8b2

Please sign in to comment.