JIT: stop computing dom start nodes for morph RPO (#94497)

Remove the up-front computation of enter blocks and dom
start nodes from the DFS computations used for RPO.

Also lay the groundwork for removing unreachable blocks, by tracking
the postorder number of the last reachable block.

Contributes to #93246.

Author: AndyAyersMS

src/coreclr/jit/compiler.h

@@ -5329,15 +5329,12 @@ class Compiler
 
     BasicBlock* fgIntersectDom(BasicBlock* a, BasicBlock* b); // Intersect two immediate dominator sets.
 
-    void fgDfsReversePostorder();
+    unsigned fgDfsReversePostorder();
     void fgDfsReversePostorderHelper(BasicBlock* block,
                                      BlockSet&   visited,
                                      unsigned&   preorderIndex,
                                      unsigned&   reversePostorderIndex);
 
-    BlockSet_ValRet_T fgDomFindStartNodes(); // Computes which basic blocks don't have incoming edges in the flow graph.
-                                             // Returns this as a set.
-
     INDEBUG(void fgDispDomTree(DomTreeNode* domTree);) // Helper that prints out the Dominator Tree in debug builds.
 
     DomTreeNode* fgBuildDomTree(); // Once we compute all the immediate dominator sets for each node in the flow graph

src/coreclr/jit/fgopt.cpp

@@ -763,59 +763,87 @@ bool Compiler::fgRemoveDeadBlocks()
 
 //-------------------------------------------------------------
 // fgDfsReversePostorder: Depth first search to establish block
-//   preorder and reverse postorder numbers, plus a reverse postorder for blocks.
+//   preorder and reverse postorder numbers, plus a reverse postorder for blocks,
+//   using all entry blocks and EH handler blocks as start blocks.
 //
 // Notes:
-//   Assumes caller has computed the fgEnterBlks set.
-//
 //   Each block's `bbPreorderNum` and `bbPostorderNum` is set.
-//
 //   The `fgBBReversePostorder` array is filled in with the `BasicBlock*` in reverse post-order.
-//
 //   This algorithm only pays attention to the actual blocks. It ignores any imaginary entry block.
 //
-void Compiler::fgDfsReversePostorder()
+//   Unreachable blocks will have higher pre and post order numbers than reachable blocks.
+//   Hence they will appear at lower indices in the fgBBReversePostorder array.
+//
+unsigned Compiler::fgDfsReversePostorder()
 {
     assert(fgBBcount == fgBBNumMax);
     assert(BasicBlockBitSetTraits::GetSize(this) == fgBBNumMax + 1);
-
-    // Make sure fgEnterBlks are still there in startNodes, even if they participate in a loop (i.e., there is
-    // an incoming edge into the block).
-    assert(fgEnterBlksSetValid);
-
     fgBBReversePostorder = new (this, CMK_DominatorMemory) BasicBlock*[fgBBNumMax + 1]{};
-
-    // visited   :  Once we run the DFS post order sort recursive algorithm, we mark the nodes we visited to avoid
-    //              backtracking.
     BlockSet visited(BlockSetOps::MakeEmpty(this));
 
-    // We begin by figuring out which basic blocks don't have incoming edges and mark them as
-    // start nodes.  Later on we run the recursive algorithm for each node that we
-    // mark in this step.
-    BlockSet_ValRet_T startNodes = fgDomFindStartNodes();
-
-    BlockSetOps::UnionD(this, startNodes, fgEnterBlks);
-    assert(BlockSetOps::IsMember(this, startNodes, fgFirstBB->bbNum));
-
-    // Call the flowgraph DFS traversal helper.
     unsigned preorderIndex  = 1;
     unsigned postorderIndex = 1;
-    for (BasicBlock* const block : Blocks())
+
+    // Walk from our primary root.
+    //
+    fgDfsReversePostorderHelper(fgFirstBB, visited, preorderIndex, postorderIndex);
+
+    // If we didn't end up visiting everything, try the EH roots.
+    //
+    if (preorderIndex != fgBBcount + 1)
     {
-        // If the block has no predecessors, and we haven't already visited it (because it's in fgEnterBlks but also
-        // reachable from the first block), go ahead and traverse starting from this block.
-        if (BlockSetOps::IsMember(this, startNodes, block->bbNum) &&
-            !BlockSetOps::IsMember(this, visited, block->bbNum))
+        for (EHblkDsc* const HBtab : EHClauses(this))
         {
-            fgDfsReversePostorderHelper(block, visited, preorderIndex, postorderIndex);
+            if (HBtab->HasFilter())
+            {
+                BasicBlock* const filterBlock = HBtab->ebdFilter;
+                if (!BlockSetOps::IsMember(this, visited, filterBlock->bbNum))
+                {
+                    fgDfsReversePostorderHelper(filterBlock, visited, preorderIndex, postorderIndex);
+                }
+            }
+
+            BasicBlock* const handlerBlock = HBtab->ebdHndBeg;
+            if (!BlockSetOps::IsMember(this, visited, handlerBlock->bbNum))
+            {
+                fgDfsReversePostorderHelper(handlerBlock, visited, preorderIndex, postorderIndex);
+            }
+
+#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
+            // For ARM code, prevent creating retless calls by visiting the call finally successors
+            //
+            if (HBtab->HasFinallyHandler())
+            {
+                for (BasicBlock* const finallyPredBlock : handlerBlock->PredBlocks())
+                {
+                    // In some rare cases the finally is a loop entry.
+                    //
+                    if (!finallyPredBlock->KindIs(BBJ_CALLFINALLY))
+                    {
+                        continue;
+                    }
+                    assert(finallyPredBlock->isBBCallAlwaysPair());
+                    BasicBlock* const pairTailBlock = finallyPredBlock->Next();
+
+                    if (!BlockSetOps::IsMember(this, visited, pairTailBlock->bbNum))
+                    {
+                        fgDfsReversePostorderHelper(pairTailBlock, visited, preorderIndex, postorderIndex);
+                    }
+                }
+            }
+#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
         }
     }
 
-    // If there are still unvisited blocks (say isolated cycles), visit them too.
+    // That's everything reachable from the roots.
     //
-    if (preorderIndex != fgBBcount + 1)
+    const unsigned highestReachablePostorderNumber = postorderIndex - 1;
+
+    // If we still didn't end up visiting everything, visit what remains.
+    //
+    if (highestReachablePostorderNumber != fgBBcount)
     {
-        JITDUMP("DFS: flow graph has some isolated cycles, doing extra traversals\n");
+        JITDUMP("DFS: there are %u unreachable blocks\n", fgBBcount - highestReachablePostorderNumber);
         for (BasicBlock* const block : Blocks())
         {
             if (!BlockSetOps::IsMember(this, visited, block->bbNum))
@@ -841,48 +869,12 @@ void Compiler::fgDfsReversePostorder()
         printf("\n");
     }
 #endif // DEBUG
-}
-
-//-------------------------------------------------------------
-// fgDomFindStartNodes: Helper for dominance computation to find the start nodes block set.
-//
-// The start nodes is a set that represents which basic blocks in the flow graph don't have incoming edges.
-// We begin assuming everything is a start block and remove any block that is a successor of another.
-//
-// Returns:
-//    Block set of start nodes.
-//
-BlockSet_ValRet_T Compiler::fgDomFindStartNodes()
-{
-    BlockSet startNodes(BlockSetOps::MakeFull(this));
-
-    for (BasicBlock* const block : Blocks())
-    {
-        for (BasicBlock* const succ : block->Succs(this))
-        {
-            BlockSetOps::RemoveElemD(this, startNodes, succ->bbNum);
-        }
-    }
-
-#ifdef DEBUG
-    if (verbose)
-    {
-        printf("\nDominator computation start blocks (those blocks with no incoming edges):\n");
-        BlockSetOps::Iter iter(this, startNodes);
-        unsigned          bbNum = 0;
-        while (iter.NextElem(&bbNum))
-        {
-            printf(FMT_BB " ", bbNum);
-        }
-        printf("\n");
-    }
-#endif // DEBUG
 
-    return startNodes;
+    return highestReachablePostorderNumber;
 }
 
 //------------------------------------------------------------------------
-// fgDfsReversePostorderHelper: Helper to assign post-order numbers to blocks.
+// fgDfsReversePostorderHelper: Helper to assign post-order numbers to blocks
 //
 // Arguments:
 //    block   - The starting entry block

src/coreclr/jit/fgprofilesynthesis.cpp

@@ -790,7 +790,6 @@ void ProfileSynthesis::RandomizeLikelihoods()
 void ProfileSynthesis::BuildReversePostorder()
 {
     m_comp->EnsureBasicBlockEpoch();
-    m_comp->fgComputeEnterBlocksSet();
     m_comp->fgDfsReversePostorder();
 
     // Build map from bbNum to Block*.

src/coreclr/jit/morph.cpp

@@ -13938,8 +13938,6 @@ PhaseStatus Compiler::fgMorphBlocks()
         // We are optimizing. Process in RPO.
         //
         fgRenumberBlocks();
-        EnsureBasicBlockEpoch();
-        fgComputeEnterBlocksSet();
         fgDfsReversePostorder();
 
         // Disallow general creation of new blocks or edges as it
@@ -13963,7 +13961,12 @@ PhaseStatus Compiler::fgMorphBlocks()
             fgFirstBB->Next()->bbFlags |= BBF_CAN_ADD_PRED;
         }
 
+        // Remember this so we can sanity check that no new blocks will get created.
+        //
         unsigned const bbNumMax = fgBBNumMax;
+
+        // Morph the blocks in RPO.
+        //
         for (unsigned i = 1; i <= bbNumMax; i++)
         {
             BasicBlock* const block = fgBBReversePostorder[i];