From eed63a7901ac0fe886f5a85cafbf0db62991ec91 Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Fri, 30 Sep 2016 15:18:55 -0400 Subject: Clear heap PerSsaData, heapSsaMap in fgResetForSsa These need to be cleared for the same reasons as the locals' PerSsaData; their omission seems to be a simple oversight. --- src/jit/ssabuilder.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/jit/ssabuilder.cpp b/src/jit/ssabuilder.cpp index 7968b5e2de..13486bc8fa 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. -- cgit v1.2.3 From 50e78c7e10d317d6042964f008f0f8078099588e Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Thu, 3 Nov 2016 11:50:05 -0400 Subject: Clear defnums and post-order nums in fgResetForSsa SSA construction will overwrite these annotations, but only for reachable code. Clear the annotations in all code in `fgResetForSsa` so that downstream analysis won't stumble over stale/invalid SSA annotations in unreachable code. --- src/jit/ssabuilder.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/jit/ssabuilder.cpp b/src/jit/ssabuilder.cpp index 13486bc8fa..f0ee461c45 100644 --- a/src/jit/ssabuilder.cpp +++ b/src/jit/ssabuilder.cpp @@ -118,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; + } + } } } -- cgit v1.2.3 From de040276a0425849e14fbc95f184bbd0e3e09ba5 Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Wed, 28 Sep 2016 12:57:01 -0400 Subject: Add JitOptRepeat debug config flags Add flag JitOptRepeat that specifies a set of methods on which to iteratively perform global optimizations multiple times, and flag JitOptRepeatCount to set the number of iterations (default 2) to apply when JitOptRepeat kicks in. These flags are debug-only; they are intended to facilitate performing experiments investigating optimization opportunities missed due to phase ordering issues. --- src/jit/block.h | 4 + src/jit/compiler.cpp | 182 +++++++++++++++++++++++++++++++++------------- src/jit/compiler.h | 7 ++ src/jit/gentree.cpp | 11 +++ src/jit/jitconfigvalues.h | 10 ++- src/jit/optimizer.cpp | 21 +++++- 6 files changed, 179 insertions(+), 56 deletions(-) diff --git a/src/jit/block.h b/src/jit/block.h index 280b60298e..86dffe14ca 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..3862c74dd1 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,66 @@ 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; + 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; + } + } + } +} + +//------------------------------------------------------------------------ +// 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 +5149,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 867a8545ef..c1023aeedc 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -7621,6 +7621,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 @@ -8345,6 +8346,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 995425b428..b7059714a6 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -16229,6 +16229,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 dc6c9976b0..36d7059443 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); } -- cgit v1.2.3 From 7f6a791f35c2b03fe71853dc4c4b4633e82a8492 Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Fri, 30 Sep 2016 15:14:10 -0400 Subject: Reset EH pred cache in ResetOptAnnotations The cache created in the first iteration may not be correct in the second. It seems there is a more general problem with failure to invalidate this cache (GitHub issue #7844), that this change is working around for the optRepeat case. --- src/jit/compiler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index 3862c74dd1..104c90de74 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -4635,6 +4635,7 @@ void Compiler::ResetOptAnnotations() fgResetForSsa(); vnStore = nullptr; m_opAsgnVarDefSsaNums = nullptr; + m_blockToEHPreds = nullptr; fgSsaPassesCompleted = 0; fgVNPassesCompleted = 0; -- cgit v1.2.3 From 86d4d5999267b687be6c404a3a998a9499f7a57f Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Wed, 26 Oct 2016 17:01:57 -0400 Subject: Clear *_ASG_LHS flags in ResetOptAnnotations This works around an apparent bug in SSA construction (GitHub issue #7846), specifically `fgPerNodeLocalVarLiveness`, where heap uses are not considered upwards-exposed if they follow a heap def in their block (which is incorrect because the store and load are not necessarily must-alias). In the non-repeat configuration, these flags are always cleared coming into SSA construction, because `TreeRenameVariables` is the only thing that sets them. --- src/jit/compiler.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index 104c90de74..74ba9f0e31 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -4650,6 +4650,21 @@ void Compiler::ResetOptAnnotations() 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; + } } } } -- cgit v1.2.3