JIT: Add a "DFS blocks" phase

src/coreclr/jit/block.h

@@ -2126,7 +2126,7 @@ class AllSuccessorEnumerator
     }
 
     // Returns the next available successor or `nullptr` if there are no more successors.
-    BasicBlock* NextSuccessor(Compiler* comp)
+    BasicBlock* NextSuccessor()
     {
         m_curSucc++;
         if (m_curSucc >= m_numSuccs)

src/coreclr/jit/compiler.cpp

@@ -4696,6 +4696,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
         // Run an early flow graph simplification pass
         //
         DoPhase(this, PHASE_EARLY_UPDATE_FLOW_GRAPH, &Compiler::fgUpdateFlowGraphPhase);
+
+        // Build post-order
+        //
+        DoPhase(this, PHASE_DFS_BLOCKS, &Compiler::fgDfsBlocks);
     }
 
     // Promote struct locals
@@ -4778,6 +4782,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
         fgRenumberBlocks();
     }
 
+    if (opts.OptimizationEnabled())
+    {
+        // Build post-order
+        //
+        DoPhase(this, PHASE_DFS_BLOCKS, &Compiler::fgDfsBlocks);
+    }
+
     // GS security checks for unsafe buffers
     //
     DoPhase(this, PHASE_GS_COOKIE, &Compiler::gsPhase);

src/coreclr/jit/compiler.h

@@ -4493,8 +4493,8 @@ class Compiler
     unsigned     fgBBNumMax;           // The max bbNum that has been assigned to basic blocks
     unsigned     fgDomBBcount;         // # of BBs for which we have dominator and reachability information
     BasicBlock** fgBBReversePostorder; // Blocks in reverse postorder
-    BasicBlock** fgSSAPostOrder;       // Blocks in postorder, computed during SSA
-    unsigned     fgSSAPostOrderCount;  // Number of blocks in fgSSAPostOrder
+    BasicBlock** fgPostOrder;          // Blocks in post order
+    unsigned     fgPostOrderCount;     // Number of blocks in fgSSAPostOrder
 
     // After the dominance tree is computed, we cache a DFS preorder number and DFS postorder number to compute
     // dominance queries in O(1). fgDomTreePreOrder and fgDomTreePostOrder are arrays giving the block's preorder and
@@ -5609,6 +5609,8 @@ class Compiler
 
     PhaseStatus fgSetBlockOrder();
 
+    PhaseStatus fgDfsBlocks();
+
     void fgRemoveReturnBlock(BasicBlock* block);
 
     void fgConvertBBToThrowBB(BasicBlock* block);

src/coreclr/jit/compphases.h

@@ -76,8 +76,8 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_BOOLS,             "Optimize bools",
 CompPhaseNameMacro(PHASE_SWITCH_RECOGNITION,         "Recognize Switch",               false, -1, false)
 CompPhaseNameMacro(PHASE_FIND_OPER_ORDER,            "Find oper order",                false, -1, false)
 CompPhaseNameMacro(PHASE_SET_BLOCK_ORDER,            "Set block order",                false, -1, true)
+CompPhaseNameMacro(PHASE_DFS_BLOCKS,                 "Set block order from DFS",       false, -1, false)
 CompPhaseNameMacro(PHASE_BUILD_SSA,                  "Build SSA representation",       true,  -1, false)
-CompPhaseNameMacro(PHASE_BUILD_SSA_TOPOSORT,         "SSA: topological sort",          false, PHASE_BUILD_SSA, false)
 CompPhaseNameMacro(PHASE_BUILD_SSA_DOMS,             "SSA: Doms1",                     false, PHASE_BUILD_SSA, false)
 CompPhaseNameMacro(PHASE_BUILD_SSA_LIVENESS,         "SSA: liveness",                  false, PHASE_BUILD_SSA, false)
 CompPhaseNameMacro(PHASE_BUILD_SSA_DF,               "SSA: DF",                        false, PHASE_BUILD_SSA, false)

src/coreclr/jit/fgdiagnostic.cpp

@@ -2976,6 +2976,7 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
     bool allNodesLinked = (fgNodeThreading == NodeThreading::AllTrees) || (fgNodeThreading == NodeThreading::LIR);
 
     unsigned numBlocks = 0;
+    unsigned maxBBNum  = 0;
 
     for (BasicBlock* const block : Blocks())
     {
@@ -2987,6 +2988,8 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
             assert(block->IsLast() || (block->bbNum + 1 == block->Next()->bbNum));
         }
 
+        maxBBNum = max(maxBBNum, block->bbNum);
+
         // Check that all the successors have the current traversal stamp. Use the 'Compiler*' version of the
         // iterator, but not for BBJ_SWITCH: we don't want to end up calling GetDescriptorForSwitch(), which will
         // dynamically create the unique switch list.
@@ -3190,6 +3193,7 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
     }
 
     assert(fgBBcount == numBlocks);
+    assert(fgBBNumMax >= maxBBNum);
 
     // Make sure the one return BB is not changed.
     if (genReturnBB != nullptr)

src/coreclr/jit/fgopt.cpp

@@ -544,6 +544,94 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock)
     return changed;
 }
 
+PhaseStatus Compiler::fgDfsBlocks()
+{
+    fgPostOrder = new (this, CMK_BasicBlock) BasicBlock*[fgBBcount];
+    BitVecTraits traits(fgBBNumMax + 1, this);
+
+    BitVec visited(BitVecOps::MakeEmpty(&traits));
+
+    unsigned preOrderIndex  = 0;
+    unsigned postOrderIndex = 0;
+
+    ArrayStack<AllSuccessorEnumerator> blocks(getAllocator(CMK_BasicBlock));
+
+    auto dfsFrom = [&](BasicBlock* firstBB) {
+
+        BitVecOps::AddElemD(&traits, visited, firstBB->bbNum);
+        blocks.Emplace(this, firstBB);
+        firstBB->bbPreorderNum = preOrderIndex++;
+
+        while (!blocks.Empty())
+        {
+            BasicBlock* block = blocks.TopRef().Block();
+            BasicBlock* succ  = blocks.TopRef().NextSuccessor();
+
+            if (succ != nullptr)
+            {
+                if (BitVecOps::TryAddElemD(&traits, visited, succ->bbNum))
+                {
+                    blocks.Emplace(this, succ);
+                    succ->bbPreorderNum = preOrderIndex++;
+                }
+            }
+            else
+            {
+                blocks.Pop();
+                fgPostOrder[postOrderIndex] = block;
+                block->bbPostorderNum       = postOrderIndex++;
+            }
+        }
+
+    };
+
+    dfsFrom(fgFirstBB);
+
+    if ((fgEntryBB != nullptr) && !BitVecOps::IsMember(&traits, visited, fgEntryBB->bbNum))
+    {
+        // OSR methods will early on create flow that looks like it goes to the
+        // patchpoint, but during morph we may transform to something that
+        // requires the original entry (fgEntryBB).
+        assert(opts.IsOSR());
+        assert((fgEntryBB->bbRefs == 1) && (fgEntryBB->bbPreds == nullptr));
+        dfsFrom(fgEntryBB);
+    }
+
+    if ((genReturnBB != nullptr) && !BitVecOps::IsMember(&traits, visited, genReturnBB->bbNum) && !fgGlobalMorphDone)
+    {
+        // We introduce the merged return BB before morph and will redirect
+        // other returns to it as part of morph; keep it reachable.
+        dfsFrom(genReturnBB);
+    }
+
+    PhaseStatus status = PhaseStatus::MODIFIED_NOTHING;
+    if (postOrderIndex != fgBBcount)
+    {
+#ifdef DEBUG
+        if (verbose)
+        {
+            printf("%u/%u blocks are unreachable and will be removed\n", fgBBcount - postOrderIndex, fgBBcount);
+            for (BasicBlock* block : Blocks())
+            {
+                if (!BitVecOps::IsMember(&traits, visited, block->bbNum))
+                {
+                    printf("  " FMT_BB "\n", block->bbNum);
+                }
+            }
+        }
+#endif
+
+        fgRemoveUnreachableBlocks(
+            [=, &traits](BasicBlock* block) { return !BitVecOps::IsMember(&traits, visited, block->bbNum); });
+
+        status = PhaseStatus::MODIFIED_EVERYTHING;
+    }
+
+    fgPostOrderCount = postOrderIndex;
+
+    return status;
+}
+
 //------------------------------------------------------------------------
 // fgComputeReachability: Compute the dominator and reachable sets.
 //

src/coreclr/jit/morph.cpp

@@ -14124,8 +14124,9 @@ PhaseStatus Compiler::fgMorphBlocks()
 
     // We are done with the global morphing phase
     //
-    fgGlobalMorph = false;
-    compCurBB     = nullptr;
+    fgGlobalMorph     = false;
+    fgGlobalMorphDone = true;
+    compCurBB         = nullptr;
 
 #ifdef DEBUG
     if (optLocalAssertionProp)

src/coreclr/jit/ssabuilder.cpp

@@ -69,8 +69,6 @@ PhaseStatus Compiler::fgSsaBuild()
     JitTestCheckSSA();
 #endif // DEBUG
 
-    fgSSAPostOrder = builder.GetPostOrder(&fgSSAPostOrderCount);
-
     return PhaseStatus::MODIFIED_EVERYTHING;
 }
 
@@ -136,88 +134,6 @@ SsaBuilder::SsaBuilder(Compiler* pCompiler)
 {
 }
 
-//------------------------------------------------------------------------
-//  TopologicalSort: Topologically sort the graph and return the number of nodes visited.
-//
-//  Arguments:
-//     postOrder - The array in which the arranged basic blocks have to be returned.
-//     count - The size of the postOrder array.
-//
-//  Return Value:
-//     The number of nodes visited while performing DFS on the graph.
-//
-unsigned SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
-{
-    Compiler* comp = m_pCompiler;
-
-    // TopologicalSort is called first so m_visited should already be empty
-    assert(BitVecOps::IsEmpty(&m_visitedTraits, m_visited));
-
-    // Display basic blocks.
-    DBEXEC(VERBOSE, comp->fgDispBasicBlocks());
-    DBEXEC(VERBOSE, comp->fgDispHandlerTab());
-
-    auto DumpBlockAndSuccessors = [](Compiler* comp, BasicBlock* block) {
-#ifdef DEBUG
-        if (comp->verboseSsa)
-        {
-            printf("[SsaBuilder::TopologicalSort] Pushing " FMT_BB ": [", block->bbNum);
-            AllSuccessorEnumerator successors(comp, block);
-            unsigned               index = 0;
-            while (true)
-            {
-                BasicBlock* succ = successors.NextSuccessor(comp);
-
-                if (succ == nullptr)
-                {
-                    break;
-                }
-
-                printf("%s" FMT_BB, (index++ ? ", " : ""), succ->bbNum);
-            }
-            printf("]\n");
-        }
-#endif
-    };
-
-    // Compute order.
-    unsigned    postIndex = 0;
-    BasicBlock* block     = comp->fgFirstBB;
-    BitVecOps::AddElemD(&m_visitedTraits, m_visited, block->bbNum);
-
-    ArrayStack<AllSuccessorEnumerator> blocks(m_allocator);
-    blocks.Emplace(comp, block);
-    DumpBlockAndSuccessors(comp, block);
-
-    while (!blocks.Empty())
-    {
-        BasicBlock* block = blocks.TopRef().Block();
-        BasicBlock* succ  = blocks.TopRef().NextSuccessor(comp);
-
-        if (succ != nullptr)
-        {
-            // if the block on TOS still has unreached successors, visit them
-            if (BitVecOps::TryAddElemD(&m_visitedTraits, m_visited, succ->bbNum))
-            {
-                blocks.Emplace(comp, succ);
-                DumpBlockAndSuccessors(comp, succ);
-            }
-        }
-        else
-        {
-            // all successors have been visited
-            blocks.Pop();
-
-            DBG_SSA_JITDUMP("[SsaBuilder::TopologicalSort] postOrder[%u] = " FMT_BB "\n", postIndex, block->bbNum);
-            postOrder[postIndex]  = block;
-            block->bbPostorderNum = postIndex;
-            postIndex++;
-        }
-    }
-
-    return postIndex;
-}
-
 /**
  * Computes the immediate dominator IDom for each block iteratively.
  *
@@ -226,10 +142,13 @@ unsigned SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
  *
  * @see "A simple, fast dominance algorithm." paper.
  */
-void SsaBuilder::ComputeImmediateDom(BasicBlock** postOrder, int count)
+void SsaBuilder::ComputeImmediateDom()
 {
     JITDUMP("[SsaBuilder::ComputeImmediateDom]\n");
 
+    BasicBlock** postOrder = m_pCompiler->fgPostOrder;
+    unsigned     count     = m_pCompiler->fgPostOrderCount;
+
     // Add entry point to visited as its IDom is NULL.
     assert(postOrder[count - 1] == m_pCompiler->fgFirstBB);
 
@@ -604,14 +523,14 @@ void SsaBuilder::AddPhiArg(
  *
  * To do so, the function computes liveness, dominance frontier and inserts a phi node,
  * if we have var v in def(b) and live-in(l) and l is in DF(b).
- *
- * @param postOrder The array of basic blocks arranged in postOrder.
- * @param count The size of valid elements in the postOrder array.
  */
-void SsaBuilder::InsertPhiFunctions(BasicBlock** postOrder, int count)
+void SsaBuilder::InsertPhiFunctions()
 {
     JITDUMP("*************** In SsaBuilder::InsertPhiFunctions()\n");
 
+    BasicBlock** postOrder = m_pCompiler->fgPostOrder;
+    unsigned     count     = m_pCompiler->fgPostOrderCount;
+
     // Compute dominance frontier.
     BlkToBlkVectorMap mapDF(m_allocator);
     ComputeDominanceFrontiers(postOrder, count, &mapDF);
@@ -622,7 +541,7 @@ void SsaBuilder::InsertPhiFunctions(BasicBlock** postOrder, int count)
 
     JITDUMP("Inserting phi functions:\n");
 
-    for (int i = 0; i < count; ++i)
+    for (unsigned i = 0; i < count; ++i)
     {
         BasicBlock* block = postOrder[i];
         DBG_SSA_JITDUMP("Considering dominance frontier of block " FMT_BB ":\n", block->bbNum);
@@ -1494,8 +1413,6 @@ void SsaBuilder::Build()
 
     // Allocate the postOrder array for the graph.
 
-    m_postOrder = new (m_allocator) BasicBlock*[blockCount];
-
     m_visitedTraits = BitVecTraits(blockCount, m_pCompiler);
     m_visited       = BitVecOps::MakeEmpty(&m_visitedTraits);
 
@@ -1511,13 +1428,10 @@ void SsaBuilder::Build()
         block->bbPostorderNum = 0;
     }
 
-    // Topologically sort the graph.
-    m_postOrderCount = TopologicalSort(m_postOrder, blockCount);
-    JITDUMP("[SsaBuilder] Topologically sorted the graph.\n");
-    EndPhase(PHASE_BUILD_SSA_TOPOSORT);
+    m_pCompiler->fgDfsBlocks();
 
     // Compute IDom(b).
-    ComputeImmediateDom(m_postOrder, m_postOrderCount);
+    ComputeImmediateDom();
 
     m_pCompiler->fgSsaDomTree = m_pCompiler->fgBuildDomTree();
     EndPhase(PHASE_BUILD_SSA_DOMS);
@@ -1536,7 +1450,7 @@ void SsaBuilder::Build()
     }
 
     // Insert phi functions.
-    InsertPhiFunctions(m_postOrder, m_postOrderCount);
+    InsertPhiFunctions();
 
     // Rename local variables and collect UD information for each ssa var.
     RenameVariables();