From e24531c7c332895b18f5e63f1e0a9f72b1c8bf09 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 18 Jul 2025 14:05:14 -0400 Subject: [PATCH] Consider conditionally executed statements for hoisting --- src/coreclr/jit/compiler.h | 6 +-- src/coreclr/jit/optimizer.cpp | 84 ++++++++++++++++++++++------------- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 70cca1ec6cac0e..509b01485ee498 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -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* 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); diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 3cdea868da54dc..94e31393dd5b79 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -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 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 @@ -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) @@ -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; } @@ -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 @@ -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; @@ -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 @@ -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* blocks, - LoopHoistContext* hoistContext) +void Compiler::optHoistLoopBlocks(FlowGraphNaturalLoop* loop, + BitVecTraits* traits, + BitVec defExecuted, + LoopHoistContext* hoistContext) { class HoistVisitor : public GenTreeVisitor { @@ -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) { @@ -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) { } @@ -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 { @@ -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. @@ -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(), @@ -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;