Skip to content

JIT: Consider conditionally executed loop statements for hoisting #117829

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

Merged
merged 1 commit into from
Jul 18, 2025
Merged

JIT: Consider conditionally executed loop statements for hoisting #117829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6954,13 +6954,13 @@ class Compiler

// Hoist all expressions in "blocks" that are invariant in "loop"
// outside of that loop.
void optHoistLoopBlocks(FlowGraphNaturalLoop* loop, ArrayStack<BasicBlock*>* blocks, LoopHoistContext* hoistContext);
void optHoistLoopBlocks(FlowGraphNaturalLoop* loop, BitVecTraits* traits, BitVec blocks, LoopHoistContext* hoistContext);

// Return true if the tree looks profitable to hoist out of "loop"
bool optIsProfitableToHoistTree(GenTree* tree, FlowGraphNaturalLoop* loop, LoopHoistContext* hoistCtxt);
bool optIsProfitableToHoistTree(GenTree* tree, FlowGraphNaturalLoop* loop, LoopHoistContext* hoistCtxt, bool defExecuted);

// Performs the hoisting "tree" into the PreHeader for "loop"
void optHoistCandidate(GenTree* tree, BasicBlock* treeBb, FlowGraphNaturalLoop* loop, LoopHoistContext* hoistCtxt);
void optHoistCandidate(GenTree* tree, BasicBlock* treeBb, FlowGraphNaturalLoop* loop, LoopHoistContext* hoistCtxt, bool defExecuted);

// Note the new SSA uses in tree
void optRecordSsaUses(GenTree* tree, BasicBlock* block);
Expand Down
84 changes: 52 additions & 32 deletions src/coreclr/jit/optimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3713,7 +3713,9 @@ bool Compiler::optHoistThisLoop(FlowGraphNaturalLoop* loop, LoopHoistContext* ho
// We really should consider hoisting from conditionally executed blocks, if they are frequently executed
// and it is safe to evaluate the tree early.
//
ArrayStack<BasicBlock*> defExec(getAllocatorLoopHoist());
assert(m_dfsTree != nullptr);
BitVecTraits traits(m_dfsTree->PostOrderTraits());
BitVec defExec(BitVecOps::MakeEmpty(&traits));

// Add the pre-headers of any child loops to the list of blocks to consider for hoisting.
// Note that these are not necessarily definitely executed. However, it is a heuristic that they will
Expand Down Expand Up @@ -3771,7 +3773,7 @@ bool Compiler::optHoistThisLoop(FlowGraphNaturalLoop* loop, LoopHoistContext* ho
}
}
JITDUMP(" -- " FMT_BB " (child loop pre-header)\n", childPreHead->bbNum);
defExec.Push(childPreHead);
BitVecOps::AddElemD(&traits, defExec, childPreHead->bbPostorderNum);
}

if (loop->ExitEdges().size() == 1)
Expand All @@ -3796,7 +3798,7 @@ bool Compiler::optHoistThisLoop(FlowGraphNaturalLoop* loop, LoopHoistContext* ho
while ((cur != nullptr) && (cur != loop->GetHeader()) && loop->ContainsBlock(cur))
{
JITDUMP(" -- " FMT_BB " (dominate exit block)\n", cur->bbNum);
defExec.Push(cur);
BitVecOps::AddElemD(&traits, defExec, cur->bbPostorderNum);
cur = cur->bbIDom;
}

Expand All @@ -3812,9 +3814,9 @@ bool Compiler::optHoistThisLoop(FlowGraphNaturalLoop* loop, LoopHoistContext* ho
}

JITDUMP(" -- " FMT_BB " (header block)\n", loop->GetHeader()->bbNum);
defExec.Push(loop->GetHeader());
BitVecOps::AddElemD(&traits, defExec, loop->GetHeader()->bbPostorderNum);

optHoistLoopBlocks(loop, &defExec, hoistCtxt);
optHoistLoopBlocks(loop, &traits, defExec, hoistCtxt);

unsigned numHoisted = hoistCtxt->m_hoistedFPExprCount + hoistCtxt->m_hoistedExprCount;
#ifdef FEATURE_MASKED_HW_INTRINSICS
Expand All @@ -3823,7 +3825,10 @@ bool Compiler::optHoistThisLoop(FlowGraphNaturalLoop* loop, LoopHoistContext* ho
return numHoisted > 0;
}

bool Compiler::optIsProfitableToHoistTree(GenTree* tree, FlowGraphNaturalLoop* loop, LoopHoistContext* hoistCtxt)
bool Compiler::optIsProfitableToHoistTree(GenTree* tree,
FlowGraphNaturalLoop* loop,
LoopHoistContext* hoistCtxt,
bool defExecuted)
{
bool loopContainsCall = m_loopSideEffects[loop->GetIndex()].ContainsCall;

Expand Down Expand Up @@ -3894,6 +3899,14 @@ bool Compiler::optIsProfitableToHoistTree(GenTree* tree, FlowGraphNaturalLoop* l
// always be a subset of the InOut variables for the loop
assert(loopVarCount <= varInOutCount);

// If this tree isn't guaranteed to execute every loop iteration,
// consider hoisting it only if it is expensive.
//
if (!defExecuted && (tree->GetCostEx() < (IND_COST_EX * 16)))
{
return false;
}

// When loopVarCount >= availRegCount we believe that all of the
// available registers will get used to hold LclVars inside the loop.
// This pessimistically assumes that each loopVar has a conflicting
Expand Down Expand Up @@ -4112,17 +4125,14 @@ void LoopSideEffects::AddModifiedElemType(Compiler* comp, CORINFO_CLASS_HANDLE s
//
// Arguments:
// loop - The loop
// blocks - A stack of blocks belonging to the loop
// traits - Bit vector traits for 'defExecuted'
// defExecuted - Bit vector of definitely executed blocks in the loop
// hoistContext - The loop hoist context
//
// Assumptions:
// The `blocks` stack contains the definitely-executed blocks in
// the loop, in the execution order, starting with the loop entry
// block on top of the stack.
//
void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
ArrayStack<BasicBlock*>* blocks,
LoopHoistContext* hoistContext)
void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
BitVecTraits* traits,
BitVec defExecuted,
LoopHoistContext* hoistContext)
{
class HoistVisitor : public GenTreeVisitor<HoistVisitor>
{
Expand Down Expand Up @@ -4161,6 +4171,8 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
FlowGraphNaturalLoop* m_loop;
LoopHoistContext* m_hoistContext;
BasicBlock* m_currentBlock;
BitVecTraits* m_traits;
BitVec m_defExec;

bool IsNodeHoistable(GenTree* node)
{
Expand Down Expand Up @@ -4283,13 +4295,19 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
UseExecutionOrder = true,
};

HoistVisitor(Compiler* compiler, FlowGraphNaturalLoop* loop, LoopHoistContext* hoistContext)
HoistVisitor(Compiler* compiler,
FlowGraphNaturalLoop* loop,
LoopHoistContext* hoistContext,
BitVecTraits* traits,
BitVec defExec)
: GenTreeVisitor(compiler)
, m_valueStack(compiler->getAllocator(CMK_LoopHoist))
, m_beforeSideEffect(true)
, m_loop(loop)
, m_hoistContext(hoistContext)
, m_currentBlock(nullptr)
, m_traits(traits)
, m_defExec(defExec)
{
}

Expand All @@ -4305,7 +4323,8 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
// hoist the top node?
if (top.m_hoistable)
{
m_compiler->optHoistCandidate(stmt->GetRootNode(), block, m_loop, m_hoistContext);
const bool defExecuted = BitVecOps::IsMember(m_traits, m_defExec, block->bbPostorderNum);
m_compiler->optHoistCandidate(stmt->GetRootNode(), block, m_loop, m_hoistContext, defExecuted);
}
else
{
Expand Down Expand Up @@ -4655,7 +4674,10 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,

if (IsHoistableOverExcepSibling(value.Node(), hasExcep))
{
m_compiler->optHoistCandidate(value.Node(), m_currentBlock, m_loop, m_hoistContext);
const bool defExecuted =
BitVecOps::IsMember(m_traits, m_defExec, m_currentBlock->bbPostorderNum);
m_compiler->optHoistCandidate(value.Node(), m_currentBlock, m_loop, m_hoistContext,
defExecuted);
}

// Don't hoist this tree again.
Expand Down Expand Up @@ -4701,12 +4723,9 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
}
};

HoistVisitor visitor(this, loop, hoistContext);

while (!blocks->Empty())
{
BasicBlock* block = blocks->Pop();
weight_t blockWeight = block->getBBWeight(this);
HoistVisitor visitor(this, loop, hoistContext, traits, defExecuted);
loop->VisitLoopBlocks([&](BasicBlock* block) -> BasicBlockVisit {
const weight_t blockWeight = block->getBBWeight(this);

JITDUMP("\n optHoistLoopBlocks " FMT_BB " (weight=%6s) of loop " FMT_LP " (head: " FMT_BB ")\n",
block->bbNum, refCntWtd2str(blockWeight, /* padForDecimalPlaces */ true), loop->GetIndex(),
Expand All @@ -4715,22 +4734,23 @@ void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop,
if (blockWeight < (BB_UNITY_WEIGHT / 10))
{
JITDUMP(" block weight is too small to perform hoisting.\n");
continue;
}
else
{
visitor.HoistBlock(block);
}

visitor.HoistBlock(block);
}
return BasicBlockVisit::Continue;
});

hoistContext->ResetHoistedInCurLoop();
}

void Compiler::optHoistCandidate(GenTree* tree,
BasicBlock* treeBb,
FlowGraphNaturalLoop* loop,
LoopHoistContext* hoistCtxt)
void Compiler::optHoistCandidate(
GenTree* tree, BasicBlock* treeBb, FlowGraphNaturalLoop* loop, LoopHoistContext* hoistCtxt, bool defExecuted)
{
// It must pass the hoistable profitability tests for this loop level
if (!optIsProfitableToHoistTree(tree, loop, hoistCtxt))
if (!optIsProfitableToHoistTree(tree, loop, hoistCtxt, defExecuted))
{
JITDUMP(" ... not profitable to hoist\n");
return;
Expand Down
Loading