Remove the `asgdLclVar` analysis from liveness.

This analysis is not necessary for correctness and does not appear to
have any impact on the generated code: in particualr, internal assembly
diffs show no changes with this analysis disabled.

For example, consider the following statement:

    `(asg (lclVar V0) (add (lclVar V0) (icon 4)))`

Under the `asgdLclVar` analysis, the visit of the lclVar node that is an
operand to the add would mark the lclVar node that is the target of the
assignment with `GTF_VAR_USEDEF` and exit without adding V0 to the
current set of used lclVars. Instead, the visit of the lclVar node that
is the target of the assignment would add V0 to both the set of used
lclVar nodes and the set of defined lclVar nodes.

With this analysis disabled, the visit of the lclVar node that is an
operand to the add will add V0 to the set of used lclVar nodes and the
visit of the lclVar node that is the target of the assignment will add
V0 to the set of defined lclVar nodes. In both cases, the result is the
same.

3 files changed, 32 insertions, 184 deletions

diff --git a/src/jit/codegenlegacy.cpp b/src/jit/codegenlegacy.cpp
index 667b9d4af8..32dd1b1aa6 100644
--- a/src/jit/codegenlegacy.cpp
+++ b/src/jit/codegenlegacy.cpp
@@ -20725,10 +20725,9 @@ bool CodeGen::genRegTrashable(regNumber reg, GenTreePtr tree)
 */
 
 GenTreePtr Compiler::fgLegacyPerStatementLocalVarLiveness(GenTreePtr startNode, // The node to start walking with.
-                                                          GenTreePtr relopNode, // The node before the startNode.
+                                                          GenTreePtr relopNode) // The node before the startNode.
                                                                                 // (It should either be NULL or
                                                                                 // a GTF_RELOP_QMARK node.)
-                                                          GenTreePtr asgdLclVar)
 {
     GenTreePtr tree;
 
@@ -20810,7 +20809,7 @@ GenTreePtr Compiler::fgLegacyPerStatementLocalVarLiveness(GenTreePtr startNode,
             case GT_LCL_FLD_ADDR:
             case GT_STORE_LCL_VAR:
             case GT_STORE_LCL_FLD:
-                fgMarkUseDef(tree->AsLclVarCommon(), asgdLclVar);
+                fgMarkUseDef(tree->AsLclVarCommon());
                 break;
 
             case GT_CLS_VAR:
@@ -20864,7 +20863,7 @@ GenTreePtr Compiler::fgLegacyPerStatementLocalVarLiveness(GenTreePtr startNode,
                     {
                         // Defines a local addr
                         assert(dummyLclVarTree != nullptr);
-                        fgMarkUseDef(dummyLclVarTree->AsLclVarCommon(), asgdLclVar);
+                        fgMarkUseDef(dummyLclVarTree->AsLclVarCommon());
                     }
                 }
                 break;
@@ -20967,7 +20966,7 @@ GenTreePtr Compiler::fgLegacyPerStatementLocalVarLiveness(GenTreePtr startNode,
                     // fgCurDefSet and fgCurUseSet into local variables defSet_BeforeSplit and useSet_BeforeSplit.
                     // The cached values will be used to restore fgCurDefSet and fgCurUseSet once we see the GT_COLON
                     // node.
-                    tree = fgLegacyPerStatementLocalVarLiveness(tree->gtNext, tree, asgdLclVar);
+                    tree = fgLegacyPerStatementLocalVarLiveness(tree->gtNext, tree);
 
                     // We must have been returned here after seeing a GT_QMARK node.
                     noway_assert(tree->gtOper == GT_QMARK);
diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index da30fa4e18..b69d574960 100644
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -3575,10 +3575,9 @@ public:
     void fgLocalVarLivenessInit();
 
 #ifdef LEGACY_BACKEND
-    GenTreePtr fgLegacyPerStatementLocalVarLiveness(GenTreePtr startNode, GenTreePtr relopNode, GenTreePtr asgdLclVar);
+    GenTreePtr fgLegacyPerStatementLocalVarLiveness(GenTreePtr startNode, GenTreePtr relopNode);
 #else
-    void fgPerNodeLocalVarLiveness(GenTree* node, GenTree* asgdLclVar);
-    void fgPerStatementLocalVarLiveness(GenTree* node, GenTree* asgdLclVar);
+    void fgPerNodeLocalVarLiveness(GenTree* node);
 #endif
     void fgPerBlockLocalVarLiveness();
 
@@ -4617,7 +4616,7 @@ private:
     bool fgCurHeapDef;   // True iff the current basic block defines the heap.
     bool fgCurHeapHavoc; // True if  the current basic block is known to set the heap to a "havoc" value.
 
-    void fgMarkUseDef(GenTreeLclVarCommon* tree, GenTree* asgdLclVar = nullptr);
+    void fgMarkUseDef(GenTreeLclVarCommon* tree);
 
     void fgBeginScopeLife(VARSET_TP* inScope, VarScopeDsc* var);
     void fgEndScopeLife(VARSET_TP* inScope, VarScopeDsc* var);
diff --git a/src/jit/liveness.cpp b/src/jit/liveness.cpp
index a673b7edca..1b19785375 100644
--- a/src/jit/liveness.cpp
+++ b/src/jit/liveness.cpp
@@ -19,34 +19,22 @@
  *
  *  Helper for Compiler::fgPerBlockLocalVarLiveness().
  *  The goal is to compute the USE and DEF sets for a basic block.
- *  However with the new improvement to the data flow analysis (DFA),
- *  we do not mark x as used in x = f(x) when there are no side effects in f(x).
- *  'asgdLclVar' is set when 'tree' is part of an expression with no side-effects
- *  which is assigned to asgdLclVar, ie. asgdLclVar = (... tree ...)
  */
-void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree, GenTree* asgdLclVar)
+void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree)
 {
-    bool       rhsUSEDEF = false;
-    unsigned   lclNum;
-    unsigned   lhsLclNum;
-    LclVarDsc* varDsc;
-
-    noway_assert(tree->gtOper == GT_LCL_VAR || tree->gtOper == GT_LCL_VAR_ADDR || tree->gtOper == GT_LCL_FLD ||
-                 tree->gtOper == GT_LCL_FLD_ADDR || tree->gtOper == GT_STORE_LCL_VAR ||
-                 tree->gtOper == GT_STORE_LCL_FLD);
-
+    unsigned lclNum;
     if (tree->gtOper == GT_LCL_VAR || tree->gtOper == GT_LCL_VAR_ADDR || tree->gtOper == GT_STORE_LCL_VAR)
     {
         lclNum = tree->gtLclNum;
     }
     else
     {
-        noway_assert(tree->OperIsLocalField());
+        assert(tree->OperIsLocalField());
         lclNum = tree->gtLclFld.gtLclNum;
     }
 
-    noway_assert(lclNum < lvaCount);
-    varDsc = lvaTable + lclNum;
+    assert(lclNum < lvaCount);
+    LclVarDsc* const varDsc = lvaTable + lclNum;
 
     // We should never encounter a reference to a lclVar that has a zero refCnt.
     if (varDsc->lvRefCnt == 0 && (!varTypeIsPromotable(varDsc) || !varDsc->lvPromoted))
@@ -56,96 +44,27 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree, GenTree* asgdLclVar)
         varDsc->lvRefCnt = 1;
     }
 
-    // NOTE: the analysis done below is neither necessary nor correct for LIR: it depends on
-    // the nodes that precede `asgdLclVar` in execution order to factor into the dataflow for the
-    // value being assigned to the local var, which is not necessarily the case without tree
-    // order. Furthermore, LIR is always traversed in an order that reflects the dataflow for the
-    // block.
-    if (asgdLclVar != nullptr)
+    if (varDsc->lvTracked)
     {
-        assert(!compCurBB->IsLIR());
-
-        /* we have an assignment to a local var : asgdLclVar = ... tree ...
-         * check for x = f(x) case */
-
-        noway_assert(asgdLclVar->gtOper == GT_LCL_VAR || asgdLclVar->gtOper == GT_STORE_LCL_VAR);
-        noway_assert(asgdLclVar->gtFlags & GTF_VAR_DEF);
+        assert(varDsc->lvVarIndex < lvaTrackedCount);
 
-        lhsLclNum = asgdLclVar->gtLclVarCommon.gtLclNum;
+        const bool isDef = (tree->gtFlags & GTF_VAR_DEF) != 0;
+        const bool isUse = !isDef || ((tree->gtFlags & (GTF_VAR_USEASG | GTF_VAR_USEDEF)) != 0);
 
-        if ((lhsLclNum == lclNum) && ((tree->gtFlags & GTF_VAR_DEF) == 0) && (tree != asgdLclVar))
+        if (isUse && !VarSetOps::IsMember(this, fgCurDefSet, varDsc->lvVarIndex))
         {
-            /* bingo - we have an x = f(x) case */
-            asgdLclVar->gtFlags |= GTF_VAR_USEDEF;
-            rhsUSEDEF = true;
+            // This is an exposed use; add it to the set of uses.
+            VarSetOps::AddElemD(this, fgCurUseSet, varDsc->lvVarIndex);
         }
-    }
-
-    /* Is this a tracked variable? */
-
-    if (varDsc->lvTracked)
-    {
-        noway_assert(varDsc->lvVarIndex < lvaTrackedCount);
 
-        if ((tree->gtFlags & GTF_VAR_DEF) != 0 && (tree->gtFlags & (GTF_VAR_USEASG | GTF_VAR_USEDEF)) == 0)
+        if (isDef)
         {
-            // if  (!(fgCurUseSet & bitMask)) printf("V%02u,T%02u def at %08p\n", lclNum, varDsc->lvVarIndex, tree);
+            // This is a def, add it to the set of defs.
             VarSetOps::AddElemD(this, fgCurDefSet, varDsc->lvVarIndex);
         }
-        else
-        {
-            // if  (!(fgCurDefSet & bitMask))
-            // {
-            //      printf("V%02u,T%02u use at ", lclNum, varDsc->lvVarIndex);
-            //      printTreeID(tree);
-            //      printf("\n");
-            // }
-
-            /* We have the following scenarios:
-             *   1. "x += something" - in this case x is flagged GTF_VAR_USEASG
-             *   2. "x = ... x ..." - the LHS x is flagged GTF_VAR_USEDEF,
-             *                        the RHS x is has rhsUSEDEF = true
-             *                        (both set by the code above)
-             *
-             * We should not mark an USE of x in the above cases provided the value "x" is not used
-             * further up in the tree. For example "while (i++)" is required to mark i as used.
-             */
-
-            /* make sure we don't include USEDEF variables in the USE set
-             * The first test is for LSH, the second (!rhsUSEDEF) is for any var in the RHS */
-
-            if ((tree->gtFlags & (GTF_VAR_USEASG | GTF_VAR_USEDEF)) == 0)
-            {
-                /* Not a special flag - check to see if used to assign to itself */
-
-                if (rhsUSEDEF)
-                {
-                    /* assign to itself - do not include it in the USE set */
-                    if (!opts.MinOpts() && !opts.compDbgCode)
-                    {
-                        return;
-                    }
-                }
-            }
-
-            /* Fall through for the "good" cases above - add the variable to the USE set */
-
-            if (!VarSetOps::IsMember(this, fgCurDefSet, varDsc->lvVarIndex))
-            {
-                VarSetOps::AddElemD(this, fgCurUseSet, varDsc->lvVarIndex);
-            }
-
-            // For defs, also add to the (all) def set.
-            if ((tree->gtFlags & GTF_VAR_DEF) != 0)
-            {
-                VarSetOps::AddElemD(this, fgCurDefSet, varDsc->lvVarIndex);
-            }
-        }
     }
     else if (varTypeIsStruct(varDsc))
     {
-        noway_assert(!varDsc->lvTracked);
-
         lvaPromotionType promotionType = lvaGetPromotionType(varDsc);
 
         if (promotionType != PROMOTION_TYPE_NONE)
@@ -290,13 +209,10 @@ void Compiler::fgLocalVarLivenessInit()
 //
 // Arguments:
 //    tree       - The current node.
-//    asgdLclVar - Either nullptr or the assignement's left-hand-side GT_LCL_VAR.
-//                 Used as an argument to fgMarkUseDef(); only valid for HIR blocks.
 //
-void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree, GenTree* asgdLclVar)
+void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree)
 {
     assert(tree != nullptr);
-    assert(asgdLclVar == nullptr || !compCurBB->IsLIR());
 
     switch (tree->gtOper)
     {
@@ -312,7 +228,7 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree, GenTree* asgdLclVar)
         case GT_LCL_FLD_ADDR:
         case GT_STORE_LCL_VAR:
         case GT_STORE_LCL_FLD:
-            fgMarkUseDef(tree->AsLclVarCommon(), asgdLclVar);
+            fgMarkUseDef(tree->AsLclVarCommon());
             break;
 
         case GT_CLS_VAR:
@@ -365,7 +281,7 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree, GenTree* asgdLclVar)
                 {
                     // Defines a local addr
                     assert(dummyLclVarTree != nullptr);
-                    fgMarkUseDef(dummyLclVarTree->AsLclVarCommon(), asgdLclVar);
+                    fgMarkUseDef(dummyLclVarTree->AsLclVarCommon());
                 }
             }
             break;
@@ -459,21 +375,6 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree, GenTree* asgdLclVar)
     }
 }
 
-void Compiler::fgPerStatementLocalVarLiveness(GenTree* startNode, GenTree* asgdLclVar)
-{
-    // The startNode must be the 1st node of the statement.
-    assert(startNode == compCurStmt->gtStmt.gtStmtList);
-
-    // The asgdLclVar node must be either nullptr or a GT_LCL_VAR or GT_STORE_LCL_VAR
-    assert((asgdLclVar == nullptr) || (asgdLclVar->gtOper == GT_LCL_VAR || asgdLclVar->gtOper == GT_STORE_LCL_VAR));
-
-    // We always walk every node in statement list
-    for (GenTreePtr node = startNode; node != nullptr; node = node->gtNext)
-    {
-        fgPerNodeLocalVarLiveness(node, asgdLclVar);
-    }
-}
-
 #endif // !LEGACY_BACKEND
 
 /*****************************************************************************/
@@ -545,10 +446,6 @@ void Compiler::fgPerBlockLocalVarLiveness()
 
     for (block = fgFirstBB; block; block = block->bbNext)
     {
-        GenTreePtr stmt;
-        GenTreePtr tree;
-        GenTreePtr asgdLclVar;
-
         VarSetOps::ClearD(this, fgCurUseSet);
         VarSetOps::ClearD(this, fgCurDefSet);
 
@@ -557,63 +454,20 @@ void Compiler::fgPerBlockLocalVarLiveness()
         fgCurHeapHavoc = false;
 
         compCurBB = block;
-
         if (!block->IsLIR())
         {
-            for (stmt = block->FirstNonPhiDef(); stmt; stmt = stmt->gtNext)
+            for (GenTreeStmt* stmt = block->FirstNonPhiDef(); stmt; stmt = stmt->gtNextStmt)
             {
-                noway_assert(stmt->gtOper == GT_STMT);
-
                 compCurStmt = stmt;
 
-                asgdLclVar = nullptr;
-                tree       = stmt->gtStmt.gtStmtExpr;
-                noway_assert(tree);
-
-                // The following code checks if we have an assignment expression
-                // which may become a GTF_VAR_USEDEF - x=f(x).
-                // consider if LHS is local var - ignore if RHS contains SIDE_EFFECTS
-
-                if ((tree->gtOper == GT_ASG && tree->gtOp.gtOp1->gtOper == GT_LCL_VAR) ||
-                    tree->gtOper == GT_STORE_LCL_VAR)
-                {
-                    noway_assert(tree->gtOp.gtOp1);
-                    GenTreePtr rhsNode;
-                    if (tree->gtOper == GT_ASG)
-                    {
-                        noway_assert(tree->gtOp.gtOp2);
-                        asgdLclVar = tree->gtOp.gtOp1;
-                        rhsNode    = tree->gtOp.gtOp2;
-                    }
-                    else
-                    {
-                        asgdLclVar = tree;
-                        rhsNode    = tree->gtOp.gtOp1;
-                    }
-
-                    // If this is an assignment to local var with no SIDE EFFECTS,
-                    // set asgdLclVar so that genMarkUseDef will flag potential
-                    // x=f(x) expressions as GTF_VAR_USEDEF.
-                    // Reset the flag before recomputing it - it may have been set before,
-                    // but subsequent optimizations could have removed the rhs reference.
-                    asgdLclVar->gtFlags &= ~GTF_VAR_USEDEF;
-                    if ((rhsNode->gtFlags & GTF_SIDE_EFFECT) == 0)
-                    {
-                        noway_assert(asgdLclVar->gtFlags & GTF_VAR_DEF);
-                    }
-                    else
-                    {
-                        asgdLclVar = nullptr;
-                    }
-                }
-
 #ifdef LEGACY_BACKEND
-                tree = fgLegacyPerStatementLocalVarLiveness(stmt->gtStmt.gtStmtList, NULL, asgdLclVar);
-
-                // We must have walked to the end of this statement.
-                noway_assert(!tree);
+                GenTree* tree = fgLegacyPerStatementLocalVarLiveness(stmt->gtStmtList, nullptr);
+                assert(tree == nullptr);
 #else  // !LEGACY_BACKEND
-                fgPerStatementLocalVarLiveness(stmt->gtStmt.gtStmtList, asgdLclVar);
+                for (GenTree* node = stmt->gtStmtList; node != nullptr; node = node->gtNext)
+                {
+                    fgPerNodeLocalVarLiveness(node);
+                }
 #endif // !LEGACY_BACKEND
             }
         }
@@ -622,13 +476,9 @@ void Compiler::fgPerBlockLocalVarLiveness()
 #ifdef LEGACY_BACKEND
             unreached();
 #else  // !LEGACY_BACKEND
-            // NOTE: the `asgdLclVar` analysis done above is not correct for LIR: it depends
-            // on all of the nodes that precede `asgdLclVar` in execution order to factor into the
-            // dataflow for the value being assigned to the local var, which is not necessarily the
-            // case without tree order. As a result, we simply pass `nullptr` for `asgdLclVar`.
             for (GenTree* node : LIR::AsRange(block).NonPhiNodes())
             {
-                fgPerNodeLocalVarLiveness(node, nullptr);
+                fgPerNodeLocalVarLiveness(node);
             }
 #endif // !LEGACY_BACKEND
         }