From 1473deaa50785b956edd7d078e68c0581c1b4d95 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 9 Dec 2023 21:03:19 +0100 Subject: [PATCH] JIT: Reuse (new) loop-finding DFS tree in SSA (#95809) * Add a new phase to canonicalize the entry BB, way back before we compute reachability * Switch SSA to reuse the DFS computed for (new) loop finding * Switch SSA to use post-order traits instead of bbNum-based traits * Quirk old loop finding to avoid finding new loops now --- src/coreclr/jit/compiler.cpp | 4 +++ src/coreclr/jit/compiler.h | 4 +++ src/coreclr/jit/compphases.h | 1 + src/coreclr/jit/fgbasic.cpp | 1 + src/coreclr/jit/optimizer.cpp | 34 ++++++++++++++++++- src/coreclr/jit/ssabuilder.cpp | 60 ++-------------------------------- src/coreclr/jit/ssabuilder.h | 7 +--- 7 files changed, 47 insertions(+), 64 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index f3eee4ec87bcd..8243e16658dc6 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4807,6 +4807,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_HEAD_TAIL_MERGE2, [this]() { return fgHeadTailMerge(false); }); + // Canonicalize entry to give a unique dominator tree root + // + DoPhase(this, PHASE_CANONICALIZE_ENTRY, &Compiler::fgCanonicalizeFirstBB); + // Compute reachability sets and dominators. // DoPhase(this, PHASE_COMPUTE_REACHABILITY, &Compiler::fgComputeReachability); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 024811667394d..54ab00f01ff50 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5097,11 +5097,13 @@ class Compiler bool fgDomsComputed; // Have we computed the dominator sets? bool fgReturnBlocksComputed; // Have we computed the return blocks list? bool fgOptimizedFinally; // Did we optimize any try-finallys? + bool fgCanonicalizedFirstBB; // Quirk: did we end up canonicalizing first BB? bool fgHasSwitch; // any BBJ_SWITCH jumps? BlockSet fgEnterBlks; // Set of blocks which have a special transfer of control; the "entry" blocks plus EH handler // begin blocks. + #ifdef DEBUG bool fgReachabilitySetsValid; // Are the bbReach sets valid? bool fgEnterBlksSetValid; // Is the fgEnterBlks set valid? @@ -5948,6 +5950,8 @@ class Compiler bool fgCheckRemoveStmt(BasicBlock* block, Statement* stmt); + PhaseStatus fgCanonicalizeFirstBB(); + bool fgCreateLoopPreHeader(unsigned lnum); void fgSetEHRegionForNewLoopHead(BasicBlock* newHead, BasicBlock* top); diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 8e6b7123ea5b7..73e0eeac6c9a3 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -64,6 +64,7 @@ CompPhaseNameMacro(PHASE_HEAD_TAIL_MERGE2, "Post-morph head and tail m CompPhaseNameMacro(PHASE_OPTIMIZE_FLOW, "Optimize control flow", false, -1, false) CompPhaseNameMacro(PHASE_OPTIMIZE_LAYOUT, "Optimize layout", false, -1, false) CompPhaseNameMacro(PHASE_COMPUTE_REACHABILITY, "Compute blocks reachability", false, -1, false) +CompPhaseNameMacro(PHASE_CANONICALIZE_ENTRY, "Canonicalize entry", false, -1, false) CompPhaseNameMacro(PHASE_SET_BLOCK_WEIGHTS, "Set block weights", false, -1, false) CompPhaseNameMacro(PHASE_ZERO_INITS, "Redundant zero Inits", false, -1, false) CompPhaseNameMacro(PHASE_FIND_LOOPS, "Find loops", false, -1, false) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 51f3f71d016a0..0d31e9f201197 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -183,6 +183,7 @@ void Compiler::fgInit() fgCountInstrumentor = nullptr; fgHistogramInstrumentor = nullptr; fgPredListSortVector = nullptr; + fgCanonicalizedFirstBB = false; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 0af1677258ce1..9322e909e1beb 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2458,7 +2458,10 @@ void Compiler::optFindNaturalLoops() LoopSearch search(this); - for (BasicBlock* head = fgFirstBB; !head->IsLast(); head = head->Next()) + // TODO-Quirk: Remove + BasicBlock* first = fgCanonicalizedFirstBB ? fgFirstBB->Next() : fgFirstBB; + + for (BasicBlock* head = first; !head->IsLast(); head = head->Next()) { BasicBlock* top = head->Next(); @@ -7944,6 +7947,35 @@ void Compiler::fgSetEHRegionForNewLoopHead(BasicBlock* newHead, BasicBlock* top) } } +//------------------------------------------------------------------------------ +// fgCanonicalizeFirstBB: Canonicalize the method entry for loop and dominator +// purposes. +// +// Returns: +// Suitable phase status. +// +PhaseStatus Compiler::fgCanonicalizeFirstBB() +{ + if (fgFirstBB->hasTryIndex()) + { + JITDUMP("Canonicalizing entry because it currently is the beginning of a try region\n"); + } + else if (fgFirstBB->bbPreds != nullptr) + { + JITDUMP("Canonicalizing entry because it currently has predecessors\n"); + } + else + { + return PhaseStatus::MODIFIED_NOTHING; + } + + assert(!fgFirstBBisScratch()); + fgEnsureFirstBBisScratch(); + // TODO-Quirk: Remove + fgCanonicalizedFirstBB = true; + return PhaseStatus::MODIFIED_EVERYTHING; +} + //------------------------------------------------------------------------------ // fgCreateLoopPreHeader: Creates a pre-header block for the given loop. // A pre-header is a block outside the loop that falls through or branches to the loop diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index 5eaeae1c7ac51..77a22759d18d2 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -224,7 +224,7 @@ void SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock* b, const BlkToBlkV for (BasicBlock* f : *bDF) { - BitVecOps::AddElemD(&m_visitedTraits, m_visited, f->bbNum); + BitVecOps::AddElemD(&m_visitedTraits, m_visited, f->bbNewPostorderNum); bIDF->push_back(f); } @@ -242,7 +242,7 @@ void SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock* b, const BlkToBlkV { for (BasicBlock* ff : *fDF) { - if (BitVecOps::TryAddElemD(&m_visitedTraits, m_visited, ff->bbNum)) + if (BitVecOps::TryAddElemD(&m_visitedTraits, m_visited, ff->bbNewPostorderNum)) { bIDF->push_back(ff); } @@ -1291,20 +1291,9 @@ void SsaBuilder::Build() } #endif - // Ensure that there's a first block outside a try, so that the dominator tree has a unique root. - SetupBBRoot(); - - // Just to keep block no. & index same add 1. - int blockCount = m_pCompiler->fgBBNumMax + 1; - - JITDUMP("[SsaBuilder] Max block count is %d.\n", blockCount); - - // Allocate the postOrder array for the graph. - - m_visitedTraits = BitVecTraits(blockCount, m_pCompiler); + m_visitedTraits = m_pCompiler->m_dfsTree->PostOrderTraits(); m_visited = BitVecOps::MakeEmpty(&m_visitedTraits); - m_pCompiler->m_dfsTree = m_pCompiler->fgComputeDfs(); m_pCompiler->fgSsaDomTree = FlowGraphDominatorTree::Build(m_pCompiler->m_dfsTree); EndPhase(PHASE_BUILD_SSA_DOMS); @@ -1331,49 +1320,6 @@ void SsaBuilder::Build() JITDUMPEXEC(m_pCompiler->DumpSsaSummary()); } -void SsaBuilder::SetupBBRoot() -{ - assert(m_pCompiler->fgPredsComputed); - - // Allocate a bbroot, if necessary. - // We need a unique block to be the root of the dominator tree. - // This can be violated if the first block is in a try, or if it is the first block of - // a loop (which would necessarily be an infinite loop) -- i.e., it has a predecessor. - - // If neither condition holds, no reason to make a new block. - if (!m_pCompiler->fgFirstBB->hasTryIndex() && m_pCompiler->fgFirstBB->bbPreds == nullptr) - { - return; - } - - BasicBlock* bbRoot = BasicBlock::New(m_pCompiler, BBJ_ALWAYS, m_pCompiler->fgFirstBB); - bbRoot->SetFlags(BBF_INTERNAL | BBF_NONE_QUIRK); - - // May need to fix up preds list, so remember the old first block. - BasicBlock* oldFirst = m_pCompiler->fgFirstBB; - - // Copy the liveness information from the first basic block. - if (m_pCompiler->fgLocalVarLivenessDone) - { - VarSetOps::Assign(m_pCompiler, bbRoot->bbLiveIn, oldFirst->bbLiveIn); - VarSetOps::Assign(m_pCompiler, bbRoot->bbLiveOut, oldFirst->bbLiveIn); - } - - // Copy the bbWeight. (This is technically wrong, if the first block is a loop head, but - // it shouldn't matter...) - bbRoot->inheritWeight(oldFirst); - - // There's an artificial incoming reference count for the first BB. We're about to make it no longer - // the first BB, so decrement that. - assert(oldFirst->bbRefs > 0); - oldFirst->bbRefs--; - - m_pCompiler->fgInsertBBbefore(m_pCompiler->fgFirstBB, bbRoot); - - assert(m_pCompiler->fgFirstBB == bbRoot); - m_pCompiler->fgAddRefPred(oldFirst, bbRoot); -} - #ifdef DEBUG //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/ssabuilder.h b/src/coreclr/jit/ssabuilder.h index 25a4f7ec3e183..d07e59e4e732c 100644 --- a/src/coreclr/jit/ssabuilder.h +++ b/src/coreclr/jit/ssabuilder.h @@ -34,11 +34,6 @@ class SsaBuilder void Build(); private: - // Ensures that the basic block graph has a root for the dominator graph, by ensuring - // that there is a first block that is not in a try region (adding an empty block for that purpose - // if necessary). Eventually should move to Compiler. - void SetupBBRoot(); - // Compute flow graph dominance frontiers. void ComputeDominanceFrontiers(BasicBlock** postOrder, int count, BlkToBlkVectorMap* mapDF); @@ -87,7 +82,7 @@ class SsaBuilder Compiler* m_pCompiler; CompAllocator m_allocator; - // Bit vector used by TopologicalSort and ComputeImmediateDom to track already visited blocks. + // Bit vector used by ComputeImmediateDom to track already visited blocks. BitVecTraits m_visitedTraits; BitVec m_visited;