JIT: Use post order computed by SSA in VN (#94623)

VN tries hard to dynamically compute a reverse post-order to visit the
flow graph in. However, SSA has already computed such an order, so
simply pass this along to VN instead.

A few positive diffs are expected. As Andy has pointed out recently the
"dynamic RPO" VN was doing does not necessarily result in an actual RPO,
so using SSA's order is expected to be a better order than what VN ends
up with today.

Author: jakobbotsch

src/coreclr/jit/compiler.h

@@ -4493,6 +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
 
     // 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
@@ -5057,7 +5059,7 @@ class Compiler
 
     // The value numbers for this compilation.
     ValueNumStore* vnStore;
-    struct ValueNumberState* vnVisitState;
+    class ValueNumberState* vnState;
 
 public:
     ValueNumStore* GetValueNumStore()
@@ -5723,7 +5725,7 @@ class Compiler
 
 protected:
     friend class SsaBuilder;
-    friend struct ValueNumberState;
+    friend class ValueNumberState;
 
     //--------------------- Detect the basic blocks ---------------------------
 

src/coreclr/jit/fgopt.cpp

@@ -769,7 +769,6 @@ bool Compiler::fgRemoveDeadBlocks()
 // Notes:
 //   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.
 //
 //   Unreachable blocks will have higher pre and post order numbers than reachable blocks.
 //   Hence they will appear at lower indices in the fgBBReversePostorder array.

src/coreclr/jit/ssabuilder.cpp

@@ -69,6 +69,8 @@ PhaseStatus Compiler::fgSsaBuild()
     JitTestCheckSSA();
 #endif // DEBUG
 
+    fgSSAPostOrder = builder.GetPostOrder(&fgSSAPostOrderCount);
+
     return PhaseStatus::MODIFIED_EVERYTHING;
 }
 
@@ -144,7 +146,7 @@ SsaBuilder::SsaBuilder(Compiler* pCompiler)
 //  Return Value:
 //     The number of nodes visited while performing DFS on the graph.
 //
-int SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
+unsigned SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
 {
     Compiler* comp = m_pCompiler;
 
@@ -179,7 +181,7 @@ int SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
     };
 
     // Compute order.
-    int         postIndex = 0;
+    unsigned    postIndex = 0;
     BasicBlock* block     = comp->fgFirstBB;
     BitVecOps::AddElemD(&m_visitedTraits, m_visited, block->bbNum);
 
@@ -206,16 +208,13 @@ int SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count)
             // all successors have been visited
             blocks.Pop();
 
-            DBG_SSA_JITDUMP("[SsaBuilder::TopologicalSort] postOrder[%d] = " FMT_BB "\n", postIndex, block->bbNum);
+            DBG_SSA_JITDUMP("[SsaBuilder::TopologicalSort] postOrder[%u] = " FMT_BB "\n", postIndex, block->bbNum);
             postOrder[postIndex]  = block;
             block->bbPostorderNum = postIndex;
             postIndex++;
         }
     }
 
-    // In the absence of EH (because catch/finally have no preds), this should be valid.
-    // assert(postIndex == (count - 1));
-
     return postIndex;
 }
 
@@ -1500,16 +1499,7 @@ void SsaBuilder::Build()
 
     // Allocate the postOrder array for the graph.
 
-    BasicBlock** postOrder;
-
-    if (blockCount > DEFAULT_MIN_OPTS_BB_COUNT)
-    {
-        postOrder = new (m_allocator) BasicBlock*[blockCount];
-    }
-    else
-    {
-        postOrder = (BasicBlock**)_alloca(blockCount * sizeof(BasicBlock*));
-    }
+    m_postOrder = new (m_allocator) BasicBlock*[blockCount];
 
     m_visitedTraits = BitVecTraits(blockCount, m_pCompiler);
     m_visited       = BitVecOps::MakeEmpty(&m_visitedTraits);
@@ -1527,12 +1517,12 @@ void SsaBuilder::Build()
     }
 
     // Topologically sort the graph.
-    int count = TopologicalSort(postOrder, blockCount);
+    m_postOrderCount = TopologicalSort(m_postOrder, blockCount);
     JITDUMP("[SsaBuilder] Topologically sorted the graph.\n");
     EndPhase(PHASE_BUILD_SSA_TOPOSORT);
 
     // Compute IDom(b).
-    ComputeImmediateDom(postOrder, count);
+    ComputeImmediateDom(m_postOrder, m_postOrderCount);
 
     m_pCompiler->fgSsaDomTree = m_pCompiler->fgBuildDomTree();
     EndPhase(PHASE_BUILD_SSA_DOMS);
@@ -1551,7 +1541,7 @@ void SsaBuilder::Build()
     }
 
     // Insert phi functions.
-    InsertPhiFunctions(postOrder, count);
+    InsertPhiFunctions(m_postOrder, m_postOrderCount);
 
     // Rename local variables and collect UD information for each ssa var.
     RenameVariables();

src/coreclr/jit/ssabuilder.h

@@ -33,6 +33,12 @@ class SsaBuilder
     // variable are stored in the "per SSA data" on the local descriptor.
     void Build();
 
+    BasicBlock** GetPostOrder(unsigned* count)
+    {
+        *count = m_postOrderCount;
+        return m_postOrder;
+    }
+
 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
@@ -44,7 +50,7 @@ class SsaBuilder
     // the blocks in post order (i.e., a node's children first) in the array. Returns the
     // number of nodes visited while sorting the graph. In other words, valid entries in
     // the output array.
-    int TopologicalSort(BasicBlock** postOrder, int count);
+    unsigned TopologicalSort(BasicBlock** postOrder, int count);
 
     // Requires "postOrder" to hold the blocks of the flowgraph in topologically sorted
     // order. Requires count to be the valid entries in the "postOrder" array. Computes
@@ -101,4 +107,6 @@ class SsaBuilder
     BitVec       m_visited;
 
     SsaRenameState m_renameStack;
+    BasicBlock**   m_postOrder      = nullptr;
+    unsigned       m_postOrderCount = 0;
 };