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

@@ -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);

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);

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)

src/coreclr/jit/fgbasic.cpp

@@ -183,6 +183,7 @@ void Compiler::fgInit()
     fgCountInstrumentor          = nullptr;
     fgHistogramInstrumentor      = nullptr;
     fgPredListSortVector         = nullptr;
+    fgCanonicalizedFirstBB       = false;
 }
 
 //------------------------------------------------------------------------

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

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
 
 //------------------------------------------------------------------------

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;