diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e1f24367d17eae..c15cb42786ed7e 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -6058,29 +6058,26 @@ Statement* Compiler::optVNAssertionPropCurStmt(BasicBlock* block, Statement* stm return nextStmt; } -/***************************************************************************** - * - * The entry point for assertion propagation - */ - -void Compiler::optAssertionPropMain() +//------------------------------------------------------------------------------ +// optAssertionPropMain: assertion propagation phase +// +// Returns: +// Suitable phase status. +// +PhaseStatus Compiler::optAssertionPropMain() { if (fgSsaPassesCompleted == 0) { - return; - } -#ifdef DEBUG - if (verbose) - { - printf("*************** In optAssertionPropMain()\n"); - printf("Blocks/Trees at start of phase\n"); - fgDispBasicBlocks(true); + return PhaseStatus::MODIFIED_NOTHING; } -#endif optAssertionInit(false); noway_assert(optAssertionCount == 0); + bool madeChanges = false; + + // Assertion prop can speculatively create trees. + INDEBUG(const unsigned baseTreeID = compGenTreeID); // First discover all value assignments and record them in the table. for (BasicBlock* const block : Blocks()) @@ -6096,13 +6093,16 @@ void Compiler::optAssertionPropMain() if (fgRemoveRestOfBlock) { fgRemoveStmt(block, stmt); - stmt = stmt->GetNextStmt(); + stmt = stmt->GetNextStmt(); + madeChanges = true; continue; } else { // Perform VN based assertion prop before assertion gen. Statement* nextStmt = optVNAssertionPropCurStmt(block, stmt); + madeChanges |= optAssertionPropagatedCurrentStmt; + INDEBUG(madeChanges |= (baseTreeID != compGenTreeID)); // Propagation resulted in removal of the remaining stmts, perform it. if (fgRemoveRestOfBlock) @@ -6139,7 +6139,7 @@ void Compiler::optAssertionPropMain() { block->bbAssertionIn = BitVecOps::MakeEmpty(apTraits); } - return; + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } #ifdef DEBUG @@ -6209,7 +6209,8 @@ void Compiler::optAssertionPropMain() if (fgRemoveRestOfBlock) { fgRemoveStmt(block, stmt); - stmt = stmt->GetNextStmt(); + stmt = stmt->GetNextStmt(); + madeChanges = true; continue; } @@ -6255,6 +6256,7 @@ void Compiler::optAssertionPropMain() #endif // Re-morph the statement. fgMorphBlockStmt(block, stmt DEBUGARG("optAssertionPropMain")); + madeChanges = true; } // Check if propagation removed statements starting from current stmt. @@ -6265,8 +6267,5 @@ void Compiler::optAssertionPropMain() optAssertionPropagatedCurrentStmt = false; // clear it back as we are done with stmts. } -#ifdef DEBUG - fgDebugCheckBBlist(); - fgDebugCheckLinks(); -#endif + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index eb0e9f16759a29..c48b8ad42b9d32 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -1838,8 +1838,9 @@ void Compiler::compInit(ArenaAllocator* pAlloc, // Initialize this to the first phase to run. mostRecentlyActivePhase = PHASE_PRE_IMPORT; - // Initially, no phase checks are active. + // Initially, no phase checks are active, and all dumps are enabled. activePhaseChecks = PhaseChecks::CHECK_NONE; + activePhaseDumps = PhaseDumps::DUMP_ALL; #ifdef FEATURE_TRACELOGGING // Make sure JIT telemetry is initialized as soon as allocations can be made @@ -4786,20 +4787,17 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl if (doRangeAnalysis) { - auto rangePhase = [this]() { - RangeCheck rc(this); - rc.OptimizeRangeChecks(); - }; - // Bounds check elimination via range analysis // - DoPhase(this, PHASE_OPTIMIZE_INDEX_CHECKS, rangePhase); + DoPhase(this, PHASE_OPTIMIZE_INDEX_CHECKS, &Compiler::rangeCheckPhase); } if (fgModified) { // update the flowgraph if we modified it during the optimization phase // + // Note: this invalidates loops, dominators and reachability + // DoPhase(this, PHASE_OPT_UPDATE_FLOW_GRAPH, &Compiler::fgUpdateFlowGraphPhase); // Recompute the edge weight if we have modified the flow graph @@ -4817,11 +4815,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl } } -#ifdef DEBUG - // Run this before we potentially tear down dominators. - fgDebugCheckLinks(compStressCompile(STRESS_REMORPH_TREES, 50)); -#endif - // Dominator and reachability sets are no longer valid. // The loop table is no longer valid. fgDomsComputed = false; @@ -4918,6 +4911,11 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl DoPhase(this, PHASE_ALIGN_LOOPS, &Compiler::placeLoopAlignInstructions); #endif + // The common phase checks and dumps are no longer relevant past this point. + // + activePhaseChecks = PhaseChecks::CHECK_NONE; + activePhaseDumps = PhaseDumps::DUMP_NONE; + // Generate code codeGen->genGenerateCode(methodCodePtr, methodCodeSize); @@ -4977,25 +4975,32 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl #if FEATURE_LOOP_ALIGN //------------------------------------------------------------------------ -// placeLoopAlignInstructions: Iterate over all the blocks and determine -// the best position to place the 'align' instruction. Inserting 'align' -// instructions after an unconditional branch is preferred over inserting -// in the block before the loop. In case there are multiple blocks -// having 'jmp', the one that has lower weight is preferred. -// If the block having 'jmp' is hotter than the block before the loop, -// the align will still be placed after 'jmp' because the processor should -// be smart enough to not fetch extra instruction beyond jmp. +// placeLoopAlignInstructions: determine where to place alignment padding +// +// Returns: +// Suitable phase status // -void Compiler::placeLoopAlignInstructions() +// Notes: +// Iterate over all the blocks and determine +// the best position to place the 'align' instruction. Inserting 'align' +// instructions after an unconditional branch is preferred over inserting +// in the block before the loop. In case there are multiple blocks +// having 'jmp', the one that has lower weight is preferred. +// If the block having 'jmp' is hotter than the block before the loop, +// the align will still be placed after 'jmp' because the processor should +// be smart enough to not fetch extra instruction beyond jmp. +// +PhaseStatus Compiler::placeLoopAlignInstructions() { if (loopAlignCandidates == 0) { - return; + return PhaseStatus::MODIFIED_NOTHING; } JITDUMP("Inside placeLoopAlignInstructions for %d loops.\n", loopAlignCandidates); // Add align only if there were any loops that needed alignment + bool madeChanges = false; weight_t minBlockSoFar = BB_MAX_WEIGHT; BasicBlock* bbHavingAlign = nullptr; BasicBlock::loopNumber currentAlignedLoopNum = BasicBlock::NOT_IN_LOOP; @@ -5005,6 +5010,7 @@ void Compiler::placeLoopAlignInstructions() // Adding align instruction in prolog is not supported // hence just remove that loop from our list. fgFirstBB->unmarkLoopAlign(this DEBUG_ARG("prolog block")); + madeChanges = true; } int loopsToProcess = loopAlignCandidates; @@ -5053,6 +5059,7 @@ void Compiler::placeLoopAlignInstructions() if ((block->bbNatLoopNum != BasicBlock::NOT_IN_LOOP) && (block->bbNatLoopNum == loopTop->bbNatLoopNum)) { loopTop->unmarkLoopAlign(this DEBUG_ARG("loop block appears before top of loop")); + madeChanges = true; } else { @@ -5070,6 +5077,7 @@ void Compiler::placeLoopAlignInstructions() if (bbHavingAlign != nullptr) { + madeChanges = true; bbHavingAlign->bbFlags |= BBF_HAS_ALIGN; } @@ -5085,6 +5093,8 @@ void Compiler::placeLoopAlignInstructions() } assert(loopsToProcess == 0); + + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } #endif diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f1463351a86251..de9449de78d9cf 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1377,11 +1377,20 @@ class TempDsc } }; +// Specify compiler data that a phase might modify +enum class PhaseStatus : unsigned +{ + MODIFIED_NOTHING, // Phase did not make any changes that warrant running post-phase checks or dumping + // the main jit data strutures. + MODIFIED_EVERYTHING, // Phase made changes that warrant running post-phase checks or dumping + // the main jit data strutures. +}; + // interface to hide linearscan implementation from rest of compiler class LinearScanInterface { public: - virtual void doLinearScan() = 0; + virtual PhaseStatus doLinearScan() = 0; virtual void recordVarLocationsAtStartOfBB(BasicBlock* bb) = 0; virtual bool willEnregisterLocalVars() const = 0; #if TRACK_LSRA_STATS @@ -1413,13 +1422,12 @@ enum class PhaseChecks CHECK_ALL }; -// Specify compiler data that a phase might modify -enum class PhaseStatus : unsigned +// Specify which dumps should be run after each phase +// +enum class PhaseDumps { - MODIFIED_NOTHING, // Phase did not make any changes that warrant running post-phase checks or dumping - // the main jit data strutures. - MODIFIED_EVERYTHING, // Phase made changes that warrant running post-phase checks or dumping - // the main jit data strutures. + DUMP_NONE, + DUMP_ALL }; // The following enum provides a simple 1:1 mapping to CLR API's @@ -2927,7 +2935,6 @@ class Compiler #endif BasicBlock* bbNewBasicBlock(BBjumpKinds jumpKind); - void placeLoopAlignInstructions(); /* XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX @@ -4635,7 +4642,11 @@ class Compiler // Do "simple lowering." This functionality is (conceptually) part of "general" // lowering that is distributed between fgMorph and the lowering phase of LSRA. - void fgSimpleLowering(); + PhaseStatus fgSimpleLowering(); + +#if FEATURE_LOOP_ALIGN + PhaseStatus placeLoopAlignInstructions(); +#endif GenTree* fgInitThisClass(); @@ -5046,7 +5057,7 @@ class Compiler template bool fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock); - void fgComputeReachability(); // Perform flow graph node reachability analysis. + PhaseStatus fgComputeReachability(); // Perform flow graph node reachability analysis. bool fgRemoveDeadBlocks(); // Identify and remove dead blocks. @@ -5960,6 +5971,7 @@ class Compiler public: void optInit(); + PhaseStatus rangeCheckPhase(); GenTree* optRemoveRangeCheck(GenTreeBoundsChk* check, GenTree* comma, Statement* stmt); GenTree* optRemoveStandaloneRangeCheck(GenTreeBoundsChk* check, Statement* stmt); void optRemoveCommaBasedRangeCheck(GenTree* comma, Statement* stmt); @@ -6695,7 +6707,7 @@ class Compiler #define FMT_CSE "CSE #%02u" public: - void optOptimizeValnumCSEs(); + PhaseStatus optOptimizeValnumCSEs(); protected: void optValnumCSE_Init(); @@ -6704,7 +6716,7 @@ class Compiler void optValnumCSE_InitDataFlow(); void optValnumCSE_DataFlow(); void optValnumCSE_Availability(); - void optValnumCSE_Heuristic(); + bool optValnumCSE_Heuristic(); bool optDoCSE; // True when we have found a duplicate CSE tree bool optValnumCSE_phase; // True when we are executing the optOptimizeValnumCSEs() phase @@ -6777,16 +6789,16 @@ class Compiler typedef JitHashTable, CopyPropSsaDefStack*> LclNumToLiveDefsMap; // Copy propagation functions. - void optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName); + bool optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName); void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName); - void optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName); + bool optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName); void optCopyPropPushDef(GenTree* defNode, GenTreeLclVarCommon* lclNode, unsigned lclNum, LclNumToLiveDefsMap* curSsaName); unsigned optIsSsaLocal(GenTreeLclVarCommon* lclNode); int optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2); - void optVnCopyProp(); + PhaseStatus optVnCopyProp(); INDEBUG(void optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName)); /************************************************************************** @@ -7289,7 +7301,7 @@ class Compiler void optAssertionRemove(AssertionIndex index); // Assertion prop data flow functions. - void optAssertionPropMain(); + PhaseStatus optAssertionPropMain(); Statement* optVNAssertionPropCurStmt(BasicBlock* block, Statement* stmt); bool optIsTreeKnownIntValue(bool vnBased, GenTree* tree, ssize_t* pConstant, GenTreeFlags* pIconFlags); ASSERT_TP* optInitAssertionDataflowFlags(); @@ -9874,6 +9886,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Phases mostRecentlyActivePhase; // the most recently active phase PhaseChecks activePhaseChecks; // the currently active phase checks + PhaseDumps activePhaseDumps; // the currently active phase dumps //------------------------------------------------------------------------- // The following keeps track of how many bytes of local frame space we've diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index d2b0e28e81b7a1..b673289f2e8b39 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -20,12 +20,14 @@ #include "ssabuilder.h" #include "treelifeupdater.h" -/************************************************************************************** - * - * Corresponding to the live definition pushes, pop the stack as we finish a sub-paths - * of the graph originating from the block. Refer SSA renaming for any additional info. - * "curSsaName" tracks the currently live definitions. - */ +//------------------------------------------------------------------------------ +// optBlockCopyPropPopStacks: pop copy prop stack +// +// Notes: +// Corresponding to the live definition pushes, pop the stack as we finish a sub-paths +// of the graph originating from the block. Refer SSA renaming for any additional info. +// "curSsaName" tracks the currently live definitions. +// void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName) { for (Statement* const stmt : block->Statements()) @@ -57,6 +59,9 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* } #ifdef DEBUG +//------------------------------------------------------------------------------ +// optDumpCopyPropStacks: dump copy prop stack +// void Compiler::optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName) { JITDUMP("{ "); @@ -79,11 +84,18 @@ void Compiler::optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName) JITDUMP("}\n\n"); } #endif -/******************************************************************************************************* - * - * Given the "lclVar" and "copyVar" compute if the copy prop will be beneficial. - * - */ +//------------------------------------------------------------------------------ +// optCopyProp_LclVarScore: compute if the copy prop will be beneficial +// +// Arguments: +// lclVarDsc - variable that is target of a potential copy prop +// copyVarDsc - variable that is source of a potential copy prop +// preferOp2 - true if ...?? +// +// Returns: +// "score" indicating relative profitability of the copy +// (non-negative: favorable) +// int Compiler::optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2) { int score = 0; @@ -129,13 +141,17 @@ int Compiler::optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDs // lclNum - The local number of said tree // curSsaName - The map from lclNum to its recently live definitions as a stack // -void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName) +// Returns: +// true if any changes were made +// +bool Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName) { assert((lclNum != BAD_VAR_NUM) && (optIsSsaLocal(tree) == lclNum) && ((tree->gtFlags & GTF_VAR_DEF) == 0)); assert(tree->gtVNPair.BothDefined()); - LclVarDsc* varDsc = lvaGetDesc(lclNum); - ValueNum lclDefVN = varDsc->GetPerSsaData(tree->GetSsaNum())->m_vnPair.GetConservative(); + bool madeChanges = false; + LclVarDsc* varDsc = lvaGetDesc(lclNum); + ValueNum lclDefVN = varDsc->GetPerSsaData(tree->GetSsaNum())->m_vnPair.GetConservative(); assert(lclDefVN != ValueNumStore::NoVN); for (LclNumToLiveDefsMap::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter) @@ -245,8 +261,12 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned DISPNODE(tree); } #endif + + madeChanges = true; break; } + + return madeChanges; } //------------------------------------------------------------------------------ @@ -353,7 +373,10 @@ void Compiler::optCopyPropPushDef(GenTree* defNode, // block - Block the tree belongs to // curSsaName - The map from lclNum to its recently live definitions as a stack // -void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName) +// Returns: +// true if any copy prop was done +// +bool Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName) { #ifdef DEBUG JITDUMP("Copy Assertion for " FMT_BB "\n", block->bbNum); @@ -366,6 +389,7 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa // We are not generating code so we don't need to deal with liveness change TreeLifeUpdater treeLifeUpdater(this); + bool madeChanges = false; // There are no definitions at the start of the block. So clear it. compCurLifeTree = nullptr; @@ -413,49 +437,49 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa continue; } - optCopyProp(stmt, tree->AsLclVarCommon(), lclNum, curSsaName); + madeChanges |= optCopyProp(stmt, tree->AsLclVarCommon(), lclNum, curSsaName); } } } + + return madeChanges; } -/************************************************************************************** - * - * This stage performs value numbering based copy propagation. Since copy propagation - * is about data flow, we cannot find them in assertion prop phase. In assertion prop - * we can identify copies that like so: if (a == b) else, i.e., control flow assertions. - * - * To identify data flow copies, we follow a similar approach to SSA renaming. We walk - * each path in the graph keeping track of every live definition. Thus when we see a - * variable that shares the VN with a live definition, we'd replace this variable with - * the variable in the live definition. - * - * We do this to be in conventional SSA form. This can very well be changed later. - * - * For example, on some path in the graph: - * a0 = x0 - * : <- other blocks - * : - * a1 = y0 - * : - * : <- other blocks - * b0 = x0, we cannot substitute x0 with a0, because currently our backend doesn't - * treat lclNum and ssaNum together as a variable, but just looks at lclNum. If we - * substituted x0 with a0, then we'd be in general SSA form. - * - */ -void Compiler::optVnCopyProp() +//------------------------------------------------------------------------------ +// optVnCopyProp: value numbering based copy propagation +// +// Returns: +// Suitable phase status +// +// Notes: +// +// This phase performs value numbering based copy propagation. Since copy propagation +// is about data flow, we cannot find them in assertion prop phase. In assertion prop +// we can identify copies that like so: if (a == b) else, i.e., control flow assertions. +// +// To identify data flow copies, we follow a similar approach to SSA renaming. We walk +// each path in the graph keeping track of every live definition. Thus when we see a +// variable that shares the VN with a live definition, we'd replace this variable with +// the variable in the live definition. +// +// We do this to be in conventional SSA form. This can very well be changed later. +// +// For example, on some path in the graph: +// a0 = x0 +// : <- other blocks +// : +// a1 = y0 +// : +// : <- other blocks +// b0 = x0, we cannot substitute x0 with a0, because currently our backend doesn't +// treat lclNum and ssaNum together as a variable, but just looks at lclNum. If we +// substituted x0 with a0, then we'd be in general SSA form. +// +PhaseStatus Compiler::optVnCopyProp() { -#ifdef DEBUG - if (verbose) - { - printf("*************** In optVnCopyProp()\n"); - } -#endif - if (fgSsaPassesCompleted == 0) { - return; + return PhaseStatus::MODIFIED_NOTHING; } VarSetOps::AssignNoCopy(this, compCurLife, VarSetOps::MakeEmpty(this)); @@ -464,17 +488,25 @@ void Compiler::optVnCopyProp() { // The map from lclNum to its recently live definitions as a stack. LclNumToLiveDefsMap m_curSsaName; + bool m_madeChanges = false; public: CopyPropDomTreeVisitor(Compiler* compiler) - : DomTreeVisitor(compiler, compiler->fgSsaDomTree), m_curSsaName(compiler->getAllocator(CMK_CopyProp)) + : DomTreeVisitor(compiler, compiler->fgSsaDomTree) + , m_curSsaName(compiler->getAllocator(CMK_CopyProp)) + , m_madeChanges(false) { } + bool MadeChanges() const + { + return m_madeChanges; + } + void PreOrderVisit(BasicBlock* block) { // TODO-Cleanup: Move this function from Compiler to this class. - m_compiler->optBlockCopyProp(block, &m_curSsaName); + m_madeChanges |= m_compiler->optBlockCopyProp(block, &m_curSsaName); } void PostOrderVisit(BasicBlock* block) @@ -507,4 +539,6 @@ void Compiler::optVnCopyProp() // Tracked variable count increases after CopyProp, so don't keep a shorter array around. // Destroy (release) the varset. VarSetOps::AssignNoCopy(this, compCurLife, VarSetOps::UninitVal()); + + return visitor.MadeChanges() ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index c1709ccb241921..8b97fc70a7bb48 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -539,28 +539,23 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock) //------------------------------------------------------------------------ // fgComputeReachability: Compute the dominator and reachable sets. // -// Use `fgReachable()` to check reachability, `fgDominate()` to check dominance. +// Returns: +// Suitable phase status // -// Also, compute the list of return blocks `fgReturnBlocks` and set of enter blocks `fgEnterBlks`. -// Delete unreachable blocks. +// Notes: +// Also computes the list of return blocks `fgReturnBlocks` +// and set of enter blocks `fgEnterBlks`. // -// Assumptions: -// Assumes the predecessor lists are computed and correct. +// Delete unreachable blocks. // -void Compiler::fgComputeReachability() +// Assumes the predecessor lists are computed and correct. +// +// Use `fgReachable()` to check reachability. +// Use `fgDominate()` to check dominance. +// +PhaseStatus Compiler::fgComputeReachability() { -#ifdef DEBUG - if (verbose) - { - printf("*************** In fgComputeReachability\n"); - } - - fgVerifyHandlerTab(); - - // Make sure that the predecessor lists are accurate assert(fgComputePredsDone); - fgDebugCheckBBlist(); -#endif // DEBUG fgComputeReturnBlocks(); @@ -591,6 +586,8 @@ void Compiler::fgComputeReachability() return true; }; + bool madeChanges = false; + do { // Just to be paranoid, avoid infinite loops; fall back to minopts. @@ -602,7 +599,7 @@ void Compiler::fgComputeReachability() // Walk the flow graph, reassign block numbers to keep them in ascending order. JITDUMP("\nRenumbering the basic blocks for fgComputeReachability pass #%u\n", passNum); passNum++; - fgRenumberBlocks(); + madeChanges |= fgRenumberBlocks(); // // Compute fgEnterBlks @@ -621,26 +618,17 @@ void Compiler::fgComputeReachability() // changed = fgRemoveUnreachableBlocks(canRemoveBlock); + madeChanges |= changed; } while (changed); -#ifdef DEBUG - if (verbose) - { - printf("\nAfter computing reachability:\n"); - fgDispBasicBlocks(verboseTrees); - printf("\n"); - } - - fgVerifyHandlerTab(); - fgDebugCheckBBlist(true); -#endif // DEBUG - // // Now, compute the dominators // fgComputeDoms(); + + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } //------------------------------------------------------------------------ @@ -5933,6 +5921,12 @@ PhaseStatus Compiler::fgUpdateFlowGraphPhase() constexpr bool doTailDup = false; constexpr bool isPhase = true; const bool madeChanges = fgUpdateFlowGraph(doTailDup, isPhase); + + // Dominator and reachability sets are no longer valid. + // The loop table is no longer valid. + fgDomsComputed = false; + optLoopTableValid = false; + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index adb0c53158342b..2613df36930adc 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -2949,6 +2949,9 @@ PhaseStatus Compiler::fgFindOperOrder() // fgSimpleLowering: do full walk of all IR, lowering selected operations // and computing lvaOutgoingArgSpaceSize. // +// Returns: +// Suitable phase status +// // Notes: // Lowers GT_ARR_LENGTH, GT_MDARR_LENGTH, GT_MDARR_LOWER_BOUND, GT_BOUNDS_CHECK. // @@ -2959,8 +2962,10 @@ PhaseStatus Compiler::fgFindOperOrder() // after optimization (in case calls are removed) and need to look at // all possible calls in the method. // -void Compiler::fgSimpleLowering() +PhaseStatus Compiler::fgSimpleLowering() { + bool madeChanges = false; + #if FEATURE_FIXED_OUT_ARGS unsigned outgoingArgSpaceSize = 0; #endif // FEATURE_FIXED_OUT_ARGS @@ -3036,6 +3041,7 @@ void Compiler::fgSimpleLowering() JITDUMP("After Lower %s:\n", GenTree::OpName(tree->OperGet())); DISPRANGE(LIR::ReadOnlyRange(arr, tree)); + madeChanges = true; break; } @@ -3043,6 +3049,7 @@ void Compiler::fgSimpleLowering() { // Add in a call to an error routine. fgSetRngChkTarget(tree, false); + madeChanges = true; break; } @@ -3133,18 +3140,7 @@ void Compiler::fgSimpleLowering() #endif // FEATURE_FIXED_OUT_ARGS -#ifdef DEBUG - if (verbose && fgRngChkThrowAdded) - { - printf("\nAfter fgSimpleLowering() added some RngChk throw blocks"); - fgDispBasicBlocks(); - fgDispHandlerTab(); - printf("\n"); - } - - fgDebugCheckBBlist(); - fgDebugCheckLinks(); -#endif + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } /***************************************************************************************************** @@ -3465,28 +3461,25 @@ bool Compiler::fgFuncletsAreCold() #endif // defined(FEATURE_EH_FUNCLETS) -/*------------------------------------------------------------------------- - * - * Walk the basic blocks list to determine the first block to place in the - * cold section. This would be the first of a series of rarely executed blocks - * such that no succeeding blocks are in a try region or an exception handler - * or are rarely executed. - */ - +//------------------------------------------------------------------------ +// fgDetermineFirstColdBlock: figure out where we might split the block +// list to put some blocks into the cold code section +// +// Returns: +// Suitable phase status +// +// Notes: +// Walk the basic blocks list to determine the first block to place in the +// cold section. This would be the first of a series of rarely executed blocks +// such that no succeeding blocks are in a try region or an exception handler +// or are rarely executed. +// PhaseStatus Compiler::fgDetermineFirstColdBlock() { -#ifdef DEBUG - if (verbose) - { - printf("\n*************** In fgDetermineFirstColdBlock()\n"); - } -#endif // DEBUG - // Since we may need to create a new transition block // we assert that it is OK to create new blocks. // assert(fgSafeBasicBlockCreation); - assert(fgFirstColdBlock == nullptr); if (!opts.compProcedureSplitting) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 4417858838d548..6119cc7aa430bc 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -1221,10 +1221,9 @@ BasicBlock* LinearScan::getNextBlock() // None // // Return Value: -// None. +// Suitable phase status // - -void LinearScan::doLinearScan() +PhaseStatus LinearScan::doLinearScan() { // Check to see whether we have any local variables to enregister. // We initialize this in the constructor based on opt settings, @@ -1278,6 +1277,8 @@ void LinearScan::doLinearScan() #endif compiler->compLSRADone = true; + + return PhaseStatus::MODIFIED_EVERYTHING; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 3361f45e27e902..e17f42d760be6d 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -615,7 +615,7 @@ class LinearScan : public LinearScanInterface LinearScan(Compiler* theCompiler); // This is the main driver - virtual void doLinearScan(); + virtual PhaseStatus doLinearScan(); static bool isSingleRegister(regMaskTP regMask) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ff657de745a7ce..75fcead06f7ca3 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14605,39 +14605,54 @@ Compiler::FoldResult Compiler::fgFoldConditional(BasicBlock* block) } #endif - /* if the block was a loop condition we may have to modify - * the loop table */ - - for (unsigned loopNum = 0; loopNum < optLoopCount; loopNum++) + // Handle updates to the loop table. + // Note this is distinct from the check for BBF_LOOP_HEAD above. + // + if (optLoopTableValid) { - /* Some loops may have been already removed by - * loop unrolling or conditional folding */ - - if (optLoopTable[loopNum].lpFlags & LPFLG_REMOVED) + for (unsigned loopNum = 0; loopNum < optLoopCount; loopNum++) { - continue; - } + LoopDsc& loop = optLoopTable[loopNum]; - /* We are only interested in the loop bottom */ + // Some loops may have been already removed by + // loop unrolling or conditional folding + // + if (loop.lpFlags & LPFLG_REMOVED) + { + continue; + } - if (optLoopTable[loopNum].lpBottom == block) - { - if (cond->AsIntCon()->gtIconVal == 0) + // Removed edge from bottom -> entry? + // + if ((loop.lpBottom == block) && (loop.lpEntry == bNotTaken)) { - /* This was a bogus loop (condition always false) - * Remove the loop from the table */ + // This either destroyed the loop or lessened its extent. + // We currently ignore the latter. + // + if (loop.lpEntry->countOfInEdges() == 1) + { + // We removed the only backedge. + // + JITDUMP("Removing loop " FMT_LP " (from " FMT_BB " to " FMT_BB + ") -- no longer has a backedge\n\n", + loopNum, loop.lpTop->bbNum, loop.lpBottom->bbNum); - optMarkLoopRemoved(loopNum); + optMarkLoopRemoved(loopNum); + loop.lpTop->unmarkLoopAlign(this DEBUG_ARG("removed loop")); + } + } - optLoopTable[loopNum].lpTop->unmarkLoopAlign(this DEBUG_ARG("Bogus loop")); + // Removed edge from head -> entry? + // + if ((loop.lpHead == block) && (loop.lpEntry == bNotTaken)) + { + // Loop is no longer reachable from outside + // + JITDUMP("Removing loop " FMT_LP " (from " FMT_BB " to " FMT_BB ") -- no longer reachable\n\n", + loopNum, loop.lpTop->bbNum, loop.lpBottom->bbNum); -#ifdef DEBUG - if (verbose) - { - printf("Removing loop " FMT_LP " (from " FMT_BB " to " FMT_BB ")\n\n", loopNum, - optLoopTable[loopNum].lpTop->bbNum, optLoopTable[loopNum].lpBottom->bbNum); - } -#endif + optMarkLoopRemoved(loopNum); + loop.lpTop->unmarkLoopAlign(this DEBUG_ARG("removed loop")); } } } diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index 03e41a07f73ca1..76912c0e174ac4 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -1728,6 +1728,7 @@ class CSE_Heuristic unsigned enregCount; // count of the number of predicted enregistered variables bool largeFrame; bool hugeFrame; + bool madeChanges; Compiler::codeOptimize codeOptKind; Compiler::CSEdsc** sortTab; size_t sortSiz; @@ -1747,6 +1748,11 @@ class CSE_Heuristic return codeOptKind; } + bool MadeChanges() const + { + return madeChanges; + } + // Perform the Initialization step for our CSE Heuristics. Determine the various cut off values to use for // the aggressive, moderate and conservative CSE promotions. Count the number of enregisterable variables. // Determine if the method has a large or huge stack frame. @@ -3410,6 +3416,7 @@ class CSE_Heuristic if (doCSE) { PerformCSE(&candidate); + madeChanges = true; } } } @@ -3422,12 +3429,14 @@ class CSE_Heuristic } }; -/***************************************************************************** - * - * Routine for performing the Value Number based CSE using our heuristics - */ - -void Compiler::optValnumCSE_Heuristic() +//------------------------------------------------------------------------ +// optValnumCSE_Heuristic: Perform common sub-expression elimination +// based on profitabiliy heuristic +// +// Returns: +// true if changes were made +// +bool Compiler::optValnumCSE_Heuristic() { #ifdef DEBUG if (verbose) @@ -3444,24 +3453,30 @@ void Compiler::optValnumCSE_Heuristic() cse_heuristic.SortCandidates(); cse_heuristic.ConsiderCandidates(); cse_heuristic.Cleanup(); -} -/***************************************************************************** - * - * Perform common sub-expression elimination. - */ + return cse_heuristic.MadeChanges(); +} -void Compiler::optOptimizeValnumCSEs() +//------------------------------------------------------------------------ +// optOptimizeValnumCSEs: Perform common sub-expression elimination +// +// Returns: +// Suitable phase status +// +PhaseStatus Compiler::optOptimizeValnumCSEs() { + #ifdef DEBUG if (optConfigDisableCSE()) { - return; // Disabled by JitNoCSE + JITDUMP("Disabled by JitNoCSE\n"); + return PhaseStatus::MODIFIED_NOTHING; } #endif optValnumCSE_phase = true; optCSEweight = -1.0f; + bool madeChanges = false; optValnumCSE_Init(); @@ -3470,10 +3485,12 @@ void Compiler::optOptimizeValnumCSEs() optValnumCSE_InitDataFlow(); optValnumCSE_DataFlow(); optValnumCSE_Availability(); - optValnumCSE_Heuristic(); + madeChanges = optValnumCSE_Heuristic(); } optValnumCSE_phase = false; + + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } /***************************************************************************** diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 4deb904b989898..14d24a83b3dfa7 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -53,6 +53,9 @@ DataFlow::DataFlow(Compiler* pCompiler) : m_pCompiler(pCompiler) // that doesn't dominate all the return blocks has its weight dropped in half // (but only if the first block *does* dominate all the returns). // +// Returns: +// Suitable phase status +// // Notes: // Depends on dominators, and fgReturnBlocks being set. // @@ -62,18 +65,16 @@ PhaseStatus Compiler::optSetBlockWeights() assert(fgDomsComputed); assert(fgReturnBlocksComputed); -#ifdef DEBUG - bool changed = false; -#endif - + bool madeChanges = false; bool firstBBDominatesAllReturns = true; const bool usingProfileWeights = fgIsUsingProfileWeights(); for (BasicBlock* const block : Blocks()) { /* Blocks that can't be reached via the first block are rarely executed */ - if (!fgReachable(fgFirstBB, block)) + if (!fgReachable(fgFirstBB, block) && !block->isRunRarely()) { + madeChanges = true; block->bbSetRunRarely(); } @@ -113,7 +114,7 @@ PhaseStatus Compiler::optSetBlockWeights() // if (!blockDominatesAllReturns) { - INDEBUG(changed = true); + madeChanges = true; // TODO-Cleanup: we should use: // block->scaleBBWeight(0.5); @@ -127,19 +128,7 @@ PhaseStatus Compiler::optSetBlockWeights() } } -#if DEBUG - if (changed && verbose) - { - printf("\nAfter optSetBlockWeights:\n"); - fgDispBasicBlocks(); - printf("\n"); - } - - /* Check that the flowgraph data (bbNum, bbRefs, bbPreds) is up-to-date */ - fgDebugCheckBBlist(); -#endif - - return PhaseStatus::MODIFIED_EVERYTHING; + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } //------------------------------------------------------------------------ @@ -10234,6 +10223,13 @@ void Compiler::optMarkLoopRemoved(unsigned loopNum) } } + // Unmark any preheader + // + if ((loop.lpFlags & LPFLG_HAS_PREHEAD) != 0) + { + loop.lpHead->bbFlags &= ~BBF_LOOP_PREHEADER; + } + loop.lpFlags |= LPFLG_REMOVED; #ifdef DEBUG diff --git a/src/coreclr/jit/phase.cpp b/src/coreclr/jit/phase.cpp index 55fe5dec844ae1..47a5f5b8656076 100644 --- a/src/coreclr/jit/phase.cpp +++ b/src/coreclr/jit/phase.cpp @@ -71,40 +71,8 @@ void Phase::PrePhase() comp->BeginPhase(m_phase); #ifdef DEBUG - - // To help in the incremental conversion of jit activity to phases - // without greatly increasing dump size or checked jit time, we - // currently allow the phases that do pre-phase checks and - // dumps via the phase object, and not via explicit calls from - // the various methods in the phase. - // - // In the long run the aim is to get rid of all pre-phase checks - // and dumps, relying instead on post-phase checks and dumps from - // the preceding phase. - // - // Currently the list is just the set of phases that have custom - // derivations from the Phase class. - static Phases s_allowlist[] = {PHASE_BUILD_SSA, PHASE_OPTIMIZE_VALNUM_CSES, PHASE_RATIONALIZE, PHASE_LOWERING, - PHASE_STACK_LEVEL_SETTER}; - bool doPrePhase = false; - - for (size_t i = 0; i < sizeof(s_allowlist) / sizeof(Phases); i++) - { - if (m_phase == s_allowlist[i]) - { - doPrePhase = true; - break; - } - } - if (VERBOSE) { - if (doPrePhase) - { - printf("Trees before %s\n", m_name); - comp->fgDispBasicBlocks(true); - } - if (comp->compIsForInlining()) { printf("\n*************** Inline @[%06u] Starting PHASE %s\n", @@ -115,17 +83,6 @@ void Phase::PrePhase() printf("\n*************** Starting PHASE %s\n", m_name); } } - - if (doPrePhase) - { - if ((comp->activePhaseChecks == PhaseChecks::CHECK_ALL) && (comp->expensiveDebugCheckLevel >= 2)) - { - // If everyone used the Phase class, this would duplicate the PostPhase() from the previous phase. - // But, not everyone does, so go ahead and do the check here, too. - comp->fgDebugCheckBBlist(); - comp->fgDebugCheckLinks(); - } - } #endif // DEBUG #if DUMP_FLOWGRAPHS @@ -146,130 +103,27 @@ void Phase::PostPhase(PhaseStatus status) #ifdef DEBUG // Don't dump or check post phase unless the phase made changes. - const bool madeChanges = (status != PhaseStatus::MODIFIED_NOTHING); - const char* const statusMessage = madeChanges ? "" : " [no changes]"; - bool doPostPhase = false; - - // To help in the incremental conversion of jit activity to phases - // without greatly increasing dump size or checked jit time, we - // currently allow the phases that do post-phase checks and - // dumps via the phase object, and not via explicit calls from - // the various methods in the phase. - // - // As we remove the explicit checks and dumps from each phase, we - // will add to this list; once all phases are updated, we can - // remove the list entirely. - // - // This list includes custom derivations from the Phase class as - // well as the new-style phases that have been updated to return - // PhaseStatus from their DoPhase methods. - // - // clang-format off - - static Phases s_allowlist[] = { - PHASE_PRE_IMPORT, - PHASE_INCPROFILE, - PHASE_IBCPREP, - PHASE_IMPORTATION, - PHASE_PATCHPOINTS, - PHASE_IBCINSTR, - PHASE_INDXCALL, - PHASE_POST_IMPORT, - PHASE_MORPH_INIT, - PHASE_MORPH_INLINE, - PHASE_ALLOCATE_OBJECTS, - PHASE_MORPH_ADD_INTERNAL, - PHASE_EMPTY_TRY, - PHASE_EMPTY_FINALLY, - PHASE_MERGE_FINALLY_CHAINS, - PHASE_CLONE_FINALLY, - PHASE_UPDATE_FINALLY_FLAGS, - PHASE_COMPUTE_PREDS, - PHASE_MERGE_THROWS, - PHASE_EARLY_UPDATE_FLOW_GRAPH, - PHASE_PROMOTE_STRUCTS, - PHASE_STR_ADRLCL, - PHASE_FWD_SUB, - PHASE_MORPH_IMPBYREF, - // - // (enable all phase checks) - // - PHASE_MORPH_GLOBAL, - PHASE_GS_COOKIE, - PHASE_COMPUTE_EDGE_WEIGHTS, -#if defined(FEATURE_EH_FUNCLETS) - PHASE_CREATE_FUNCLETS, -#endif - PHASE_INVERT_LOOPS, - PHASE_OPTIMIZE_FLOW, - // reachability - // block weights - PHASE_FIND_LOOPS, - PHASE_CLONE_LOOPS, - PHASE_UNROLL_LOOPS, - PHASE_CLEAR_LOOP_INFO, - PHASE_MARK_LOCAL_VARS, - PHASE_OPTIMIZE_ADD_COPIES, - PHASE_OPTIMIZE_BOOLS, - PHASE_FIND_OPER_ORDER, - PHASE_SET_BLOCK_ORDER, - PHASE_BUILD_SSA, - // (ssa subphases) - PHASE_EARLY_PROP, - PHASE_VALUE_NUMBER, - PHASE_HOIST_LOOP_CODE, - // copy prop - // PHASE_OPTIMIZE_BRANCHES, - // cse - // assertion prop - // range check - // PHASE_OPT_UPDATE_FLOW_GRAPH, - // edge weights 2 - PHASE_INSERT_GC_POLLS, - PHASE_OPTIMIZE_LAYOUT, - // first cold block - PHASE_RATIONALIZE, - PHASE_LOWERING, - // lsra - PHASE_STACK_LEVEL_SETTER - // align loops - // codegen - }; - - // clang-format on - - // Also note when this phase has not opted into the active post phase checks. // - const bool doPostPhaseChecks = comp->activePhaseChecks == PhaseChecks::CHECK_ALL; - const char* checkMessage = - madeChanges && doPostPhaseChecks ? " [phase has not yet enabled common post phase checks]" : ""; + const bool madeChanges = (status != PhaseStatus::MODIFIED_NOTHING); + const bool doPostPhase = madeChanges; + const bool doPostPhaseChecks = (comp->activePhaseChecks == PhaseChecks::CHECK_ALL); + const bool doPostPhaseDumps = (comp->activePhaseDumps == PhaseDumps::DUMP_ALL); - if (madeChanges) - { - for (size_t i = 0; i < sizeof(s_allowlist) / sizeof(Phases); i++) - { - if (m_phase == s_allowlist[i]) - { - doPostPhase = true; - checkMessage = ""; - break; - } - } - } + const char* const statusMessage = madeChanges ? "" : " [no changes]"; if (VERBOSE) { if (comp->compIsForInlining()) { - printf("\n*************** Inline @[%06u] Finishing PHASE %s%s%s\n", - Compiler::dspTreeID(comp->impInlineInfo->iciCall), m_name, statusMessage, checkMessage); + printf("\n*************** Inline @[%06u] Finishing PHASE %s%s\n", + Compiler::dspTreeID(comp->impInlineInfo->iciCall), m_name, statusMessage); } else { - printf("\n*************** Finishing PHASE %s%s%s\n", m_name, statusMessage, checkMessage); + printf("\n*************** Finishing PHASE %s%s\n", m_name, statusMessage); } - if (doPostPhase) + if (doPostPhase && doPostPhaseDumps) { printf("Trees after %s\n", m_name); comp->fgDispBasicBlocks(true); diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index b94f124d003280..a228c2ad9fc042 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -6,6 +6,19 @@ #include "jitpch.h" #include "rangecheck.h" +//------------------------------------------------------------------------ +// rangeCheckPhase: optimize bounds checks via range analysis +// +// Returns: +// Suitable phase status +// +PhaseStatus Compiler::rangeCheckPhase() +{ + RangeCheck rc(this); + const bool madeChanges = rc.OptimizeRangeChecks(); + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; +} + // Max stack depth (path length) in walking the UD chain. static const int MAX_SEARCH_DEPTH = 100; @@ -1563,20 +1576,14 @@ void RangeCheck::MapMethodDefs() #endif // Entry point to range check optimizations. -void RangeCheck::OptimizeRangeChecks() +bool RangeCheck::OptimizeRangeChecks() { if (m_pCompiler->fgSsaPassesCompleted == 0) { - return; - } -#ifdef DEBUG - if (m_pCompiler->verbose) - { - JITDUMP("*************** In OptimizeRangeChecks()\n"); - JITDUMP("Blocks/trees before phase\n"); - m_pCompiler->fgDispBasicBlocks(true); + return false; } -#endif + + bool madeChanges = false; // Walk through trees looking for arrBndsChk node and check if it can be optimized. for (BasicBlock* const block : m_pCompiler->Blocks()) @@ -1589,7 +1596,7 @@ void RangeCheck::OptimizeRangeChecks() { if (IsOverBudget() && !m_updateStmt) { - return; + return madeChanges; } OptimizeRangeCheck(block, stmt, tree); @@ -1599,7 +1606,10 @@ void RangeCheck::OptimizeRangeChecks() { m_pCompiler->gtSetStmtInfo(stmt); m_pCompiler->fgSetStmtSeq(stmt); + madeChanges = true; } } } + + return madeChanges; } diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h index db1c25a09eda54..2dfd12db56dc10 100644 --- a/src/coreclr/jit/rangecheck.h +++ b/src/coreclr/jit/rangecheck.h @@ -608,9 +608,9 @@ class RangeCheck // TODO-CQ: This is not general enough. bool BetweenBounds(Range& range, GenTree* upper, int arrSize); - // Entry point to optimize range checks in the block. Assumes value numbering + // Entry point to optimize range checks in the method. Assumes value numbering // and assertion prop phases are completed. - void OptimizeRangeChecks(); + bool OptimizeRangeChecks(); // Given a "tree" node, check if it contains array bounds check node and // optimize to remove it, if possible. Requires "stmt" and "block" that diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 2493aea39e71a6..822e68683927a0 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -749,6 +749,30 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock return false; } + // If block is a loop header, skip jump threading. + // + // This is an artificial limitation to ensure that subsequent loop table valididity + // checking can pass. We do not expect a loop entry to have multiple non-loop predecessors. + // + // This only blocks jump threading in a small number of cases. + // Revisit once we can ensure that loop headers are not critical blocks. + // + for (unsigned loopNum = 0; loopNum < optLoopCount; loopNum++) + { + const LoopDsc& loop = optLoopTable[loopNum]; + + if (loop.lpFlags & LPFLG_REMOVED) + { + continue; + } + + if (block == loop.lpHead) + { + JITDUMP(FMT_BB " is the header for " FMT_LP "; no threading\n", block->bbNum, loopNum); + return false; + } + } + // Since flow is going to bypass block, make sure there // is nothing in block that can cause a side effect. //