diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0f979e47009383..d5b465d6dfa821 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2089,7 +2089,7 @@ class FlowGraphNaturalLoop GenTreeLclVarCommon* FindDef(unsigned lclNum); void MatchInit(NaturalLoopIterInfo* info, BasicBlock* initBlock, GenTree* init); - bool MatchLimit(NaturalLoopIterInfo* info, GenTree* test); + bool MatchLimit(unsigned iterVar, GenTree* test, NaturalLoopIterInfo* info); bool CheckLoopConditionBaseCase(BasicBlock* initBlock, NaturalLoopIterInfo* info); template static bool EvaluateRelop(T op1, T op2, genTreeOps oper); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 79ee2e97881edf..d475ff7ec1c2b4 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -4349,7 +4349,7 @@ FlowGraphNaturalLoops* FlowGraphNaturalLoops::Find(const FlowGraphDfsTree* dfsTr // Find the exit edges // - loop->VisitLoopBlocks([=](BasicBlock* loopBlock) { + loop->VisitLoopBlocksReversePostOrder([=](BasicBlock* loopBlock) { loopBlock->VisitRegularSuccs(comp, [=](BasicBlock* succBlock) { if (!loop->ContainsBlock(succBlock)) { @@ -4929,51 +4929,79 @@ bool FlowGraphNaturalLoop::AnalyzeIteration(NaturalLoopIterInfo* info) JITDUMP(" Preheader = " FMT_BB "\n", preheader->bbNum); - // TODO-Quirk: For backwards compatibility always try the lexically - // bottom-most block for the loop variable. - BasicBlock* bottom = GetLexicallyBottomMostBlock(); - JITDUMP(" Bottom = " FMT_BB "\n", bottom->bbNum); + BasicBlock* initBlock = nullptr; + GenTree* init = nullptr; + GenTree* test = nullptr; - BasicBlock* cond = bottom; - BasicBlock* initBlock = preheader; - GenTree* init; - GenTree* test; - if (!cond->KindIs(BBJ_COND) || - !comp->optExtractInitTestIncr(&initBlock, bottom, m_header, &init, &test, &info->IterTree)) + info->IterVar = BAD_VAR_NUM; + + for (FlowEdge* exitEdge : ExitEdges()) { - // TODO-CQ: Try all exit edges here to see if we can find an induction variable. - JITDUMP(" Could not extract induction variable from bottom\n"); - return false; - } + BasicBlock* cond = exitEdge->getSourceBlock(); + JITDUMP(" Checking exiting block " FMT_BB "\n", cond->bbNum); + if (!cond->KindIs(BBJ_COND)) + { + JITDUMP(" Not a BBJ_COND\n"); + continue; + } - assert(ContainsBlock(cond->GetTrueTarget()) != ContainsBlock(cond->GetFalseTarget())); + GenTree* iterTree = nullptr; + initBlock = preheader; + if (!comp->optExtractInitTestIncr(&initBlock, cond, m_header, &init, &test, &iterTree)) + { + JITDUMP(" Could not extract an IV\n"); + continue; + } - info->TestBlock = cond; - info->IterVar = comp->optIsLoopIncrTree(info->IterTree); - info->ExitedOnTrue = !ContainsBlock(cond->GetTrueTarget()); + unsigned iterVar = comp->optIsLoopIncrTree(iterTree); + assert(iterVar != BAD_VAR_NUM); + LclVarDsc* const iterVarDsc = comp->lvaGetDesc(iterVar); + // Bail on promoted case, otherwise we'd have to search the loop + // for both iterVar and its parent. + // TODO-CQ: Fix this + // + if (iterVarDsc->lvIsStructField) + { + JITDUMP(" iterVar V%02u is a promoted field\n", iterVar); + continue; + } - // TODO-CQ: Currently, since we only look at the lexically bottom most block all loops have - // ExitedOnTrue == false. Once we recognize more structures this can be true. - assert(!info->ExitedOnTrue); + // Bail on the potentially aliased case. + // + if (iterVarDsc->IsAddressExposed()) + { + JITDUMP(" iterVar V%02u is address exposed\n", iterVar); + continue; + } - assert(info->IterVar != BAD_VAR_NUM); - LclVarDsc* const iterVarDsc = comp->lvaGetDesc(info->IterVar); + if (!MatchLimit(iterVar, test, info)) + { + continue; + } - // Bail on promoted case, otherwise we'd have to search the loop - // for both iterVar and its parent. - // TODO-CQ: Fix this - // - if (iterVarDsc->lvIsStructField) - { - JITDUMP(" iterVar V%02u is a promoted field\n", info->IterVar); - return false; + bool result = VisitDefs([=](GenTreeLclVarCommon* def) { + if ((def->GetLclNum() != iterVar) || (def == iterTree)) + return true; + + JITDUMP(" Loop has extraneous def [%06u]\n", Compiler::dspTreeID(def)); + return false; + }); + + if (!result) + { + continue; + } + + info->TestBlock = cond; + info->IterVar = iterVar; + info->IterTree = iterTree; + info->ExitedOnTrue = !ContainsBlock(cond->GetTrueTarget()); + break; } - // Bail on the potentially aliased case. - // - if (iterVarDsc->IsAddressExposed()) + if (info->IterVar == BAD_VAR_NUM) { - JITDUMP(" iterVar V%02u is address exposed\n", info->IterVar); + JITDUMP(" Could not find any IV\n"); return false; } @@ -4988,11 +5016,6 @@ bool FlowGraphNaturalLoop::AnalyzeIteration(NaturalLoopIterInfo* info) Compiler::dspTreeID(info->IterTree)); } - if (!MatchLimit(info, test)) - { - return false; - } - MatchInit(info, initBlock, init); bool result = VisitDefs([=](GenTreeLclVarCommon* def) { @@ -5036,7 +5059,7 @@ bool FlowGraphNaturalLoop::AnalyzeIteration(NaturalLoopIterInfo* info) } #endif - return result; + return true; } //------------------------------------------------------------------------ @@ -5071,8 +5094,9 @@ void FlowGraphNaturalLoop::MatchInit(NaturalLoopIterInfo* info, BasicBlock* init // induction variable. // // Parameters: -// info - [in, out] Info structure to query and fill out -// test - Loop condition test +// iterVar - Local number of potential IV +// test - Loop condition test +// info - [out] Info structure to fill out with information about the limit // // Returns: // True if the loop condition was recognized and "info" was filled out. @@ -5081,8 +5105,13 @@ void FlowGraphNaturalLoop::MatchInit(NaturalLoopIterInfo* info, BasicBlock* init // Unlike the initialization, we do require that we are able to match the // loop condition. // -bool FlowGraphNaturalLoop::MatchLimit(NaturalLoopIterInfo* info, GenTree* test) +bool FlowGraphNaturalLoop::MatchLimit(unsigned iterVar, GenTree* test, NaturalLoopIterInfo* info) { + info->HasConstLimit = false; + info->HasSimdLimit = false; + info->HasArrayLengthLimit = false; + info->HasInvariantLocalLimit = false; + Compiler* comp = m_dfsTree->GetCompiler(); // Obtain the relop from the "test" tree. @@ -5106,12 +5135,12 @@ bool FlowGraphNaturalLoop::MatchLimit(NaturalLoopIterInfo* info, GenTree* test) GenTree* limitOp; // Make sure op1 or op2 is the iterVar. - if (opr1->OperIsScalarLocal() && (opr1->AsLclVarCommon()->GetLclNum() == info->IterVar)) + if (opr1->OperIsScalarLocal() && (opr1->AsLclVarCommon()->GetLclNum() == iterVar)) { iterOp = opr1; limitOp = opr2; } - else if (opr2->OperIsScalarLocal() && (opr2->AsLclVarCommon()->GetLclNum() == info->IterVar)) + else if (opr2->OperIsScalarLocal() && (opr2->AsLclVarCommon()->GetLclNum() == iterVar)) { iterOp = opr2; limitOp = opr1; @@ -5141,14 +5170,14 @@ bool FlowGraphNaturalLoop::MatchLimit(NaturalLoopIterInfo* info, GenTree* test) // if (comp->lvaGetDesc(limitOp->AsLclVarCommon())->IsAddressExposed()) { - JITDUMP(" Limit var V%02u is address exposed\n", limitOp->AsLclVarCommon()->GetLclNum()); + JITDUMP(" Limit var V%02u is address exposed\n", limitOp->AsLclVarCommon()->GetLclNum()); return false; } GenTreeLclVarCommon* def = FindDef(limitOp->AsLclVarCommon()->GetLclNum()); if (def != nullptr) { - JITDUMP(" Limit var V%02u modified by [%06u]\n", limitOp->AsLclVarCommon()->GetLclNum(), + JITDUMP(" Limit var V%02u modified by [%06u]\n", limitOp->AsLclVarCommon()->GetLclNum(), Compiler::dspTreeID(def)); return false; } @@ -5163,20 +5192,20 @@ bool FlowGraphNaturalLoop::MatchLimit(NaturalLoopIterInfo* info, GenTree* test) if (!array->OperIs(GT_LCL_VAR)) { - JITDUMP(" Array limit tree [%06u] not analyzable\n", Compiler::dspTreeID(limitOp)); + JITDUMP(" Array limit tree [%06u] not analyzable\n", Compiler::dspTreeID(limitOp)); return false; } if (comp->lvaGetDesc(array->AsLclVarCommon())->IsAddressExposed()) { - JITDUMP(" Array base local V%02u is address exposed\n", array->AsLclVarCommon()->GetLclNum()); + JITDUMP(" Array base local V%02u is address exposed\n", array->AsLclVarCommon()->GetLclNum()); return false; } GenTreeLclVarCommon* def = FindDef(array->AsLclVarCommon()->GetLclNum()); if (def != nullptr) { - JITDUMP(" Array limit var V%02u modified by [%06u]\n", array->AsLclVarCommon()->GetLclNum(), + JITDUMP(" Array limit var V%02u modified by [%06u]\n", array->AsLclVarCommon()->GetLclNum(), Compiler::dspTreeID(def)); return false; } @@ -5185,7 +5214,7 @@ bool FlowGraphNaturalLoop::MatchLimit(NaturalLoopIterInfo* info, GenTree* test) } else { - JITDUMP(" Loop limit tree [%06u] not analyzable\n", Compiler::dspTreeID(limitOp)); + JITDUMP(" Loop limit tree [%06u] not analyzable\n", Compiler::dspTreeID(limitOp)); return false; } diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 8e5124745e9e79..eb9941b5f569bf 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -393,11 +393,11 @@ bool Compiler::optIsLoopTestEvalIntoTemp(Statement* testStmt, Statement** newTes // Arguments: // pInitBlock - [IN/OUT] *pInitBlock is the loop head block on entry, and is set to the initBlock on exit, // if `**ppInit` is non-null. -// bottom - Loop bottom block -// top - Loop top block -// ppInit - The init stmt of the loop if found. -// ppTest - The test stmt of the loop if found. -// ppIncr - The incr stmt of the loop if found. +// cond - A BBJ_COND block that exits the loop +// header - Loop header block +// ppInit - [out] The init stmt of the loop if found. +// ppTest - [out] The test stmt of the loop if found. +// ppIncr - [out] The incr stmt of the loop if found. // // Return Value: // The results are put in "ppInit", "ppTest" and "ppIncr" if the method @@ -406,25 +406,15 @@ bool Compiler::optIsLoopTestEvalIntoTemp(Statement* testStmt, Statement** newTes // to nullptr. Return value will never be false if `init` is not found. // // Operation: -// Check if the "test" stmt is last stmt in the loop "bottom". Try to find the "incr" stmt. -// Check previous stmt of "test" to get the "incr" stmt. If it is not found it could be a loop of the -// below form. -// -// +-------<-----------------<-----------+ -// | | -// v | -// BBinit(head) -> BBcond(top) -> BBLoopBody(bottom) ---^ -// -// Check if the "incr" tree is present in the loop "top" node as the last stmt. -// Also check if the "test" tree is assigned to a tmp node and the tmp is used -// in the jtrue condition. +// Check if the "test" stmt is last stmt in an exiting BBJ_COND block of the loop. Try to find the "incr" stmt. +// Check previous stmt of "test" to get the "incr" stmt. // // Note: // This method just retrieves what it thinks is the "test" node, // the callers are expected to verify that "iterVar" is used in the test. // bool Compiler::optExtractInitTestIncr( - BasicBlock** pInitBlock, BasicBlock* bottom, BasicBlock* top, GenTree** ppInit, GenTree** ppTest, GenTree** ppIncr) + BasicBlock** pInitBlock, BasicBlock* cond, BasicBlock* header, GenTree** ppInit, GenTree** ppTest, GenTree** ppIncr) { assert(pInitBlock != nullptr); assert(ppInit != nullptr); @@ -433,8 +423,8 @@ bool Compiler::optExtractInitTestIncr( // Check if last two statements in the loop body are the increment of the iterator // and the loop termination test. - noway_assert(bottom->bbStmtList != nullptr); - Statement* testStmt = bottom->lastStmt(); + noway_assert(cond->bbStmtList != nullptr); + Statement* testStmt = cond->lastStmt(); noway_assert(testStmt != nullptr && testStmt->GetNextStmt() == nullptr); Statement* newTestStmt; @@ -444,7 +434,7 @@ bool Compiler::optExtractInitTestIncr( } // Check if we have the incr stmt before the test stmt, if we don't, - // check if incr is part of the loop "top". + // check if incr is part of the loop "header". Statement* incrStmt = testStmt->GetPrevStmt(); // If we've added profile instrumentation, we may need to skip past a BB counter update. @@ -472,7 +462,7 @@ bool Compiler::optExtractInitTestIncr( // If we are rebuilding the loops, we would already have the pre-header block introduced // the first time, which might be empty if no hoisting has yet occurred. In this case, look a // little harder for the possible loop initialization statement. - if (initBlock->KindIs(BBJ_ALWAYS) && initBlock->TargetIs(top) && (initBlock->countOfInEdges() == 1) && + if (initBlock->KindIs(BBJ_ALWAYS) && initBlock->TargetIs(header) && (initBlock->countOfInEdges() == 1) && !initBlock->IsFirst() && initBlock->Prev()->bbFallsThrough()) { initBlock = initBlock->Prev(); @@ -1377,6 +1367,19 @@ bool Compiler::optTryUnrollLoop(FlowGraphNaturalLoop* loop, bool* changedIR) return false; } + // The loop test must be both an exit and a backedge. + // FlowGraphNaturalLoop::AnalyzeIteration ensures it is an exit but we must + // make sure it is a backedge so that we can legally redirect it to the + // next iteration. If it isn't a backedge then redirecting it would skip + // all code between the loop test and the backedge. + assert(loop->ContainsBlock(iterInfo.TestBlock->GetTrueTarget()) != + loop->ContainsBlock(iterInfo.TestBlock->GetFalseTarget())); + if (!iterInfo.TestBlock->TrueTargetIs(loop->GetHeader()) && !iterInfo.TestBlock->FalseTargetIs(loop->GetHeader())) + { + JITDUMP("Failed to unroll loop " FMT_LP ": test block is not a backedge\n", loop->GetIndex()); + return false; + } + // Get the loop data: // - initial constant // - limit constant