From f2ed9204c916b2dd7c9f69cb547bdab8d516f435 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Sat, 5 Mar 2022 18:28:14 -0800 Subject: [PATCH] Remove loop cloning var initialization condition Assume that any pre-existing initialization is ok. Check it against zero if necessary. Const inits remain as before. Lots of diffs due to more cloning for cases of `for (i = expression...` where `expression` is not just a constant or local var. --- src/coreclr/jit/compiler.cpp | 2 - src/coreclr/jit/compiler.h | 45 ++++---- src/coreclr/jit/fgdiagnostic.cpp | 12 +-- src/coreclr/jit/loopcloning.cpp | 16 ++- src/coreclr/jit/loopcloning.h | 2 +- src/coreclr/jit/optimizer.cpp | 172 +++++++++++++------------------ 6 files changed, 99 insertions(+), 150 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index a2676c57c78bb..e348db612e52e 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -319,7 +319,6 @@ unsigned totalLoopCount; // counts the total number of natural loops unsigned totalUnnatLoopCount; // counts the total number of (not-necessarily natural) loops unsigned totalUnnatLoopOverflows; // # of methods that identified more unnatural loops than we can represent unsigned iterLoopCount; // counts the # of loops with an iterator (for like) -unsigned simpleTestLoopCount; // counts the # of loops with an iterator and a simple loop condition (iter < const) unsigned constIterLoopCount; // counts the # of loops with a constant iterator (for like) bool hasMethodLoops; // flag to keep track if we already counted a method as having loops unsigned loopsThisMethod; // counts the number of loops in the current method @@ -1556,7 +1555,6 @@ void Compiler::compShutdown() fprintf(fout, "Total number of 'unnatural' loops is %5u\n", totalUnnatLoopCount); fprintf(fout, "# of methods overflowing unnat loop limit is %5u\n", totalUnnatLoopOverflows); fprintf(fout, "Total number of loops with an iterator is %5u\n", iterLoopCount); - fprintf(fout, "Total number of loops with a simple iterator is %5u\n", simpleTestLoopCount); fprintf(fout, "Total number of loops with a constant iterator is %5u\n", constIterLoopCount); fprintf(fout, "--------------------------------------------------\n"); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 44651689b3bc2..94e7a166acae9 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2498,13 +2498,13 @@ enum LoopFlags : unsigned short // LPFLG_UNUSED = 0x0001, // LPFLG_UNUSED = 0x0002, - LPFLG_ITER = 0x0004, // loop of form: for (i = icon or lclVar; test_condition(); i++) + LPFLG_ITER = 0x0004, // loop of form: for (i = icon or expression; test_condition(); i++) // LPFLG_UNUSED = 0x0008, LPFLG_CONTAINS_CALL = 0x0010, // If executing the loop body *may* execute a call - LPFLG_VAR_INIT = 0x0020, // iterator is initialized with a local var (var # found in lpVarInit) - LPFLG_CONST_INIT = 0x0040, // iterator is initialized with a constant (found in lpConstInit) - LPFLG_SIMD_LIMIT = 0x0080, // iterator is compared with vector element count (found in lpConstLimit) + // LPFLG_UNUSED = 0x0020, + LPFLG_CONST_INIT = 0x0040, // iterator is initialized with a constant (found in lpConstInit) + LPFLG_SIMD_LIMIT = 0x0080, // iterator is compared with vector element count (found in lpConstLimit) LPFLG_VAR_LIMIT = 0x0100, // iterator is compared with a local var (var # found in lpVarLimit) LPFLG_CONST_LIMIT = 0x0200, // iterator is compared with a constant (found in lpConstLimit) @@ -6929,16 +6929,11 @@ class Compiler var_types lpIterOperType() const; // For overflow instructions - // Set to the block where we found the initialization for LPFLG_CONST_INIT or LPFLG_VAR_INIT loops. + // Set to the block where we found the initialization for LPFLG_CONST_INIT loops. // Initially, this will be 'head', but 'head' might change if we insert a loop pre-header block. BasicBlock* lpInitBlock; - union { - int lpConstInit; // initial constant value of iterator - // : Valid if LPFLG_CONST_INIT - unsigned lpVarInit; // initial local var number to which we initialize the iterator - // : Valid if LPFLG_VAR_INIT - }; + int lpConstInit; // initial constant value of iterator : Valid if LPFLG_CONST_INIT // The following is for LPFLG_ITER loops only (i.e. the loop condition is "i RELOP const or var") @@ -12068,21 +12063,19 @@ extern Histogram bbOneBBSizeTable; #if COUNT_LOOPS -extern unsigned totalLoopMethods; // counts the total number of methods that have natural loops -extern unsigned maxLoopsPerMethod; // counts the maximum number of loops a method has -extern unsigned totalLoopOverflows; // # of methods that identified more loops than we can represent -extern unsigned totalLoopCount; // counts the total number of natural loops -extern unsigned totalUnnatLoopCount; // counts the total number of (not-necessarily natural) loops -extern unsigned totalUnnatLoopOverflows; // # of methods that identified more unnatural loops than we can represent -extern unsigned iterLoopCount; // counts the # of loops with an iterator (for like) -extern unsigned simpleTestLoopCount; // counts the # of loops with an iterator and a simple loop condition (iter < - // const) -extern unsigned constIterLoopCount; // counts the # of loops with a constant iterator (for like) -extern bool hasMethodLoops; // flag to keep track if we already counted a method as having loops -extern unsigned loopsThisMethod; // counts the number of loops in the current method -extern bool loopOverflowThisMethod; // True if we exceeded the max # of loops in the method. -extern Histogram loopCountTable; // Histogram of loop counts -extern Histogram loopExitCountTable; // Histogram of loop exit counts +extern unsigned totalLoopMethods; // counts the total number of methods that have natural loops +extern unsigned maxLoopsPerMethod; // counts the maximum number of loops a method has +extern unsigned totalLoopOverflows; // # of methods that identified more loops than we can represent +extern unsigned totalLoopCount; // counts the total number of natural loops +extern unsigned totalUnnatLoopCount; // counts the total number of (not-necessarily natural) loops +extern unsigned totalUnnatLoopOverflows; // # of methods that identified more unnatural loops than we can represent +extern unsigned iterLoopCount; // counts the # of loops with an iterator (for like) +extern unsigned constIterLoopCount; // counts the # of loops with a constant iterator (for like) +extern bool hasMethodLoops; // flag to keep track if we already counted a method as having loops +extern unsigned loopsThisMethod; // counts the number of loops in the current method +extern bool loopOverflowThisMethod; // True if we exceeded the max # of loops in the method. +extern Histogram loopCountTable; // Histogram of loop counts +extern Histogram loopExitCountTable; // Histogram of loop exit counts #endif // COUNT_LOOPS diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 13fd1384b15bb..0e4f7ec3ec487 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -3781,13 +3781,10 @@ void Compiler::fgDebugCheckLoopTable() } // Check the flags. - // Note that the various init/limit flags are only used when LPFLG_ITER is set, but they are set first, + // Note that the various limit flags are only used when LPFLG_ITER is set, but they are set first, // separately, and only if everything works out is LPFLG_ITER set. If LPFLG_ITER is NOT set, the // individual flags are not un-set (arguably, they should be). - // Only one of the `init` flags can be set. - assert(genCountBits((unsigned)(loop.lpFlags & (LPFLG_VAR_INIT | LPFLG_CONST_INIT))) <= 1); - // Only one of the `limit` flags can be set. (Note that LPFLG_SIMD_LIMIT is a "sub-flag" that can be // set when LPFLG_CONST_LIMIT is set.) assert(genCountBits((unsigned)(loop.lpFlags & (LPFLG_VAR_LIMIT | LPFLG_CONST_LIMIT | LPFLG_ARRLEN_LIMIT))) <= @@ -3799,14 +3796,9 @@ void Compiler::fgDebugCheckLoopTable() assert(loop.lpFlags & LPFLG_CONST_LIMIT); } - if (loop.lpFlags & (LPFLG_CONST_INIT | LPFLG_VAR_INIT)) + if (loop.lpFlags & LPFLG_CONST_INIT) { assert(loop.lpInitBlock != nullptr); - - if (loop.lpFlags & LPFLG_VAR_INIT) - { - assert(loop.lpVarInit < lvaCount); - } } if (loop.lpFlags & LPFLG_ITER) diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index b56ace060c685..ab68249fd4cc0 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1038,17 +1038,18 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext if (loop->lpFlags & LPFLG_CONST_INIT) { // Only allowing non-negative const init at this time. - // REVIEW: why? + // This is because the variable initialized with this constant will be used as an array index, + // and array indices must be non-negative. if (loop->lpConstInit < 0) { JITDUMP("> Init %d is invalid\n", loop->lpConstInit); return false; } } - else if (loop->lpFlags & LPFLG_VAR_INIT) + else { - // initVar >= 0 - const unsigned initLcl = loop->lpVarInit; + // iterVar >= 0 + const unsigned initLcl = loop->lpIterVar(); if (!genActualTypeIsInt(lvaGetDesc(initLcl))) { JITDUMP("> Init var V%02u not compatible with TYP_INT\n", initLcl); @@ -1059,11 +1060,6 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext LC_Expr(LC_Ident(0, LC_Ident::Const))); context->EnsureConditions(loopNum)->Push(geZero); } - else - { - JITDUMP("> Not variable init\n"); - return false; - } // Limit Conditions LC_Ident ident; @@ -1096,7 +1092,7 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext ArrIndex* index = new (getAllocator(CMK_LoopClone)) ArrIndex(getAllocator(CMK_LoopClone)); if (!loop->lpArrLenLimit(this, index)) { - JITDUMP("> ArrLen not matching"); + JITDUMP("> ArrLen not matching\n"); return false; } ident = LC_Ident(LC_Array(LC_Array::Jagged, index, LC_Array::ArrLen)); diff --git a/src/coreclr/jit/loopcloning.h b/src/coreclr/jit/loopcloning.h index b16c36be4dc90..0cd3b1a917329 100644 --- a/src/coreclr/jit/loopcloning.h +++ b/src/coreclr/jit/loopcloning.h @@ -138,7 +138,7 @@ exception occurs. 1. Loop detection has completed and the loop table is populated. 2. The loops that will be considered are the ones with the LPFLG_ITER flag: - "for (i = icon or lclVar; test_condition(); i++)" + "for ( ; test_condition(); i++)" Limitations diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 4b9b12c70e6f2..a45789bd2c583 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -595,12 +595,12 @@ void Compiler::optClearLoopIterInfo() for (unsigned lnum = 0; lnum < optLoopCount; lnum++) { LoopDsc& loop = optLoopTable[lnum]; - loop.lpFlags &= ~(LPFLG_ITER | LPFLG_VAR_INIT | LPFLG_CONST_INIT | LPFLG_SIMD_LIMIT | LPFLG_VAR_LIMIT | - LPFLG_CONST_LIMIT | LPFLG_ARRLEN_LIMIT); + loop.lpFlags &= ~(LPFLG_ITER | LPFLG_CONST_INIT | LPFLG_SIMD_LIMIT | LPFLG_VAR_LIMIT | LPFLG_CONST_LIMIT | + LPFLG_ARRLEN_LIMIT); loop.lpIterTree = nullptr; loop.lpInitBlock = nullptr; - loop.lpConstInit = -1; // union with loop.lpVarInit + loop.lpConstInit = -1; loop.lpTestTree = nullptr; } } @@ -670,12 +670,8 @@ void Compiler::optPrintLoopInfo(const LoopDsc* loop, bool printVerbose /* = fals { printf(" from %d", loop->lpConstInit); } - if (loop->lpFlags & LPFLG_VAR_INIT) - { - printf(" from V%02u", loop->lpVarInit); - } - if (loop->lpFlags & (LPFLG_CONST_INIT | LPFLG_VAR_INIT)) + if (loop->lpFlags & LPFLG_CONST_INIT) { if (loop->lpInitBlock != loop->lpHead) { @@ -776,22 +772,28 @@ void Compiler::optPrintLoopTable() //------------------------------------------------------------------------ // optPopulateInitInfo: Populate loop init info in the loop table. +// We assume the iteration variable is initialized already and check appropriately. +// This only checks for the special case of a constant initialization. // // Arguments: // loopInd - loop index // initBlock - block in which the initialization lives. -// init - the tree that is supposed to initialize the loop iterator. +// init - the tree that is supposed to initialize the loop iterator. Might be nullptr. // iterVar - loop iteration variable. // // Return Value: -// "false" if the loop table could not be populated with the loop iterVar init info. +// "true" if a constant initializer was found. // // Operation: -// The 'init' tree is checked if its lhs is a local and rhs is either -// a const or a local. +// The 'init' tree is checked if its lhs is a local and rhs is a const. // bool Compiler::optPopulateInitInfo(unsigned loopInd, BasicBlock* initBlock, GenTree* init, unsigned iterVar) { + if (init == nullptr) + { + return false; + } + // Operator should be = if (init->gtOper != GT_ASG) { @@ -808,22 +810,27 @@ bool Compiler::optPopulateInitInfo(unsigned loopInd, BasicBlock* initBlock, GenT // RHS can be constant or local var. // TODO-CQ: CLONE: Add arr length for descending loops. - if (rhs->gtOper == GT_CNS_INT && rhs->TypeGet() == TYP_INT) + if (rhs->gtOper != GT_CNS_INT || rhs->TypeGet() != TYP_INT) { - optLoopTable[loopInd].lpFlags |= LPFLG_CONST_INIT; - optLoopTable[loopInd].lpConstInit = (int)rhs->AsIntCon()->gtIconVal; - optLoopTable[loopInd].lpInitBlock = initBlock; - } - else if (rhs->gtOper == GT_LCL_VAR) - { - optLoopTable[loopInd].lpFlags |= LPFLG_VAR_INIT; - optLoopTable[loopInd].lpVarInit = rhs->AsLclVarCommon()->GetLclNum(); - optLoopTable[loopInd].lpInitBlock = initBlock; + return false; } - else + + // We found an initializer in the `head` block. For this to be used, we need to make sure the + // "iterVar" initialization is never skipped. That is, every pred of ENTRY other than HEAD is in the loop. + for (BasicBlock* const predBlock : optLoopTable[loopInd].lpEntry->PredBlocks()) { - return false; + if ((predBlock != initBlock) && !optLoopTable[loopInd].lpContains(predBlock)) + { + JITDUMP(FMT_LP ": initialization not guaranteed through `head` block; ignore constant initializer\n", + loopInd); + return false; + } } + + optLoopTable[loopInd].lpFlags |= LPFLG_CONST_INIT; + optLoopTable[loopInd].lpConstInit = (int)rhs->AsIntCon()->gtIconVal; + optLoopTable[loopInd].lpInitBlock = initBlock; + return true; } @@ -1083,6 +1090,8 @@ bool Compiler::optIsLoopTestEvalIntoTemp(Statement* testStmt, Statement** newTes // Return Value: // The results are put in "ppInit", "ppTest" and "ppIncr" if the method // returns true. Returns false if the information can't be extracted. +// Extracting the `init` is optional; if one is not found, *ppInit is set +// 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". If found good, @@ -1149,39 +1158,42 @@ bool Compiler::optExtractInitTestIncr( // Find the last statement in the loop pre-header which we expect to be the initialization of // the loop iterator. Statement* phdrStmt = head->firstStmt(); - if (phdrStmt == nullptr) + if (phdrStmt != nullptr) { - return false; - } - - Statement* initStmt = phdrStmt->GetPrevStmt(); - noway_assert(initStmt != nullptr && (initStmt->GetNextStmt() == nullptr)); + Statement* initStmt = phdrStmt->GetPrevStmt(); + noway_assert(initStmt != nullptr && (initStmt->GetNextStmt() == nullptr)); - // If it is a duplicated loop condition, skip it. - if (initStmt->GetRootNode()->OperIs(GT_JTRUE)) - { - bool doGetPrev = true; -#ifdef DEBUG - if (opts.optRepeat) - { - // Previous optimization passes may have inserted compiler-generated - // statements other than duplicated loop conditions. - doGetPrev = (initStmt->GetPrevStmt() != nullptr); - } - else + // If it is a duplicated loop condition, skip it. + if (initStmt->GetRootNode()->OperIs(GT_JTRUE)) { - // Must be a duplicated loop condition. - noway_assert(initStmt->GetRootNode()->gtOper == GT_JTRUE); - } + bool doGetPrev = true; +#ifdef DEBUG + if (opts.optRepeat) + { + // Previous optimization passes may have inserted compiler-generated + // statements other than duplicated loop conditions. + doGetPrev = (initStmt->GetPrevStmt() != nullptr); + } + else + { + // Must be a duplicated loop condition. + noway_assert(initStmt->GetRootNode()->gtOper == GT_JTRUE); + } #endif // DEBUG - if (doGetPrev) - { - initStmt = initStmt->GetPrevStmt(); + if (doGetPrev) + { + initStmt = initStmt->GetPrevStmt(); + } + noway_assert(initStmt != nullptr); } - noway_assert(initStmt != nullptr); + + *ppInit = initStmt->GetRootNode(); + } + else + { + *ppInit = nullptr; } - *ppInit = initStmt->GetRootNode(); *ppTest = testStmt->GetRootNode(); *ppIncr = incrStmt->GetRootNode(); @@ -1291,6 +1303,8 @@ bool Compiler::optRecordLoop( // incremented (decremented or lsh, rsh, mul) with a constant value // 3. The iterator is incremented exactly once // 4. The loop condition must use the iterator. + // 5. Finding a constant initializer is optional; if the initializer is not found, or is not constant, + // it is still considered a for-like loop. // if (bottom->bbJumpKind == BBJ_COND) { @@ -1299,56 +1313,37 @@ bool Compiler::optRecordLoop( GenTree* incr; if (!optExtractInitTestIncr(head, bottom, top, &init, &test, &incr)) { + JITDUMP(FMT_LP ": couldn't find init/test/incr; not LPFLG_ITER loop\n", loopInd); goto DONE_LOOP; } unsigned iterVar = BAD_VAR_NUM; if (!optComputeIterInfo(incr, head->bbNext, bottom, &iterVar)) { + JITDUMP(FMT_LP ": increment expression not appropriate form, or not loop invariant; not LPFLG_ITER loop\n", + loopInd); goto DONE_LOOP; } - // Make sure the "iterVar" initialization is never skipped, - // i.e. every pred of ENTRY other than HEAD is in the loop. - for (BasicBlock* const predBlock : entry->PredBlocks()) - { - if ((predBlock != head) && !optLoopTable[loopInd].lpContains(predBlock)) - { - goto DONE_LOOP; - } - } - - if (!optPopulateInitInfo(loopInd, head, init, iterVar)) - { - goto DONE_LOOP; - } + optPopulateInitInfo(loopInd, head, init, iterVar); // Check that the iterator is used in the loop condition. if (!optCheckIterInLoopTest(loopInd, test, head->bbNext, bottom, iterVar)) { + JITDUMP(FMT_LP ": iterator not used in loop condition; not LPFLG_ITER loop\n", loopInd); goto DONE_LOOP; } - // We know the loop has an iterator at this point ->flag it as LPFLG_ITER - // Record the iterator, the pointer to the test node - // and the initial value of the iterator (constant or local var) + // We know the loop has an iterator at this point; flag it as LPFLG_ITER. + JITDUMP(FMT_LP ": setting LPFLG_ITER\n", loopInd); optLoopTable[loopInd].lpFlags |= LPFLG_ITER; // Record iterator. optLoopTable[loopInd].lpIterTree = incr; #if COUNT_LOOPS - // Save the initial value of the iterator - can be lclVar or constant - // Flag the loop accordingly. - iterLoopCount++; -#endif -#if COUNT_LOOPS - simpleTestLoopCount++; -#endif - -#if COUNT_LOOPS // Check if a constant iteration loop. if ((optLoopTable[loopInd].lpFlags & LPFLG_CONST_INIT) && (optLoopTable[loopInd].lpFlags & LPFLG_CONST_LIMIT)) { @@ -1356,31 +1351,6 @@ bool Compiler::optRecordLoop( constIterLoopCount++; } #endif - -#ifdef DEBUG - if (verbose && 0) - { - printf("\nConstant loop initializer:\n"); - gtDispTree(init); - - printf("\nConstant loop body:\n"); - - BasicBlock* block = head; - do - { - block = block->bbNext; - for (Statement* const stmt : block->Statements()) - { - if (stmt->GetRootNode() == incr) - { - break; - } - printf("\n"); - gtDispTree(stmt->GetRootNode()); - } - } while (block != bottom); - } -#endif // DEBUG } DONE_LOOP: