JIT: Sparse conditional propagation in value numbering

src/coreclr/jit/compiler.h

@@ -5057,6 +5057,7 @@ class Compiler
 
     // The value numbers for this compilation.
     ValueNumStore* vnStore;
+    struct ValueNumberState* vnVisitState;
 
 public:
     ValueNumStore* GetValueNumStore()

src/coreclr/jit/valuenum.cpp

@@ -9658,6 +9658,8 @@ struct ValueNumberState
         BVB_complete     = 0x1,
         BVB_onAllDone    = 0x2,
         BVB_onNotAllDone = 0x4,
+        // Set for statically reachable blocks that we have proven unreachable.
+        BVB_provenUnreachable = 0x8,
     };
 
     bool GetVisitBit(unsigned bbNum, BlockVisitBits bvb)
@@ -9787,7 +9789,8 @@ struct ValueNumberState
             JITDUMP("     Not yet completed.\n");
 #endif // DEBUG_VN_VISIT
 
-            bool allPredsVisited = true;
+            bool allPredsVisited   = true;
+            bool provenUnreachable = true;
             for (FlowEdge* pred = m_comp->BlockPredsWithEH(succ); pred != nullptr; pred = pred->getNextPredEdge())
             {
                 BasicBlock* predBlock = pred->getSourceBlock();
@@ -9796,6 +9799,12 @@ struct ValueNumberState
                     allPredsVisited = false;
                     break;
                 }
+
+                if (provenUnreachable && !GetVisitBit(predBlock->bbNum, BVB_provenUnreachable) &&
+                    IsReachableFromPred(predBlock, succ))
+                {
+                    provenUnreachable = false;
+                }
             }
 
             if (allPredsVisited)
@@ -9808,6 +9817,11 @@ struct ValueNumberState
                 assert(!GetVisitBit(succ->bbNum, BVB_onAllDone));
                 m_toDoAllPredsDone.Push(succ);
                 SetVisitBit(succ->bbNum, BVB_onAllDone);
+
+                if (provenUnreachable)
+                {
+                    SetVisitBit(succ->bbNum, BVB_provenUnreachable);
+                }
             }
             else
             {
@@ -9829,6 +9843,31 @@ struct ValueNumberState
         });
     }
 
+    bool IsReachableFromPred(BasicBlock* predBlock, BasicBlock* block)
+    {
+        if (!predBlock->KindIs(BBJ_COND) || predBlock->JumpsToNext())
+        {
+            return true;
+        }
+
+        GenTree* lastTree = predBlock->lastStmt()->GetRootNode();
+        assert(lastTree->OperIs(GT_JTRUE));
+
+        GenTree* cond = lastTree->gtGetOp1();
+        // TODO-Cleanup: Using liberal VNs here is a bit questionable as it
+        // adds a cross-phase dependency on RBO to definitely fold this branch
+        // away.
+        ValueNum normalVN = m_comp->vnStore->VNNormalValue(cond->GetVN(VNK_Liberal));
+        if (!m_comp->vnStore->IsVNConstant(normalVN))
+        {
+            return true;
+        }
+
+        bool        isTaken         = normalVN != m_comp->vnStore->VNZeroForType(TYP_INT);
+        BasicBlock* unreachableSucc = isTaken ? predBlock->Next() : predBlock->GetJumpDest();
+        return block != unreachableSucc;
+    }
+
     bool ToDoExists()
     {
         return m_toDoAllPredsDone.Size() > 0 || m_toDoNotAllPredsDone.Size() > 0;
@@ -9965,6 +10004,8 @@ PhaseStatus Compiler::fgValueNumber()
 
     ValueNumberState vs(this);
 
+    vnVisitState = &vs;
+
     // Push the first block.  This has no preds.
     vs.m_toDoAllPredsDone.Push(fgFirstBB);
 
@@ -9973,7 +10014,13 @@ PhaseStatus Compiler::fgValueNumber()
         while (vs.m_toDoAllPredsDone.Size() > 0)
         {
             BasicBlock* toDo = vs.m_toDoAllPredsDone.Pop();
+
+            // TODO-TP: We can skip VN'ing blocks we have proven unreachable
+            // here. However, currently downstream phases are not prepared to
+            // handle the fact that some blocks that are seemingly reachable
+            // have not been VN'd.
             fgValueNumberBlock(toDo);
+
             // Record that we've visited "toDo", and add successors to the right sets.
             vs.FinishVisit(toDo);
         }
@@ -10010,8 +10057,7 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
 
     Statement* stmt = blk->firstStmt();
 
-    // First: visit phi's.  If "newVNForPhis", give them new VN's.  If not,
-    // first check to see if all phi args have the same value.
+    // First: visit phis and check to see if all phi args have the same value.
     for (; (stmt != nullptr) && stmt->IsPhiDefnStmt(); stmt = stmt->GetNextStmt())
     {
         GenTreeLclVar* newSsaDef = stmt->GetRootNode()->AsLclVar();
@@ -10021,9 +10067,22 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
 
         for (GenTreePhi::Use& use : phiNode->Uses())
         {
-            GenTreePhiArg* phiArg         = use.GetNode()->AsPhiArg();
-            ValueNum       phiArgSsaNumVN = vnStore->VNForIntCon(phiArg->GetSsaNum());
-            ValueNumPair   phiArgVNP      = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum())->m_vnPair;
+            GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg();
+            if (vnVisitState->GetVisitBit(phiArg->gtPredBB->bbNum, ValueNumberState::BVB_provenUnreachable))
+            {
+                JITDUMP("  Phi arg [%06u] refers to proven unreachable pred " FMT_BB "\n", dspTreeID(phiArg),
+                        phiArg->gtPredBB->bbNum);
+
+                if ((use.GetNext() != nullptr) || (phiVNP.GetLiberal() != ValueNumStore::NoVN))
+                {
+                    continue;
+                }
+
+                JITDUMP("  ..but it looks like all preds are unreachable, so we are using it anyway\n");
+            }
+
+            ValueNum     phiArgSsaNumVN = vnStore->VNForIntCon(phiArg->GetSsaNum());
+            ValueNumPair phiArgVNP      = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum())->m_vnPair;
 
             phiArg->gtVNPair = phiArgVNP;
 
@@ -10049,12 +10108,9 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk)
             }
         }
 
-#ifdef DEBUG
-        // There should be at least to 2 PHI arguments so phiVN's VNs should always be VNF_Phi functions.
-        VNFuncApp phiFunc;
-        assert(vnStore->GetVNFunc(phiVNP.GetLiberal(), &phiFunc) && (phiFunc.m_func == VNF_Phi));
-        assert(vnStore->GetVNFunc(phiVNP.GetConservative(), &phiFunc) && (phiFunc.m_func == VNF_Phi));
-#endif
+        // We should have visited at least one phi arg in the loop above
+        assert(phiVNP.GetLiberal() != ValueNumStore::NoVN);
+        assert(phiVNP.GetConservative() != ValueNumStore::NoVN);
 
         ValueNumPair newSsaDefVNP;