Change the SSA def node to ASG

src/jit/compiler.h

@@ -207,35 +207,52 @@ struct VarScopeDsc
 #endif
 };
 
-// This is the location of a SSA definition.
-struct DefLoc
+// This class stores information associated with a LclVar SSA definition.
+class LclSsaVarDsc
 {
-    BasicBlock* m_blk;
-    GenTree*    m_tree;
+    // The basic block where the definition occurs. Definitions of uninitialized variables
+    // are considered to occur at the start of the first basic block (fgFirstBB).
+    //
+    // TODO-Cleanup: In the case of uninitialized variables the block is set to nullptr by
+    // SsaBuilder and changed to fgFirstBB during value numbering. It would be useful to
+    // investigate and perhaps eliminate this rather unexpected behavior.
+    BasicBlock* m_block;
+    // The GT_ASG node that generates the definition, or nullptr for definitions
+    // of uninitialized variables.
+    GenTreeOp* m_asg;
 
-    DefLoc() : m_blk(nullptr), m_tree(nullptr)
+public:
+    LclSsaVarDsc() : m_block(nullptr), m_asg(nullptr)
     {
     }
 
-    DefLoc(BasicBlock* block, GenTree* tree) : m_blk(block), m_tree(tree)
+    LclSsaVarDsc(BasicBlock* block, GenTreeOp* asg) : m_block(block), m_asg(asg)
     {
+        assert((asg == nullptr) || asg->OperIs(GT_ASG));
     }
-};
 
-// This class stores information associated with a LclVar SSA definition.
-class LclSsaVarDsc
-{
-public:
-    LclSsaVarDsc()
+    BasicBlock* GetBlock() const
+    {
+        return m_block;
+    }
+
+    void SetBlock(BasicBlock* block)
+    {
+        m_block = block;
+    }
+
+    GenTreeOp* GetAssignment() const
     {
+        return m_asg;
     }
 
-    LclSsaVarDsc(BasicBlock* block, GenTree* tree) : m_defLoc(block, tree)
+    void SetAssignment(GenTreeOp* asg)
     {
+        assert((asg == nullptr) || asg->OperIs(GT_ASG));
+        m_asg = asg;
     }
 
     ValueNumPair m_vnPair;
-    DefLoc       m_defLoc;
 };
 
 // This class stores information associated with a memory SSA definition.

src/jit/earlyprop.cpp

@@ -424,56 +424,51 @@ GenTree* Compiler::optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropK
     }
 
     // Track along the use-def chain to get the array length
-    GenTree* treelhs = lvaTable[lclNum].GetPerSsaData(ssaNum)->m_defLoc.m_tree;
+    GenTreeOp* ssaDefAsg = lvaTable[lclNum].GetPerSsaData(ssaNum)->GetAssignment();
 
-    if (treelhs == nullptr)
+    if (ssaDefAsg == nullptr)
     {
         // Incoming parameters or live-in variables don't have actual definition tree node
         // for their FIRST_SSA_NUM. See SsaBuilder::RenameVariables.
         assert(ssaNum == SsaConfig::FIRST_SSA_NUM);
     }
     else
     {
-        GenTree** lhsPtr;
-        GenTree*  treeDefParent = treelhs->gtGetParent(&lhsPtr);
+        assert(ssaDefAsg->OperIs(GT_ASG));
 
-        if (treeDefParent->OperGet() == GT_ASG)
-        {
-            assert(treelhs == treeDefParent->gtGetOp1());
-            GenTree* treeRhs = treeDefParent->gtGetOp2();
+        GenTree* treeRhs = ssaDefAsg->gtGetOp2();
 
-            if (treeRhs->OperIsScalarLocal() && lvaInSsa(treeRhs->AsLclVarCommon()->GetLclNum()))
-            {
-                // Recursively track the Rhs
-                unsigned rhsLclNum = treeRhs->AsLclVarCommon()->GetLclNum();
-                unsigned rhsSsaNum = treeRhs->AsLclVarCommon()->GetSsaNum();
+        if (treeRhs->OperIsScalarLocal() && lvaInSsa(treeRhs->AsLclVarCommon()->GetLclNum()))
+        {
+            // Recursively track the Rhs
+            unsigned rhsLclNum = treeRhs->AsLclVarCommon()->GetLclNum();
+            unsigned rhsSsaNum = treeRhs->AsLclVarCommon()->GetSsaNum();
 
-                value = optPropGetValueRec(rhsLclNum, rhsSsaNum, valueKind, walkDepth + 1);
-            }
-            else
+            value = optPropGetValueRec(rhsLclNum, rhsSsaNum, valueKind, walkDepth + 1);
+        }
+        else
+        {
+            if (valueKind == optPropKind::OPK_ARRAYLEN)
             {
-                if (valueKind == optPropKind::OPK_ARRAYLEN)
+                value = getArrayLengthFromAllocation(treeRhs);
+                if (value != nullptr)
                 {
-                    value = getArrayLengthFromAllocation(treeRhs);
-                    if (value != nullptr)
+                    if (!value->IsCnsIntOrI())
                     {
-                        if (!value->IsCnsIntOrI())
-                        {
-                            // Leave out non-constant-sized array
-                            value = nullptr;
-                        }
+                        // Leave out non-constant-sized array
+                        value = nullptr;
                     }
                 }
-                else if (valueKind == optPropKind::OPK_OBJ_GETTYPE)
+            }
+            else if (valueKind == optPropKind::OPK_OBJ_GETTYPE)
+            {
+                value = getObjectHandleNodeFromAllocation(treeRhs);
+                if (value != nullptr)
                 {
-                    value = getObjectHandleNodeFromAllocation(treeRhs);
-                    if (value != nullptr)
+                    if (!value->IsCnsIntOrI())
                     {
-                        if (!value->IsCnsIntOrI())
-                        {
-                            // Leave out non-constant-sized array
-                            value = nullptr;
-                        }
+                        // Leave out non-constant-sized array
+                        value = nullptr;
                     }
                 }
             }
@@ -548,15 +543,15 @@ void Compiler::optFoldNullCheck(GenTree* tree)
 
         if (ssaNum != SsaConfig::RESERVED_SSA_NUM)
         {
-            DefLoc      defLoc   = lvaTable[lclNum].GetPerSsaData(ssaNum)->m_defLoc;
-            BasicBlock* defBlock = defLoc.m_blk;
+            LclSsaVarDsc* defLoc   = lvaTable[lclNum].GetPerSsaData(ssaNum);
+            BasicBlock*   defBlock = defLoc->GetBlock();
 
             if (compCurBB == defBlock)
             {
-                GenTree* defTree   = defLoc.m_tree;
-                GenTree* defParent = defTree->gtGetParent(nullptr);
+                GenTree* defParent = defLoc->GetAssignment();
+                assert(defParent->OperIs(GT_ASG));
 
-                if ((defParent->OperGet() == GT_ASG) && (defParent->gtNext == nullptr))
+                if (defParent->gtNext == nullptr)
                 {
                     GenTree* defRHS = defParent->gtGetOp2();
                     if (defRHS->OperGet() == GT_COMMA)

src/jit/optcse.cpp

@@ -2375,12 +2375,12 @@ class CSE_Heuristic
 
                     // These should not have been set yet, since this is the first and
                     // only def for this CSE.
-                    assert(ssaVarDsc->m_defLoc.m_blk == nullptr);
-                    assert(ssaVarDsc->m_defLoc.m_tree == nullptr);
+                    assert(ssaVarDsc->GetBlock() == nullptr);
+                    assert(ssaVarDsc->GetAssignment() == nullptr);
 
-                    ssaVarDsc->m_vnPair        = val->gtVNPair;
-                    ssaVarDsc->m_defLoc.m_blk  = blk;
-                    ssaVarDsc->m_defLoc.m_tree = asg->AsOp()->gtOp1;
+                    ssaVarDsc->m_vnPair = val->gtVNPair;
+                    ssaVarDsc->SetBlock(blk);
+                    ssaVarDsc->SetAssignment(asg->AsOp());
                 }
 
                 /* Create a reference to the CSE temp */

src/jit/optimizer.cpp

@@ -6871,7 +6871,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack<BasicBlock*>* blo
                 // and the SSA definition must be outside the loop we're hoisting from ...
                 isInvariant = isInvariant &&
                               !m_compiler->optLoopTable[m_loopNum].lpContains(
-                                  m_compiler->lvaGetDesc(lclNum)->GetPerSsaData(lclVar->GetSsaNum())->m_defLoc.m_blk);
+                                  m_compiler->lvaGetDesc(lclNum)->GetPerSsaData(lclVar->GetSsaNum())->GetBlock());
                 // and the VN of the tree is considered invariant as well.
                 //
                 // TODO-CQ: This VN invariance check should not be necessary and in some cases it is conservative - it
@@ -7260,7 +7260,7 @@ bool Compiler::optVNIsLoopInvariant(ValueNum vn, unsigned lnum, VNToBoolMap* loo
             unsigned      lclNum = funcApp.m_args[0];
             unsigned      ssaNum = funcApp.m_args[1];
             LclSsaVarDsc* ssaDef = lvaTable[lclNum].GetPerSsaData(ssaNum);
-            res                  = !optLoopContains(lnum, ssaDef->m_defLoc.m_blk->bbNatLoopNum);
+            res                  = !optLoopContains(lnum, ssaDef->GetBlock()->bbNatLoopNum);
         }
         else if (funcApp.m_func == VNF_PhiMemoryDef)
         {

src/jit/rangecheck.cpp

@@ -395,9 +395,9 @@ bool RangeCheck::IsMonotonicallyIncreasing(GenTree* expr, bool rejectNegativeCon
     // If the rhs expr is local, then try to find the def of the local.
     else if (expr->IsLocal())
     {
-        BasicBlock* asgBlock;
-        GenTreeOp*  asg = GetSsaDefAsg(expr->AsLclVarCommon(), &asgBlock);
-        return (asg != nullptr) && IsMonotonicallyIncreasing(asg->gtGetOp2(), rejectNegativeConst);
+        LclSsaVarDsc* ssaDef = GetSsaDefAsg(expr->AsLclVarCommon());
+        return (ssaDef != nullptr) &&
+               IsMonotonicallyIncreasing(ssaDef->GetAssignment()->gtGetOp2(), rejectNegativeConst);
     }
     else if (expr->OperGet() == GT_ADD)
     {
@@ -425,7 +425,7 @@ bool RangeCheck::IsMonotonicallyIncreasing(GenTree* expr, bool rejectNegativeCon
 }
 
 // Given a lclvar use, try to find the lclvar's defining assignment and its containing block.
-GenTreeOp* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse, BasicBlock** asgBlock)
+LclSsaVarDsc* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse)
 {
     unsigned ssaNum = lclUse->GetSsaNum();
 
@@ -434,33 +434,28 @@ GenTreeOp* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse, BasicBlock** as
         return nullptr;
     }
 
-    LclSsaVarDsc* ssaData = m_pCompiler->lvaTable[lclUse->GetLclNum()].GetPerSsaData(ssaNum);
-    GenTree*      lclDef  = ssaData->m_defLoc.m_tree;
+    LclSsaVarDsc* ssaDef = m_pCompiler->lvaGetDesc(lclUse)->GetPerSsaData(ssaNum);
 
-    if (lclDef == nullptr)
+    // RangeCheck does not care about uninitialized variables.
+    if (ssaDef->GetAssignment() == nullptr)
     {
         return nullptr;
     }
 
-    // We have the def node but we also need the assignment node to get its source.
-    // gtGetParent can be used to get the assignment node but it's rather expensive
-    // and not strictly necessary here, there shouldn't be any other node between
-    // the assignment node and its destination node.
-    GenTree* asg = lclDef->gtNext;
-
-    if (!asg->OperIs(GT_ASG) || (asg->gtGetOp1() != lclDef))
+    // RangeCheck does not understand definitions generated by LCL_FLD nodes
+    // nor definitions generated by indirect stores to local variables.
+    if (!ssaDef->GetAssignment()->gtGetOp1()->OperIs(GT_LCL_VAR))
     {
         return nullptr;
     }
 
 #ifdef DEBUG
     Location* loc = GetDef(lclUse);
-    assert(loc->parent == asg);
-    assert(loc->block == ssaData->m_defLoc.m_blk);
+    assert(loc->parent == ssaDef->GetAssignment());
+    assert(loc->block == ssaDef->GetBlock());
 #endif
 
-    *asgBlock = ssaData->m_defLoc.m_blk;
-    return asg->AsOp();
+    return ssaDef;
 }
 
 #ifdef DEBUG
@@ -838,27 +833,26 @@ Range RangeCheck::ComputeRangeForLocalDef(BasicBlock*          block,
                                           GenTreeLclVarCommon* lcl,
                                           bool monotonic DEBUGARG(int indent))
 {
-    BasicBlock* asgBlock;
-    GenTreeOp*  asg = GetSsaDefAsg(lcl, &asgBlock);
-    if (asg == nullptr)
+    LclSsaVarDsc* ssaDef = GetSsaDefAsg(lcl);
+    if (ssaDef == nullptr)
     {
         return Range(Limit(Limit::keUnknown));
     }
 #ifdef DEBUG
     if (m_pCompiler->verbose)
     {
         JITDUMP("----------------------------------------------------\n");
-        m_pCompiler->gtDispTree(asg);
+        m_pCompiler->gtDispTree(ssaDef->GetAssignment());
         JITDUMP("----------------------------------------------------\n");
     }
 #endif
-    assert(asg->OperIs(GT_ASG));
-    Range range = GetRange(asgBlock, asg->gtGetOp2(), monotonic DEBUGARG(indent));
+    Range range = GetRange(ssaDef->GetBlock(), ssaDef->GetAssignment()->gtGetOp2(), monotonic DEBUGARG(indent));
     if (!BitVecOps::MayBeUninit(block->bbAssertionIn))
     {
         JITDUMP("Merge assertions from " FMT_BB ":%s for assignment about [%06d]\n", block->bbNum,
-                BitVecOps::ToString(m_pCompiler->apTraits, block->bbAssertionIn), Compiler::dspTreeID(asg->gtGetOp1()));
-        MergeEdgeAssertions(asg->gtGetOp1()->AsLclVarCommon(), block->bbAssertionIn, &range);
+                BitVecOps::ToString(m_pCompiler->apTraits, block->bbAssertionIn),
+                Compiler::dspTreeID(ssaDef->GetAssignment()->gtGetOp1()));
+        MergeEdgeAssertions(ssaDef->GetAssignment()->gtGetOp1()->AsLclVarCommon(), block->bbAssertionIn, &range);
         JITDUMP("done merging\n");
     }
     return range;
@@ -973,9 +967,8 @@ bool RangeCheck::DoesBinOpOverflow(BasicBlock* block, GenTreeOp* binop)
 // Check if the var definition the rhs involves arithmetic that overflows.
 bool RangeCheck::DoesVarDefOverflow(GenTreeLclVarCommon* lcl)
 {
-    BasicBlock* asgBlock;
-    GenTreeOp*  asg = GetSsaDefAsg(lcl, &asgBlock);
-    return (asg == nullptr) || DoesOverflow(asgBlock, asg->gtGetOp2());
+    LclSsaVarDsc* ssaDef = GetSsaDefAsg(lcl);
+    return (ssaDef == nullptr) || DoesOverflow(ssaDef->GetBlock(), ssaDef->GetAssignment()->gtGetOp2());
 }
 
 bool RangeCheck::DoesPhiOverflow(BasicBlock* block, GenTree* expr)

src/jit/rangecheck.h

@@ -523,7 +523,7 @@ class RangeCheck
 
 private:
     // Given a lclvar use, try to find the lclvar's defining assignment and its containing block.
-    GenTreeOp* GetSsaDefAsg(GenTreeLclVarCommon* lclUse, BasicBlock** asgBlock);
+    LclSsaVarDsc* GetSsaDefAsg(GenTreeLclVarCommon* lclUse);
 
     GenTreeBoundsChk* m_pCurBndsChk;