JIT: Fix VN-based dead store removal for unused SSA defs (#123621)

The VN-based dead store removal phase only handled value-redundant
stores but had no mechanism to remove stores with zero remaining uses.
After Redundant branch opts removes JTRUE references, inlining temps'
dead stores were left behind, preventing fgFoldCondToReturnBlock from
producing branchless codegen.

- Add an SSA use recount pass to get accurate counts after earlier
  phases may have removed references without updating use counts
- Change the inner loop to start at defIndex=0 so inlining temps'
  first explicit def (at m_array[0], with no implicit entry def) is
  considered for removal
- Add a "no remaining uses" dead store removal path that removes the
  entire statement when data has no side effects, falling back to the
  COMMA approach otherwise
- Revert optimizebools.cpp to the original hasSingleStmt() check,
  since the dead stores are now properly removed before Optimize bools

Author: yykkibbb

src/coreclr/jit/compiler.h

@@ -293,6 +293,13 @@ class LclSsaVarDsc
         return m_numUses;
     }
 
+    void ResetUses()
+    {
+        m_numUses    = 0;
+        m_hasPhiUse  = false;
+        m_hasGlobalUse = false;
+    }
+
     void AddUse(BasicBlock* block)
     {
         if (block != m_block)

src/coreclr/jit/optimizebools.cpp

@@ -1769,24 +1769,10 @@ bool Compiler::fgFoldCondToReturnBlock(BasicBlock* block)
 
     // Is block a BBJ_RETURN(1/0) ?
     auto isReturnBool = [](const BasicBlock* block, bool value) {
-        if (block->KindIs(BBJ_RETURN) && (block->lastStmt() != nullptr))
+        if (block->KindIs(BBJ_RETURN) && block->hasSingleStmt())
         {
             GenTree* node = block->lastStmt()->GetRootNode();
-            if (!(node->OperIs(GT_RETURN) && node->gtGetOp1()->IsIntegralConst(value ? 1 : 0)))
-            {
-                return false;
-            }
-            // Allow preceding statements if they have no globally visible side effects
-            // (e.g., dead local stores left over from inlining).
-            for (Statement* stmt = block->firstStmt(); stmt != block->lastStmt();
-                 stmt          = stmt->GetNextStmt())
-            {
-                if (GTF_GLOBALLY_VISIBLE_SIDE_EFFECTS(stmt->GetRootNode()->gtFlags))
-                {
-                    return false;
-                }
-            }
-            return true;
+            return node->OperIs(GT_RETURN) && node->gtGetOp1()->IsIntegralConst(value ? 1 : 0);
         }
         return false;
     };

src/coreclr/jit/optimizer.cpp

@@ -5727,6 +5727,51 @@ PhaseStatus Compiler::optVNBasedDeadStoreRemoval()
 
     bool madeChanges = false;
 
+    // Recount actual SSA uses. Earlier phases (e.g., Redundant branch opts) may
+    // have removed trees that referenced SSA defs without updating the use counts.
+    // A fresh count lets us identify stores whose values are truly never read.
+    for (unsigned lclNum = 0; lclNum < lvaCount; lclNum++)
+    {
+        if (!lvaInSsa(lclNum))
+        {
+            continue;
+        }
+
+        LclVarDsc* varDsc   = lvaGetDesc(lclNum);
+        unsigned   defCount = varDsc->lvPerSsaData.GetCount();
+        for (unsigned i = 0; i < defCount; i++)
+        {
+            varDsc->lvPerSsaData.GetSsaDefByIndex(i)->ResetUses();
+        }
+    }
+
+    for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->Next())
+    {
+        for (Statement* stmt = block->firstStmt(); stmt != nullptr; stmt = stmt->GetNextStmt())
+        {
+            for (GenTree* node = stmt->GetTreeList(); node != nullptr; node = node->gtNext)
+            {
+                if (node->OperIsLocalRead())
+                {
+                    GenTreeLclVarCommon* lcl    = node->AsLclVarCommon();
+                    unsigned            lclNum = lcl->GetLclNum();
+                    if (lvaInSsa(lclNum) && lcl->HasSsaName())
+                    {
+                        LclVarDsc* varDsc = lvaGetDesc(lclNum);
+                        if (node->OperIs(GT_PHI_ARG))
+                        {
+                            varDsc->GetPerSsaData(lcl->GetSsaNum())->AddPhiUse(block);
+                        }
+                        else
+                        {
+                            varDsc->GetPerSsaData(lcl->GetSsaNum())->AddUse(block);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     for (unsigned lclNum = 0; lclNum < lvaCount; lclNum++)
     {
         if (!lvaInSsa(lclNum))
@@ -5749,7 +5794,7 @@ PhaseStatus Compiler::optVNBasedDeadStoreRemoval()
             continue;
         }
 
-        for (unsigned defIndex = 1; defIndex < defCount; defIndex++)
+        for (unsigned defIndex = 0; defIndex < defCount; defIndex++)
         {
             LclSsaVarDsc*        defDsc = varDsc->lvPerSsaData.GetSsaDefByIndex(defIndex);
             GenTreeLclVarCommon* store  = defDsc->GetDefNode();
@@ -5766,6 +5811,60 @@ PhaseStatus Compiler::optVNBasedDeadStoreRemoval()
                     continue;
                 }
 
+                // If this SSA def has no remaining uses, the store is dead
+                // regardless of the stored value. Earlier phases (e.g.,
+                // Redundant branch opts) may have removed all references.
+                if (defDsc->GetNumUses() == 0)
+                {
+                    JITDUMP("Removed dead store (no remaining uses):\n");
+                    DISPTREE(store);
+
+                    GenTree* data = store->AsLclVarCommon()->Data();
+
+                    bool removed = false;
+                    if ((data->gtFlags & GTF_SIDE_EFFECT) == 0)
+                    {
+                        // Data has no side effects; try to remove the entire statement.
+                        BasicBlock* block = defDsc->GetBlock();
+                        for (Statement* stmt = block->firstStmt(); stmt != nullptr; stmt = stmt->GetNextStmt())
+                        {
+                            if (stmt->GetRootNode() == store)
+                            {
+                                fgRemoveStmt(block, stmt);
+                                removed = true;
+                                break;
+                            }
+                        }
+                    }
+
+                    if (!removed)
+                    {
+                        // Data has side effects or store is not the statement root;
+                        // keep them via a COMMA.
+                        // TODO-ASG: delete this hack.
+                        GenTree* nop  = gtNewNothingNode();
+                        data->gtNext  = nop;
+                        nop->gtPrev   = data;
+                        nop->gtNext   = store;
+                        store->gtPrev = nop;
+
+                        store->ChangeOper(GT_COMMA);
+                        store->AsOp()->gtOp2 = nop;
+                        store->gtType        = TYP_VOID;
+                        store->SetAllEffectsFlags(data);
+                        gtUpdateTreeAncestorsSideEffects(store);
+                    }
+
+                    madeChanges = true;
+                    continue;
+                }
+
+                // Value-redundancy check requires a previous def to compare against.
+                if (defIndex == 0)
+                {
+                    continue;
+                }
+
                 ValueNum oldStoreValue;
                 if ((store->gtFlags & GTF_VAR_USEASG) == 0)
                 {