From 94477ebb1e8364b8c17a320a68a900408431ca3b Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Mon, 25 Nov 2024 16:57:49 +0000 Subject: [PATCH] JIT: Remove some `fgRenumberBlocks` calls (#110026) --- src/coreclr/jit/compiler.cpp | 9 --------- src/coreclr/jit/fginline.cpp | 8 -------- src/coreclr/jit/fgopt.cpp | 14 +------------- src/coreclr/jit/flowgraph.cpp | 7 +------ src/coreclr/jit/gschecks.cpp | 6 ------ src/coreclr/jit/helperexpansion.cpp | 5 ----- src/coreclr/jit/jiteh.cpp | 4 ++-- src/coreclr/jit/optimizer.cpp | 11 +++-------- 8 files changed, 7 insertions(+), 57 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index f1db977d09b5a..78888aef6a2e4 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4761,13 +4761,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl }; DoPhase(this, PHASE_POST_MORPH, postMorphPhase); - // If we needed to create any new BasicBlocks then renumber the blocks - // - if (fgBBcount > preMorphBBCount) - { - fgRenumberBlocks(); - } - // GS security checks for unsafe buffers // DoPhase(this, PHASE_GS_COOKIE, &Compiler::gsPhase); @@ -5888,8 +5881,6 @@ void Compiler::RecomputeFlowGraphAnnotations() // Recompute reachability sets, dominators, and loops. optResetLoopInfo(); - fgRenumberBlocks(); - fgInvalidateDfsTree(); fgDfsBlocksAndRemove(); optFindLoops(); diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 9d5cdbba32e0c..d7abf54dfc394 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -913,14 +913,6 @@ PhaseStatus Compiler::fgInline() #endif // DEBUG - if (madeChanges) - { - // Optional quirk to keep this as zero diff. Some downstream phases are bbNum sensitive - // but rely on the ambient bbNums. - // - fgRenumberBlocks(); - } - if (fgPgoConsistent) { Metrics.ProfileConsistentAfterInline++; diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 64696e13324f6..a11b3c9f63ba4 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -752,25 +752,13 @@ PhaseStatus Compiler::fgPostImportationCleanup() } } - // Did we alter any flow or EH? - // - const bool madeFlowChanges = (addedBlocks > 0) || (delCnt > 0) || (removedBlks > 0); - - // Renumber the basic blocks if so. - // - if (madeFlowChanges) - { - JITDUMP("\nRenumbering the basic blocks for fgPostImportationCleanup\n"); - fgRenumberBlocks(); - } - #ifdef DEBUG fgVerifyHandlerTab(); #endif // DEBUG // Did we make any changes? // - const bool madeChanges = madeFlowChanges || addedTemps; + const bool madeChanges = (addedBlocks > 0) || (delCnt > 0) || (removedBlks > 0) || addedTemps; // Note that we have now run post importation cleanup, // so we can enable more stringent checking. diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 10f929a9cd74c..52df8bab32b4f 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -152,13 +152,8 @@ PhaseStatus Compiler::fgInsertGCPolls() block = curBasicBlock; } - // If we split a block to create a GC Poll, call fgUpdateChangedFlowGraph. // We should never split blocks unless we're optimizing. - if (createdPollBlocks) - { - noway_assert(opts.OptimizationEnabled()); - fgRenumberBlocks(); - } + assert(!createdPollBlocks || opts.OptimizationEnabled()); return result; } diff --git a/src/coreclr/jit/gschecks.cpp b/src/coreclr/jit/gschecks.cpp index 06802181eea22..ed2d7538a3978 100644 --- a/src/coreclr/jit/gschecks.cpp +++ b/src/coreclr/jit/gschecks.cpp @@ -35,12 +35,6 @@ PhaseStatus Compiler::gsPhase() gsCopyShadowParams(); } - // If we needed to create any new BasicBlocks then renumber the blocks - if (fgBBcount > prevBBCount) - { - fgRenumberBlocks(); - } - madeChanges = true; } else diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index 4a30e7325e2af..56fd853acb0d6 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -1225,11 +1225,6 @@ PhaseStatus Compiler::fgExpandHelper(bool skipRarelyRunBlocks) if (result == PhaseStatus::MODIFIED_EVERYTHING) { fgInvalidateDfsTree(); - - if (opts.OptimizationEnabled()) - { - fgRenumberBlocks(); - } } return result; diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index e3d1714702404..96bc754d31716 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -2123,8 +2123,8 @@ void Compiler::fgNormalizeEH() if (modified) { JITDUMP("Added at least one basic block in fgNormalizeEH.\n"); - fgRenumberBlocks(); - // fgRenumberBlocks() will dump all the blocks and the handler table, so we don't need to do it here. + JITDUMPEXEC(fgDispBasicBlocks()); + JITDUMPEXEC(fgDispHandlerTab()); INDEBUG(fgVerifyHandlerTab()); } else diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index d4646254fa653..57ff28ecb06b5 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -40,6 +40,7 @@ DataFlow::DataFlow(Compiler* pCompiler) PhaseStatus Compiler::optSetBlockWeights() { noway_assert(opts.OptimizationEnabled()); + bool madeChanges = false; assert(m_dfsTree != nullptr); if (m_domTree == nullptr) @@ -53,11 +54,11 @@ PhaseStatus Compiler::optSetBlockWeights() if (m_dfsTree->HasCycle()) { + madeChanges = fgRenumberBlocks(); optMarkLoopHeads(); optFindAndScaleGeneralLoopBlocks(); } - bool madeChanges = false; bool firstBBDominatesAllReturns = true; const bool usingProfileWeights = fgIsUsingProfileWeights(); @@ -1335,13 +1336,11 @@ PhaseStatus Compiler::optUnrollLoops() m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); } - fgRenumberBlocks(); - DBEXEC(verbose, fgDispBasicBlocks()); } #ifdef DEBUG - fgDebugCheckBBlist(true); + fgDebugCheckBBlist(); #endif // DEBUG return anyIRchange ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; @@ -2687,8 +2686,6 @@ PhaseStatus Compiler::optFindLoopsPhase() } #endif - fgRenumberBlocks(); - assert(m_dfsTree != nullptr); optFindLoops(); @@ -2713,8 +2710,6 @@ void Compiler::optFindLoops() m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); } - fgRenumberBlocks(); - // Starting now we require all loops to be in canonical form. optLoopsCanonical = true;