Merge pull request #17431 from CarolEidt/Fix17389

LSRA: remove last uses only at use point

1 files changed, 33 insertions, 23 deletions

diff --git a/src/jit/lsrabuild.cpp b/src/jit/lsrabuild.cpp
index 62692d587a..195d1e8b11 100644
--- a/src/jit/lsrabuild.cpp
+++ b/src/jit/lsrabuild.cpp
@@ -1347,21 +1347,6 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, BasicBlock* block, Lsra
             assert(varDsc->lvTracked);
             unsigned varIndex = varDsc->lvVarIndex;
 
-            // We have only approximate last-use information at this point.  This is because the
-            // execution order doesn't actually reflect the true order in which the localVars
-            // are referenced - but the order of the RefPositions will, so we recompute it after
-            // RefPositions are built.
-            // Use the old value for setting currentLiveVars - note that we do this with the
-            // not-quite-correct setting of lastUse.  However, this is OK because
-            // 1) this is only for preferencing, which doesn't require strict correctness, and
-            // 2) the cases where these out-of-order uses occur should not overlap a kill.
-            // TODO-Throughput: clean this up once we have the execution order correct.  At that point
-            // we can update currentLiveVars at the same place that we create the RefPosition.
-            if ((tree->gtFlags & GTF_VAR_DEATH) != 0)
-            {
-                VarSetOps::RemoveElemD(compiler, currentLiveVars, varIndex);
-            }
-
             if (!tree->IsUnusedValue() && !tree->isContained())
             {
                 assert(produce != 0);
@@ -1436,11 +1421,6 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, BasicBlock* block, Lsra
                     srcInterval->assignRelatedInterval(varDefInterval);
                 }
             }
-
-            if ((tree->gtFlags & GTF_VAR_DEATH) == 0)
-            {
-                VarSetOps::AddElemD(compiler, currentLiveVars, varIndex);
-            }
         }
         else if (store->gtOp1->OperIs(GT_BITCAST))
         {
@@ -1596,9 +1576,34 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, BasicBlock* block, Lsra
 
         assert((candidates & allRegs(srcInterval->registerType)) != 0);
 
-        // For non-localVar uses we record nothing, as nothing needs to be written back to the tree.
-        GenTree* const refPosNode = srcInterval->isLocalVar ? useNode : nullptr;
-        RefPosition*   pos        = newRefPosition(srcInterval, currentLoc, RefTypeUse, refPosNode, candidates, 0);
+        GenTree* refPosNode;
+        if (srcInterval->isLocalVar)
+        {
+            // We have only approximate last-use information at this point.  This is because the
+            // execution order doesn't actually reflect the true order in which the localVars
+            // are referenced - but the order of the RefPositions will, so we recompute it after
+            // RefPositions are built.
+            // Use the old value for setting currentLiveVars - note that we do this with the
+            // not-quite-correct setting of lastUse.  However, this is OK because
+            // 1) this is only for preferencing, which doesn't require strict correctness, and
+            //    for determing which largeVectors require having their upper-half saved & restored.
+            //    (Issue #17481 tracks the issue that this system results in excessive spills and
+            //    should be changed.)
+            // 2) the cases where these out-of-order uses occur should not overlap a kill (they are
+            //    only known to occur within a single expression).
+            if ((useNode->gtFlags & GTF_VAR_DEATH) != 0)
+            {
+                VarSetOps::RemoveElemD(compiler, currentLiveVars, srcInterval->getVarIndex(compiler));
+            }
+            refPosNode = useNode;
+        }
+        else
+        {
+            // For non-localVar uses we record nothing, as nothing needs to be written back to the tree.
+            refPosNode = nullptr;
+        }
+
+        RefPosition* pos = newRefPosition(srcInterval, currentLoc, RefTypeUse, refPosNode, candidates, 0);
         if (delayRegFree)
         {
             pos->delayRegFree = true;
@@ -1736,6 +1741,11 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, BasicBlock* block, Lsra
         else
         {
             assert(registerTypesEquivalent(interval->registerType, registerType));
+            assert(interval->isLocalVar);
+            if ((tree->gtFlags & GTF_VAR_DEATH) == 0)
+            {
+                VarSetOps::AddElemD(compiler, currentLiveVars, interval->getVarIndex(compiler));
+            }
         }
 
         if (prefSrcInterval != nullptr)