From a4c3369052f1557617787e8348e5753e1a3f57bd Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 10 Sep 2024 13:42:23 -0400 Subject: [PATCH 1/4] Run new block layout once --- src/coreclr/jit/compiler.cpp | 24 ++++++++++-------------- src/coreclr/jit/fgopt.cpp | 17 ++++------------- src/coreclr/jit/lower.cpp | 4 +--- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 2b9425e6fe5025..e6b15d8de42e83 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5177,10 +5177,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // Conditional to Switch conversion // DoPhase(this, PHASE_SWITCH_RECOGNITION, &Compiler::optSwitchRecognition); - - // Determine start of cold region if we are hot/cold splitting - // - DoPhase(this, PHASE_DETERMINE_FIRST_COLD_BLOCK, &Compiler::fgDetermineFirstColdBlock); } #ifdef DEBUG @@ -5236,8 +5232,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl m_pLowering->FinalizeOutgoingArgSpace(); // We can not add any new tracked variables after this point. - lvaTrackedFixed = true; - const unsigned numBlocksBeforeLSRA = fgBBcount; + lvaTrackedFixed = true; // Now that lowering is completed we can proceed to perform register allocation // @@ -5251,8 +5246,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl if (opts.OptimizationEnabled()) { - // LSRA may introduce new blocks. If it does, rerun layout. - if ((fgBBcount != numBlocksBeforeLSRA) && JitConfig.JitDoReversePostOrderLayout()) + // We won't introduce new blocks from here on out, + // so run the new block layout. + // + if (JitConfig.JitDoReversePostOrderLayout()) { auto lateLayoutPhase = [this] { fgDoReversePostOrderLayout(); @@ -5261,16 +5258,15 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl }; DoPhase(this, PHASE_OPTIMIZE_LAYOUT, lateLayoutPhase); - - if (fgFirstColdBlock != nullptr) - { - fgFirstColdBlock = nullptr; - DoPhase(this, PHASE_DETERMINE_FIRST_COLD_BLOCK, &Compiler::fgDetermineFirstColdBlock); - } } // Now that the flowgraph is finalized, run post-layout optimizations. + // DoPhase(this, PHASE_OPTIMIZE_POST_LAYOUT, &Compiler::optOptimizePostLayout); + + // Determine start of cold region if we are hot/cold splitting + // + DoPhase(this, PHASE_DETERMINE_FIRST_COLD_BLOCK, &Compiler::fgDetermineFirstColdBlock); } #if FEATURE_LOOP_ALIGN diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 5a71d474b6f365..4630780865d7e6 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -3336,25 +3336,16 @@ bool Compiler::fgReorderBlocks(bool useProfile) if (fgIsUsingProfileWeights()) { optimizedSwitches = fgOptimizeSwitchJumps(); - if (optimizedSwitches) - { - fgUpdateFlowGraph(); - } } if (useProfile) { + // Don't run the new layout until we get to the backend, + // since LSRA can introduce new blocks, and lowering can churn the flowgraph. + // if (JitConfig.JitDoReversePostOrderLayout()) { - fgDoReversePostOrderLayout(); - fgMoveColdBlocks(); - - // Renumber blocks to facilitate LSRA's order of block visitation - // TODO: Consider removing this, and using traversal order in lSRA - // - fgRenumberBlocks(); - - return true; + return (newRarelyRun || movedBlocks || optimizedSwitches); } // We will be reordering blocks, so ensure the false target of a BBJ_COND block is its next block diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index d92895a9d49804..6d85ecf90c28cc 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7858,9 +7858,7 @@ PhaseStatus Lowering::DoPhase() comp->fgLocalVarLiveness(); // local var liveness can delete code, which may create empty blocks - // Don't churn the flowgraph with aggressive compaction since we've already run block layout - bool modified = comp->fgUpdateFlowGraph(/* doTailDuplication */ false, /* isPhase */ false, - /* doAggressiveCompaction */ false); + bool modified = comp->fgUpdateFlowGraph(/* doTailDuplication */ false, /* isPhase */ false); if (modified) { From e86c9504e6127761a2e833f09df17752b451c24e Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 10 Sep 2024 15:29:54 -0400 Subject: [PATCH 2/4] Add back fgUpdateFlowGraph call --- src/coreclr/jit/fgopt.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 4630780865d7e6..b11f6aa2edf594 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -3336,6 +3336,10 @@ bool Compiler::fgReorderBlocks(bool useProfile) if (fgIsUsingProfileWeights()) { optimizedSwitches = fgOptimizeSwitchJumps(); + if (optimizedSwitches) + { + fgUpdateFlowGraph(); + } } if (useProfile) From 8cddc67069dd38cff68a3e7a7571e93ec24d7d41 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 10 Sep 2024 15:33:42 -0400 Subject: [PATCH 3/4] Remove compaction change --- src/coreclr/jit/lower.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 6d85ecf90c28cc..52853b016e4292 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7858,8 +7858,9 @@ PhaseStatus Lowering::DoPhase() comp->fgLocalVarLiveness(); // local var liveness can delete code, which may create empty blocks - bool modified = comp->fgUpdateFlowGraph(/* doTailDuplication */ false, /* isPhase */ false); - + // Don't churn the flowgraph with aggressive compaction since we've already run block layout + bool modified = comp->fgUpdateFlowGraph(/* doTailDuplication */ false, /* isPhase */ false, + /* doAggressiveCompaction */ false); if (modified) { comp->fgDfsBlocksAndRemove(); From 217c1737088dfe0c9dfeb3fe9c31c178e72d94c7 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 20 Sep 2024 15:35:31 -0400 Subject: [PATCH 4/4] Undo whitespace change --- src/coreclr/jit/lower.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 52853b016e4292..d92895a9d49804 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7861,6 +7861,7 @@ PhaseStatus Lowering::DoPhase() // Don't churn the flowgraph with aggressive compaction since we've already run block layout bool modified = comp->fgUpdateFlowGraph(/* doTailDuplication */ false, /* isPhase */ false, /* doAggressiveCompaction */ false); + if (modified) { comp->fgDfsBlocksAndRemove();