Merge pull request #7996 from JosephTremoulet/OptRepeat

Add JitOptRepeat debug config flags

7 files changed, 223 insertions, 56 deletions

diff --git a/src/jit/block.h b/src/jit/block.h
index 20d4952ad6..1e6d1754ab 100644
--- a/src/jit/block.h
+++ b/src/jit/block.h
@@ -353,6 +353,10 @@ struct BasicBlock : private LIR::Range
                                        // BBJ_CALLFINALLY block, as well as, on x86, the final step block out of a
                                        // finally.
 
+// Flags that relate blocks to loop structure.
+
+#define BBF_LOOP_FLAGS (BBF_LOOP_PREHEADER | BBF_LOOP_HEAD | BBF_LOOP_CALL0 | BBF_LOOP_CALL1)
+
     bool isRunRarely()
     {
         return ((bbFlags & BBF_RUN_RARELY) != 0);
diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp
index b5cd8c1330..74ba9f0e31 100644
--- a/src/jit/compiler.cpp
+++ b/src/jit/compiler.cpp
@@ -3024,6 +3024,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
     opts.disAsm2         = false;
     opts.dspUnwind       = false;
     opts.compLongAddress = false;
+    opts.optRepeat       = false;
 
 #ifdef LATE_DISASM
     opts.doLateDisasm = false;
@@ -3117,6 +3118,11 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
         {
             opts.compLongAddress = true;
         }
+
+        if (JitConfig.JitOptRepeat().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
+        {
+            opts.optRepeat = true;
+        }
     }
 
     if (verboseDump)
@@ -4359,6 +4365,7 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
         bool doCopyProp      = true;
         bool doAssertionProp = true;
         bool doRangeAnalysis = true;
+        int  iterations      = 1;
 
 #ifdef DEBUG
         doSsa           = (JitConfig.JitDoSsa() != 0);
@@ -4368,72 +4375,88 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
         doCopyProp      = doValueNum && (JitConfig.JitDoCopyProp() != 0);
         doAssertionProp = doValueNum && (JitConfig.JitDoAssertionProp() != 0);
         doRangeAnalysis = doAssertionProp && (JitConfig.JitDoRangeAnalysis() != 0);
-#endif
 
-        if (doSsa)
+        if (opts.optRepeat)
         {
-            fgSsaBuild();
-            EndPhase(PHASE_BUILD_SSA);
+            iterations = JitConfig.JitOptRepeatCount();
         }
+#endif
 
-        if (doEarlyProp)
+        while (iterations > 0)
         {
-            /* Propagate array length and rewrite getType() method call */
-            optEarlyProp();
-            EndPhase(PHASE_EARLY_PROP);
-        }
+            if (doSsa)
+            {
+                fgSsaBuild();
+                EndPhase(PHASE_BUILD_SSA);
+            }
 
-        if (doValueNum)
-        {
-            fgValueNumber();
-            EndPhase(PHASE_VALUE_NUMBER);
-        }
+            if (doEarlyProp)
+            {
+                /* Propagate array length and rewrite getType() method call */
+                optEarlyProp();
+                EndPhase(PHASE_EARLY_PROP);
+            }
 
-        if (doLoopHoisting)
-        {
-            /* Hoist invariant code out of loops */
-            optHoistLoopCode();
-            EndPhase(PHASE_HOIST_LOOP_CODE);
-        }
+            if (doValueNum)
+            {
+                fgValueNumber();
+                EndPhase(PHASE_VALUE_NUMBER);
+            }
 
-        if (doCopyProp)
-        {
-            /* Perform VN based copy propagation */
-            optVnCopyProp();
-            EndPhase(PHASE_VN_COPY_PROP);
-        }
+            if (doLoopHoisting)
+            {
+                /* Hoist invariant code out of loops */
+                optHoistLoopCode();
+                EndPhase(PHASE_HOIST_LOOP_CODE);
+            }
+
+            if (doCopyProp)
+            {
+                /* Perform VN based copy propagation */
+                optVnCopyProp();
+                EndPhase(PHASE_VN_COPY_PROP);
+            }
 
 #if FEATURE_ANYCSE
-        /* Remove common sub-expressions */
-        optOptimizeCSEs();
+            /* Remove common sub-expressions */
+            optOptimizeCSEs();
 #endif // FEATURE_ANYCSE
 
 #if ASSERTION_PROP
-        if (doAssertionProp)
-        {
-            /* Assertion propagation */
-            optAssertionPropMain();
-            EndPhase(PHASE_ASSERTION_PROP_MAIN);
-        }
+            if (doAssertionProp)
+            {
+                /* Assertion propagation */
+                optAssertionPropMain();
+                EndPhase(PHASE_ASSERTION_PROP_MAIN);
+            }
 
-        if (doRangeAnalysis)
-        {
-            /* Optimize array index range checks */
-            RangeCheck rc(this);
-            rc.OptimizeRangeChecks();
-            EndPhase(PHASE_OPTIMIZE_INDEX_CHECKS);
-        }
+            if (doRangeAnalysis)
+            {
+                /* Optimize array index range checks */
+                RangeCheck rc(this);
+                rc.OptimizeRangeChecks();
+                EndPhase(PHASE_OPTIMIZE_INDEX_CHECKS);
+            }
 #endif // ASSERTION_PROP
 
-        /* update the flowgraph if we modified it during the optimization phase*/
-        if (fgModified)
-        {
-            fgUpdateFlowGraph();
-            EndPhase(PHASE_UPDATE_FLOW_GRAPH);
+            /* update the flowgraph if we modified it during the optimization phase*/
+            if (fgModified)
+            {
+                fgUpdateFlowGraph();
+                EndPhase(PHASE_UPDATE_FLOW_GRAPH);
 
-            // Recompute the edge weight if we have modified the flow graph
-            fgComputeEdgeWeights();
-            EndPhase(PHASE_COMPUTE_EDGE_WEIGHTS2);
+                // Recompute the edge weight if we have modified the flow graph
+                fgComputeEdgeWeights();
+                EndPhase(PHASE_COMPUTE_EDGE_WEIGHTS2);
+            }
+
+            // Iterate if requested, resetting annotations first.
+            if (--iterations == 0)
+            {
+                break;
+            }
+            ResetOptAnnotations();
+            RecomputeLoopInfo();
         }
     }
 
@@ -4597,6 +4620,82 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
 #endif // FUNC_INFO_LOGGING
 }
 
+//------------------------------------------------------------------------
+// ResetOptAnnotations: Clear annotations produced during global optimizations.
+//
+// Notes:
+//    The intent of this method is to clear any information typically assumed
+//    to be set only once; it is used between iterations when JitOptRepeat is
+//    in effect.
+
+void Compiler::ResetOptAnnotations()
+{
+    assert(opts.optRepeat);
+    assert(JitConfig.JitOptRepeatCount() > 0);
+    fgResetForSsa();
+    vnStore               = nullptr;
+    m_opAsgnVarDefSsaNums = nullptr;
+    m_blockToEHPreds      = nullptr;
+    fgSsaPassesCompleted  = 0;
+    fgVNPassesCompleted   = 0;
+
+    for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->bbNext)
+    {
+        for (GenTreeStmt* stmt = block->firstStmt(); stmt != nullptr; stmt = stmt->getNextStmt())
+        {
+            stmt->gtFlags &= ~GTF_STMT_HAS_CSE;
+
+            for (GenTreePtr tree = stmt->gtStmt.gtStmtList; tree != nullptr; tree = tree->gtNext)
+            {
+                tree->ClearVN();
+                tree->ClearAssertion();
+                tree->gtCSEnum = NO_CSE;
+
+                // Clear any *_ASG_LHS flags -- these are set during SSA construction,
+                // and the heap live-in calculation depends on them being unset coming
+                // into SSA construction (without clearing them, a block that has a
+                // heap def via one of these before any heap use is treated as not having
+                // an upwards-exposed heap use, even though subsequent heap uses may not
+                // be killed by the store; this seems to be a bug, worked around here).
+                if (tree->OperIsIndir())
+                {
+                    tree->gtFlags &= ~GTF_IND_ASG_LHS;
+                }
+                else if (tree->OperGet() == GT_CLS_VAR)
+                {
+                    tree->gtFlags &= ~GTF_CLS_VAR_ASG_LHS;
+                }
+            }
+        }
+    }
+}
+
+//------------------------------------------------------------------------
+// RecomputeLoopInfo: Recompute loop annotations between opt-repeat iterations.
+//
+// Notes:
+//    The intent of this method is to update loop structure annotations, and those
+//    they depend on; these annotations may have become stale during optimization,
+//    and need to be up-to-date before running another iteration of optimizations.
+
+void Compiler::RecomputeLoopInfo()
+{
+    assert(opts.optRepeat);
+    assert(JitConfig.JitOptRepeatCount() > 0);
+    // Recompute reachability sets, dominators, and loops.
+    optLoopCount   = 0;
+    fgDomsComputed = false;
+    for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->bbNext)
+    {
+        block->bbFlags &= ~BBF_LOOP_FLAGS;
+    }
+    fgComputeReachability();
+    // Rebuild the loop tree annotations themselves.  Since this is performed as
+    // part of 'optOptimizeLoops', this will also re-perform loop rotation, but
+    // not other optimizations, as the others are not part of 'optOptimizeLoops'.
+    optOptimizeLoops();
+}
+
 /*****************************************************************************/
 void Compiler::ProcessShutdownWork(ICorStaticInfo* statInfo)
 {
@@ -5066,6 +5165,7 @@ void Compiler::compCompileFinish()
                                            // the prolog which requires memory
         (info.compLocalsCount <= 32) && (!opts.MinOpts()) && // We may have too many local variables, etc
         (getJitStressLevel() == 0) &&                        // We need extra memory for stress
+        !opts.optRepeat &&                                   // We need extra memory to repeat opts
         !compAllocator->bypassHostAllocator() && // ArenaAllocator::getDefaultPageSize() is artificially low for
                                                  // DirectAlloc
         (compAllocator->getTotalBytesAllocated() > (2 * ArenaAllocator::getDefaultPageSize())) &&
diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index adbfcf3a6d..80dc6507ab 100644
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -7632,6 +7632,7 @@ public:
         bool altJit;     // True if we are an altjit and are compiling this method
 
 #ifdef DEBUG
+        bool optRepeat;                // Repeat optimizer phases k times
         bool compProcedureSplittingEH; // Separate cold code from hot code for functions with EH
         bool dspCode;                  // Display native code generated
         bool dspEHTable;               // Display the EH table reported to the VM
@@ -8356,6 +8357,12 @@ protected:
 #endif
     void compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags* compileFlags);
 
+    // Clear annotations produced during optimizations; to be used between iterations when repeating opts.
+    void ResetOptAnnotations();
+
+    // Regenerate loop descriptors; to be used between iterations when repeating opts.
+    void RecomputeLoopInfo();
+
 #ifdef PROFILING_SUPPORTED
     // Data required for generating profiler Enter/Leave/TailCall hooks
 
diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp
index 093bbe4913..9ed1f51c35 100644
--- a/src/jit/gentree.cpp
+++ b/src/jit/gentree.cpp
@@ -16250,6 +16250,17 @@ void GenTree::ParseArrayAddress(
             // TODO-Review: A NotAField here indicates a failure to properly maintain the field sequence
             // See test case self_host_tests_x86\jit\regression\CLR-x86-JIT\v1-m12-beta2\ b70992\ b70992.exe
             // Safest thing to do here is to drop back to MinOpts
+            CLANG_FORMAT_COMMENT_ANCHOR;
+
+#ifdef DEBUG
+            if (comp->opts.optRepeat)
+            {
+                // We don't guarantee preserving these annotations through the entire optimizer, so
+                // just conservatively return null if under optRepeat.
+                *pArr = nullptr;
+                return;
+            }
+#endif // DEBUG
             noway_assert(!"fldSeqIter is NotAField() in ParseArrayAddress");
         }
 
diff --git a/src/jit/jitconfigvalues.h b/src/jit/jitconfigvalues.h
index c471c9a327..f53de13447 100644
--- a/src/jit/jitconfigvalues.h
+++ b/src/jit/jitconfigvalues.h
@@ -154,10 +154,12 @@ CONFIG_METHODSET(JitNoProcedureSplittingEH, W("JitNoProcedureSplittingEH")) // D
                                                                             // exception handling
 CONFIG_METHODSET(JitStressOnly, W("JitStressOnly")) // Internal Jit stress mode: stress only the specified method(s)
 CONFIG_METHODSET(JitUnwindDump, W("JitUnwindDump")) // Dump the unwind codes for the method
-CONFIG_METHODSET(NgenDisasm, W("NgenDisasm"))       // Same as JitDisasm, but for ngen
-CONFIG_METHODSET(NgenDump, W("NgenDump"))           // Same as JitDump, but for ngen
-CONFIG_METHODSET(NgenDumpIR, W("NgenDumpIR"))       // Same as JitDumpIR, but for ngen
-CONFIG_METHODSET(NgenEHDump, W("NgenEHDump"))       // Dump the EH table for the method, as reported to the VM
+CONFIG_METHODSET(JitOptRepeat, W("JitOptRepeat"))   // Runs optimizer multiple times on the method
+CONFIG_INTEGER(JitOptRepeatCount, W("JitOptRepeatCount"), 2) // Number of times to repeat opts when repeating
+CONFIG_METHODSET(NgenDisasm, W("NgenDisasm"))                // Same as JitDisasm, but for ngen
+CONFIG_METHODSET(NgenDump, W("NgenDump"))                    // Same as JitDump, but for ngen
+CONFIG_METHODSET(NgenDumpIR, W("NgenDumpIR"))                // Same as JitDumpIR, but for ngen
+CONFIG_METHODSET(NgenEHDump, W("NgenEHDump"))                // Dump the EH table for the method, as reported to the VM
 CONFIG_METHODSET(NgenGCDump, W("NgenGCDump"))
 CONFIG_METHODSET(NgenUnwindDump, W("NgenUnwindDump")) // Dump the unwind codes for the method
 CONFIG_STRING(JitDumpFg, W("JitDumpFg"))              // Dumps Xml/Dot Flowgraph for specified method
diff --git a/src/jit/optimizer.cpp b/src/jit/optimizer.cpp
index fdcae0eeee..27990e8a79 100644
--- a/src/jit/optimizer.cpp
+++ b/src/jit/optimizer.cpp
@@ -1085,9 +1085,24 @@ bool Compiler::optExtractInitTestIncr(
     // If it is a duplicated loop condition, skip it.
     if (init->gtFlags & GTF_STMT_CMPADD)
     {
-        // Must be a duplicated loop condition.
-        noway_assert(init->gtStmt.gtStmtExpr->gtOper == GT_JTRUE);
-        init = init->gtPrev;
+        bool doGetPrev = true;
+#ifdef DEBUG
+        if (opts.optRepeat)
+        {
+            // Previous optimization passes may have inserted compiler-generated
+            // statements other than duplicated loop conditions.
+            doGetPrev = (init->gtPrev != nullptr);
+        }
+        else
+        {
+            // Must be a duplicated loop condition.
+            noway_assert(init->gtStmt.gtStmtExpr->gtOper == GT_JTRUE);
+        }
+#endif // DEBUG
+        if (doGetPrev)
+        {
+            init = init->gtPrev;
+        }
         noway_assert(init != nullptr);
     }
 
diff --git a/src/jit/ssabuilder.cpp b/src/jit/ssabuilder.cpp
index 7968b5e2de..f0ee461c45 100644
--- a/src/jit/ssabuilder.cpp
+++ b/src/jit/ssabuilder.cpp
@@ -103,6 +103,8 @@ void Compiler::fgResetForSsa()
     {
         lvaTable[i].lvPerSsaData.Reset();
     }
+    lvHeapPerSsaData.Reset();
+    m_heapSsaMap = nullptr;
     for (BasicBlock* blk = fgFirstBB; blk != nullptr; blk = blk->bbNext)
     {
         // Eliminate phis.
@@ -116,6 +118,32 @@ void Compiler::fgResetForSsa()
                 blk->bbTreeList->gtPrev = last;
             }
         }
+
+        // Clear post-order numbers and SSA numbers; SSA construction will overwrite these,
+        // but only for reachable code, so clear them to avoid analysis getting confused
+        // by stale annotations in unreachable code.
+        blk->bbPostOrderNum = 0;
+        for (GenTreeStmt* stmt = blk->firstStmt(); stmt != nullptr; stmt = stmt->getNextStmt())
+        {
+            for (GenTreePtr tree = stmt->gtStmt.gtStmtList; tree != nullptr; tree = tree->gtNext)
+            {
+                if (tree->IsLocal())
+                {
+                    tree->gtLclVarCommon.SetSsaNum(SsaConfig::RESERVED_SSA_NUM);
+                    continue;
+                }
+
+                Compiler::IndirectAssignmentAnnotation* pIndirAssign = nullptr;
+                if ((tree->OperGet() != GT_ASG) || !GetIndirAssignMap()->Lookup(tree, &pIndirAssign) ||
+                    (pIndirAssign == nullptr))
+                {
+                    continue;
+                }
+
+                pIndirAssign->m_defSsaNum = SsaConfig::RESERVED_SSA_NUM;
+                pIndirAssign->m_useSsaNum = SsaConfig::RESERVED_SSA_NUM;
+            }
+        }
     }
 }