JIT: Extend USEASG liveness to cover partial writes to retbufs (#105996)

Liveness will add uses for explicit USEASG local stores that are not dead, but not for similar stores to locals via calls. Extend the coverage to calls. Fixes #105667.
dotnet · Aug 6, 2024 · fdb7415 · fdb7415
1 parent 4783909
commit fdb7415
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 30 deletions.
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -5591,7 +5591,7 @@ class Compiler
 
     void fgLiveVarAnalysis();
 
-    void fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call);
+    GenTreeLclVarCommon* fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call);
 
     void fgComputeLifeTrackedLocalUse(VARSET_TP& life, LclVarDsc& varDsc, GenTreeLclVarCommon* node);
     bool fgComputeLifeTrackedLocalDef(VARSET_TP&           life,

diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp
@@ -772,9 +772,13 @@ void Compiler::fgLiveVarAnalysis()
 //    keepAliveVars - Tracked locals that must be kept alive everywhere in the block
 //    call          - The call node in question.
 //
-void Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call)
+// Returns:
+//    local defined by the call, if any (eg retbuf)
+//
+GenTreeLclVarCommon* Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call)
 {
     assert(call != nullptr);
+    GenTreeLclVarCommon* definedLcl = nullptr;
 
     // If this is a tail-call via helper, and we have any unmanaged p/invoke calls in
     // the method, then we're going to run the p/invoke epilog
@@ -834,11 +838,13 @@ void Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars
         }
     }
 
-    GenTreeLclVarCommon* definedLcl = gtCallGetDefinedRetBufLclAddr(call);
+    definedLcl = gtCallGetDefinedRetBufLclAddr(call);
     if (definedLcl != nullptr)
     {
         fgComputeLifeLocal(life, keepAliveVars, definedLcl);
     }
+
+    return definedLcl;
 }
 
 //------------------------------------------------------------------------
@@ -1171,54 +1177,63 @@ void Compiler::fgComputeLife(VARSET_TP&           life,
     AGAIN:
         assert(tree->OperGet() != GT_QMARK);
 
+        bool       isUse        = false;
+        bool       doAgain      = false;
+        bool       storeRemoved = false;
+        LclVarDsc* varDsc       = nullptr;
+
         if (tree->IsCall())
         {
-            fgComputeLifeCall(life, keepAliveVars, tree->AsCall());
+            GenTreeLclVarCommon* const definedLcl = fgComputeLifeCall(life, keepAliveVars, tree->AsCall());
+            if (definedLcl != nullptr)
+            {
+                isUse  = (definedLcl->gtFlags & GTF_VAR_USEASG) != 0;
+                varDsc = lvaGetDesc(definedLcl);
+            }
         }
         else if (tree->OperIsNonPhiLocal())
         {
+            isUse            = (tree->gtFlags & GTF_VAR_USEASG) != 0;
             bool isDeadStore = fgComputeLifeLocal(life, keepAliveVars, tree);
             if (isDeadStore)
             {
-                LclVarDsc* varDsc       = lvaGetDesc(tree->AsLclVarCommon());
-                bool       isUse        = (tree->gtFlags & GTF_VAR_USEASG) != 0;
-                bool       doAgain      = false;
-                bool       storeRemoved = false;
+                varDsc = lvaGetDesc(tree->AsLclVarCommon());
 
                 if (fgRemoveDeadStore(&tree, varDsc, life, &doAgain, pStmtInfoDirty, &storeRemoved DEBUGARG(treeModf)))
                 {
                     assert(!doAgain);
                     break;
                 }
+            }
+        }
 
-                if (isUse && !storeRemoved)
+        // SSA and VN treat "partial definitions" as true uses, so for this
+        // front-end liveness pass we must add them to the live set in case
+        // we failed to remove the dead store.
+        //
+        if ((varDsc != nullptr) && isUse && !storeRemoved)
+        {
+            if (varDsc->lvTracked)
+            {
+                VarSetOps::AddElemD(this, life, varDsc->lvVarIndex);
+            }
+            if (varDsc->lvPromoted)
+            {
+                for (unsigned fieldIndex = 0; fieldIndex < varDsc->lvFieldCnt; fieldIndex++)
                 {
-                    // SSA and VN treat "partial definitions" as true uses, so for this
-                    // front-end liveness pass we must add them to the live set in case
-                    // we failed to remove the dead store.
-                    if (varDsc->lvTracked)
-                    {
-                        VarSetOps::AddElemD(this, life, varDsc->lvVarIndex);
-                    }
-                    if (varDsc->lvPromoted)
+                    LclVarDsc* fieldVarDsc = lvaGetDesc(varDsc->lvFieldLclStart + fieldIndex);
+                    if (fieldVarDsc->lvTracked)
                     {
-                        for (unsigned fieldIndex = 0; fieldIndex < varDsc->lvFieldCnt; fieldIndex++)
-                        {
-                            LclVarDsc* fieldVarDsc = lvaGetDesc(varDsc->lvFieldLclStart + fieldIndex);
-                            if (fieldVarDsc->lvTracked)
-                            {
-                                VarSetOps::AddElemD(this, life, fieldVarDsc->lvVarIndex);
-                            }
-                        }
+                        VarSetOps::AddElemD(this, life, fieldVarDsc->lvVarIndex);
                     }
                 }
-
-                if (doAgain)
-                {
-                    goto AGAIN;
-                }
             }
         }
+
+        if (doAgain)
+        {
+            goto AGAIN;
+        }
     }
 }