diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 68ddba5f2415f..e7822dc591597 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7646,6 +7646,8 @@ class Compiler FlowGraphNaturalLoop* loop, BasicBlock* exiting, LoopLocalOccurrences* loopLocals); + bool optCanAndShouldChangeExitTest(GenTree* cond, bool dump); + bool optPrimaryIVHasNonLoopUses(unsigned lclNum, FlowGraphNaturalLoop* loop, 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 52258326ef5cb..fba8febf0fe7c 100644 --- a/src/coreclr/jit/inductionvariableopts.cpp +++ b/src/coreclr/jit/inductionvariableopts.cpp @@ -992,6 +992,9 @@ bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevConte BasicBlock* exiting, LoopLocalOccurrences* loopLocals) { + // Note: keep the heuristics here in sync with + // `StrengthReductionContext::IsUseExpectedToBeRemoved`. + assert(exiting->KindIs(BBJ_COND)); Statement* jtrueStmt = exiting->lastStmt(); @@ -999,21 +1002,8 @@ bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevConte assert(jtrue->OperIs(GT_JTRUE)); GenTree* cond = jtrue->gtGetOp1(); - if ((jtrue->gtFlags & GTF_SIDE_EFFECT) != 0) + if (!optCanAndShouldChangeExitTest(cond, /* dump */ true)) { - // If the IV is used as part of the side effect then we can't - // transform; otherwise we could. TODO-CQ: Make this determination and - // extract side effects from the jtrue to make this work. - JITDUMP(" No; exit node has side effects\n"); - return false; - } - - bool checkProfitability = !compStressCompile(STRESS_DOWNWARDS_COUNTED_LOOPS, 50); - - if (checkProfitability && cond->OperIsCompare() && - (cond->gtGetOp1()->IsIntegralConst(0) || cond->gtGetOp2()->IsIntegralConst(0))) - { - JITDUMP(" No; operand of condition [%06u] is already 0\n", dspTreeID(cond)); return false; } @@ -1029,34 +1019,10 @@ bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevConte break; } - unsigned candidateLclNum = stmt->GetRootNode()->AsLclVarCommon()->GetLclNum(); - LclVarDsc* candidateVarDsc = lvaGetDesc(candidateLclNum); - if (candidateVarDsc->lvIsStructField && loopLocals->HasAnyOccurrences(loop, candidateVarDsc->lvParentLcl)) - { - continue; - } + unsigned candidateLclNum = stmt->GetRootNode()->AsLclVarCommon()->GetLclNum(); - if (candidateVarDsc->lvDoNotEnregister) + if (optPrimaryIVHasNonLoopUses(candidateLclNum, loop, loopLocals)) { - // This filters out locals that may be live into exceptional exits. - continue; - } - - BasicBlockVisit visitResult = loop->VisitRegularExitBlocks([=](BasicBlock* block) { - if (VarSetOps::IsMember(this, block->bbLiveIn, candidateVarDsc->lvVarIndex)) - { - return BasicBlockVisit::Abort; - } - - return BasicBlockVisit::Continue; - }); - - if (visitResult == BasicBlockVisit::Abort) - { - // Live into an exit. - // TODO-CQ: In some cases it may be profitable to materialize the final value after the loop. - // This requires analysis on whether the required expressions are available there - // (and whether it doesn't extend their lifetimes too much). continue; } @@ -1065,7 +1031,8 @@ bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevConte if (stmt == jtrueStmt) { hasUseInTest = true; - // Use is inside the loop test that has no side effects (as we checked above), can remove + // Use is inside the loop test that we know we can change (from + // calling optCanAndShouldChangeExitTest above) return true; } @@ -1110,15 +1077,13 @@ bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevConte removableLocals.Push(candidateLclNum); } + bool checkProfitability = !compStressCompile(STRESS_DOWNWARDS_COUNTED_LOOPS, 50); if (checkProfitability && (removableLocals.Height() <= 0)) { JITDUMP(" Found no potentially removable locals when making this loop downwards counted\n"); 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); - if (loop->MayExecuteBlockMultipleTimesPerIteration(exiting)) { JITDUMP(" Exiting block may be executed multiple times per iteration; cannot place decrement in it\n"); @@ -1210,20 +1175,111 @@ bool Compiler::optMakeExitTestDownwardsCounted(ScalarEvolutionContext& scevConte return true; } +//------------------------------------------------------------------------ +// optCanAndShouldChangeExitTest: +// Check if the exit test can be rephrased to a downwards counted exit test +// (being compared to zero). +// +// Parameters: +// cond - The exit test +// dump - Whether to JITDUMP the reason for the decisions +// +// Returns: +// True if the exit test can be changed. +// +bool Compiler::optCanAndShouldChangeExitTest(GenTree* cond, bool dump) +{ + if ((cond->gtFlags & GTF_SIDE_EFFECT) != 0) + { + // This would be possible if the IV use is not part of the side effect, + // in which case we could extract them. However, these cases turn out + // to be never analyzable for us even if we tried to do that, so just + // do the easy check here. + if (dump) + { + JITDUMP(" No; exit node has side effects\n"); + } + + return false; + } + + bool checkProfitability = !compStressCompile(STRESS_DOWNWARDS_COUNTED_LOOPS, 50); + + if (checkProfitability && cond->OperIsCompare() && + (cond->gtGetOp1()->IsIntegralConst(0) || cond->gtGetOp2()->IsIntegralConst(0))) + { + if (dump) + { + JITDUMP(" No; operand of condition [%06u] is already 0\n", dspTreeID(cond)); + } + + return false; + } + + return true; +} + +//------------------------------------------------------------------------ +// optPrimaryIVHasNonLoopUses: +// Check if a primary IV may have uses of the primary IV that we do not +// reason about. +// +// Parameters: +// lclNum - The primary IV +// loop - The loop +// loopLocals - Data structure tracking local uses +// +// Returns: +// True if the primary IV may have non-loop uses (or if it is a field with +// uses of the parent struct). +// +bool Compiler::optPrimaryIVHasNonLoopUses(unsigned lclNum, FlowGraphNaturalLoop* loop, LoopLocalOccurrences* loopLocals) +{ + LclVarDsc* varDsc = lvaGetDesc(lclNum); + if (varDsc->lvIsStructField && loopLocals->HasAnyOccurrences(loop, varDsc->lvParentLcl)) + { + return true; + } + + if (varDsc->lvDoNotEnregister) + { + // This filters out locals that may be live into exceptional exits. + return true; + } + + BasicBlockVisit visitResult = loop->VisitRegularExitBlocks([=](BasicBlock* block) { + if (VarSetOps::IsMember(this, block->bbLiveIn, varDsc->lvVarIndex)) + { + return BasicBlockVisit::Abort; + } + + return BasicBlockVisit::Continue; + }); + + if (visitResult == BasicBlockVisit::Abort) + { + // Live into an exit. + // TODO-CQ: In some cases it may be profitable to materialize the final value after the loop. + // This requires analysis on whether the required expressions are available there + // (and whether it doesn't extend their lifetimes too much). + return true; + } + + return false; +} + struct CursorInfo { BasicBlock* Block; Statement* Stmt; GenTree* Tree; ScevAddRec* IV; - bool IsInsideExitTest = false; - CursorInfo(BasicBlock* block, Statement* stmt, GenTree* tree, ScevAddRec* iv, bool isInsideExitTest) + CursorInfo(BasicBlock* block, Statement* stmt, GenTree* tree, ScevAddRec* iv) : Block(block) , Stmt(stmt) , Tree(tree) , IV(iv) - , IsInsideExitTest(isInsideExitTest) { } }; @@ -1242,6 +1298,7 @@ class StrengthReductionContext void InitializeSimplificationAssumptions(); bool InitializeCursors(GenTreeLclVarCommon* primaryIVLcl, ScevAddRec* primaryIV); + bool IsUseExpectedToBeRemoved(BasicBlock* block, Statement* stmt, GenTreeLclVarCommon* tree); void AdvanceCursors(ArrayStack* cursors, ArrayStack* nextCursors); bool CheckAdvancedCursors(ArrayStack* cursors, int derivedLevel, ScevAddRec** nextIV); bool StaysWithinManagedObject(ArrayStack* cursors, ScevAddRec* addRec); @@ -1337,6 +1394,13 @@ bool StrengthReductionContext::TryStrengthReduce() continue; } + if (m_comp->optPrimaryIVHasNonLoopUses(primaryIVLcl->GetLclNum(), m_loop, &m_loopLocals)) + { + // We won't be able to remove this primary IV + JITDUMP(" Has non-loop uses\n"); + continue; + } + ScevAddRec* primaryIV = static_cast(candidate); if (!InitializeCursors(primaryIVLcl, primaryIV)) @@ -1470,16 +1534,11 @@ bool StrengthReductionContext::InitializeCursors(GenTreeLclVarCommon* primaryIVL m_cursors2.Reset(); auto visitor = [=](BasicBlock* block, Statement* stmt, GenTreeLclVarCommon* tree) { - if (stmt->GetRootNode()->OperIsLocalStore()) + if (IsUseExpectedToBeRemoved(block, stmt, tree)) { - GenTreeLclVarCommon* lcl = stmt->GetRootNode()->AsLclVarCommon(); - if ((lcl->GetLclNum() == primaryIVLcl->GetLclNum()) && ((lcl->Data()->gtFlags & GTF_SIDE_EFFECT) == 0)) - { - // Store to the primary IV without side effects; if we end - // up strength reducing, then this store is expected to be - // removed by making the loop downwards counted. - return true; - } + // If we do strength reduction we expect to be able to remove this + // use; do not create a cursor for it. + return true; } if (!tree->OperIs(GT_LCL_VAR)) @@ -1487,16 +1546,12 @@ bool StrengthReductionContext::InitializeCursors(GenTreeLclVarCommon* primaryIVL return false; } - bool isInsideExitTest = - block->KindIs(BBJ_COND) && (stmt == block->lastStmt()) && - (!m_loop->ContainsBlock(block->GetTrueTarget()) || !m_loop->ContainsBlock(block->GetFalseTarget())); - if (tree->GetSsaNum() != primaryIVLcl->GetSsaNum()) { // Most likely a post-incremented use of the primary IV; we // could replace these as well, but currently we only handle // the cases where we expect the use to be removed. - return isInsideExitTest; + return false; } Scev* iv = m_scevContext.Analyze(block, tree); @@ -1513,8 +1568,8 @@ bool StrengthReductionContext::InitializeCursors(GenTreeLclVarCommon* primaryIVL // as the primary IV. assert(Scev::Equals(m_scevContext.Simplify(iv, m_simplAssumptions), primaryIV)); - m_cursors1.Emplace(block, stmt, tree, primaryIV, isInsideExitTest); - m_cursors2.Emplace(block, stmt, tree, primaryIV, isInsideExitTest); + m_cursors1.Emplace(block, stmt, tree, primaryIV); + m_cursors2.Emplace(block, stmt, tree, primaryIV); return true; }; @@ -1532,8 +1587,7 @@ bool StrengthReductionContext::InitializeCursors(GenTreeLclVarCommon* primaryIVL for (int i = 0; i < m_cursors1.Height(); i++) { CursorInfo& cursor = m_cursors1.BottomRef(i); - printf(" [%d] [%06u]%s: ", i, Compiler::dspTreeID(cursor.Tree), - cursor.IsInsideExitTest ? " (in-test)" : ""); + printf(" [%d] [%06u]: ", i, Compiler::dspTreeID(cursor.Tree)); cursor.IV->Dump(m_comp); printf("\n"); } @@ -1543,6 +1597,83 @@ bool StrengthReductionContext::InitializeCursors(GenTreeLclVarCommon* primaryIVL return true; } +//------------------------------------------------------------------------ +// IsUseExpectedToBeRemoved: Check if a use of a primary IV is expected to be +// removed if we strength reduce other uses of the primary IV. +// +// Parameters: +// block - Block containing the use +// stmt - Statement containing the use +// tree - Actual use of the primary IV +// +// Returns: +// True if the use is expected to be removable. +// +bool StrengthReductionContext::IsUseExpectedToBeRemoved(BasicBlock* block, Statement* stmt, GenTreeLclVarCommon* tree) +{ + unsigned primaryIVLclNum = tree->GetLclNum(); + if (stmt->GetRootNode()->OperIsLocalStore()) + { + GenTreeLclVarCommon* lcl = stmt->GetRootNode()->AsLclVarCommon(); + if ((lcl->GetLclNum() == primaryIVLclNum) && ((lcl->Data()->gtFlags & GTF_SIDE_EFFECT) == 0)) + { + // Store to the primary IV without side effects; if we end + // up strength reducing, then this store is expected to be + // removed by making the loop downwards counted. + return true; + } + + return false; + } + + bool isInsideExitTest = + block->KindIs(BBJ_COND) && (stmt == block->lastStmt()) && + (!m_loop->ContainsBlock(block->GetTrueTarget()) || !m_loop->ContainsBlock(block->GetFalseTarget())); + + if (isInsideExitTest) + { + // The downwards loop transformation may be able to remove this use. + // Here we duplicate some of the logic from + // optMakeExitTestDownwardsCounted to predict whether that will happen. + GenTree* jtrue = block->lastStmt()->GetRootNode(); + GenTree* cond = jtrue->gtGetOp1(); + + // Is the exit test changeable? + if (!m_comp->optCanAndShouldChangeExitTest(cond, /* dump */ false)) + { + return false; + } + + // Does the exit dominate all backedges such that we can place IV + // updates before it? + for (FlowEdge* edge : m_loop->BackEdges()) + { + if (!m_comp->m_domTree->Dominates(block, edge->getSourceBlock())) + { + return false; + } + } + + // Will the exit only run once per iteration? + if (m_loop->MayExecuteBlockMultipleTimesPerIteration(block)) + { + return false; + } + + // Can we compute the trip count from the exit test? + if (m_scevContext.ComputeExitNotTakenCount(block) == nullptr) + { + return false; + } + + // If all of those things are true, we are most likely going to be able + // to convert the exit test to a down-counting one after we have removed the other uses of the IV. + return true; + } + + return false; +} + //------------------------------------------------------------------------ // AdvanceCursors: Advance cursors stored in "cursors" and store the advanced // result in "nextCursors". @@ -1560,8 +1691,7 @@ void StrengthReductionContext::AdvanceCursors(ArrayStack* cursors, A CursorInfo& cursor = cursors->BottomRef(i); CursorInfo& nextCursor = nextCursors->BottomRef(i); - assert((nextCursor.Block == cursor.Block) && (nextCursor.Stmt == cursor.Stmt) && - (nextCursor.IsInsideExitTest == cursor.IsInsideExitTest)); + assert((nextCursor.Block == cursor.Block) && (nextCursor.Stmt == cursor.Stmt)); nextCursor.Tree = cursor.Tree; do @@ -1604,8 +1734,7 @@ void StrengthReductionContext::AdvanceCursors(ArrayStack* cursors, A for (int i = 0; i < nextCursors->Height(); i++) { CursorInfo& nextCursor = nextCursors->BottomRef(i); - printf(" [%d] [%06u]%s: ", i, nextCursor.Tree == nullptr ? 0 : Compiler::dspTreeID(nextCursor.Tree), - nextCursor.IsInsideExitTest ? " (in-test)" : ""); + printf(" [%d] [%06u]%s: ", i, nextCursor.Tree == nullptr ? 0 : Compiler::dspTreeID(nextCursor.Tree)); if (nextCursor.IV == nullptr) { printf(""); @@ -1649,13 +1778,6 @@ bool StrengthReductionContext::CheckAdvancedCursors(ArrayStack* curs { CursorInfo& cursor = cursors->BottomRef(i); - // Uses inside the exit test only need to opportunistically - // match. We check these after. - if (cursor.IsInsideExitTest) - { - continue; - } - if ((cursor.IV != nullptr) && ((*nextIV == nullptr) || Scev::Equals(cursor.IV, *nextIV))) { *nextIV = cursor.IV; @@ -1666,50 +1788,6 @@ bool StrengthReductionContext::CheckAdvancedCursors(ArrayStack* curs return false; } - // Now check all exit test uses. - for (int i = 0; i < cursors->Height(); i++) - { - CursorInfo& cursor = cursors->BottomRef(i); - - if (!cursor.IsInsideExitTest) - { - continue; - } - - if ((cursor.IV != nullptr) && ((*nextIV == nullptr) || Scev::Equals(cursor.IV, *nextIV))) - { - *nextIV = cursor.IV; - continue; - } - - // Use inside exit test does not match. - if (derivedLevel <= 1) - { - // We weren't able to advance the match in the exit test at all; in - // this situation we expect the downwards optimization to be able - // to remove the use of the primary IV, so this is ok. Remove the - // cursor pointing to the use inside the test. - JITDUMP(" [%d] does not match, but is inside loop test; ignoring mismatch and removing cursor\n", i); - - std::swap(m_cursors1.BottomRef(i), m_cursors1.TopRef(0)); - std::swap(m_cursors2.BottomRef(i), m_cursors2.TopRef(0)); - - m_cursors1.Pop(); - m_cursors2.Pop(); - - i--; - } - else - { - // We already found a derived IV in the exit test that matches, so - // stop here and allow the replacement to replace the uses of the - // current derived IV, including the one in the exit test - // statement. - JITDUMP(" [%d] does not match; will not advance\n", i); - return false; - } - } - return *nextIV != nullptr; }