Revert "Use SSA def descriptors in copy propagation (dotnet#65250)"

This reverts commit 9012b23.

Author: BruceForstall

src/coreclr/jit/compiler.h

@@ -352,13 +352,6 @@ class SsaDefArray
         assert(ssaNum != SsaConfig::RESERVED_SSA_NUM);
         return GetSsaDefByIndex(ssaNum - GetMinSsaNum());
     }
-
-    // Get an SSA number associated with the specified SSA def (that must be in this array).
-    unsigned GetSsaNum(T* ssaDef)
-    {
-        assert((m_array <= ssaDef) && (ssaDef < &m_array[m_count]));
-        return GetMinSsaNum() + static_cast<unsigned>(ssaDef - &m_array[0]);
-    }
 };
 
 enum RefCountState
@@ -1089,13 +1082,6 @@ class LclVarDsc
         return lvPerSsaData.GetSsaDef(ssaNum);
     }
 
-    // Returns the SSA number for "ssaDef". Requires "ssaDef" to be a valid definition
-    // of this variable.
-    unsigned GetSsaNumForSsaDef(LclSsaVarDsc* ssaDef)
-    {
-        return lvPerSsaData.GetSsaNum(ssaDef);
-    }
-
     var_types GetRegisterType(const GenTreeLclVarCommon* tree) const;
 
     var_types GetRegisterType() const;
@@ -7414,54 +7400,17 @@ class Compiler
 
 public:
     // VN based copy propagation.
-
-    // In DEBUG builds, we'd like to know the tree that the SSA definition was pushed for.
-    // While for ordinary SSA defs it will be available (as an ASG) in the SSA descriptor,
-    // for locals which will use "definitions from uses", it will not be, so we store it
-    // in this class instead.
-    class CopyPropSsaDef
-    {
-        LclSsaVarDsc* m_ssaDef;
-#ifdef DEBUG
-        GenTree* m_defNode;
-#endif
-    public:
-        CopyPropSsaDef(LclSsaVarDsc* ssaDef, GenTree* defNode)
-            : m_ssaDef(ssaDef)
-#ifdef DEBUG
-            , m_defNode(defNode)
-#endif
-        {
-        }
-
-        LclSsaVarDsc* GetSsaDef() const
-        {
-            return m_ssaDef;
-        }
-
-#ifdef DEBUG
-        GenTree* GetDefNode() const
-        {
-            return m_defNode;
-        }
-#endif
-    };
-
-    typedef ArrayStack<CopyPropSsaDef> CopyPropSsaDefStack;
-    typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, CopyPropSsaDefStack*> LclNumToLiveDefsMap;
+    typedef ArrayStack<GenTree*> GenTreePtrStack;
+    typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, GenTreePtrStack*> LclNumToGenTreePtrStack;
 
     // Copy propagation functions.
-    void optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName);
-    void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName);
-    void optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName);
-    void optCopyPropPushDef(GenTreeOp*           asg,
-                            GenTreeLclVarCommon* lclNode,
-                            unsigned             lclNum,
-                            LclNumToLiveDefsMap* curSsaName);
+    void optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToGenTreePtrStack* curSsaName);
+    void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
+    void optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName);
     unsigned optIsSsaLocal(GenTreeLclVarCommon* lclNode);
-    int optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2);
+    int optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2);
     void optVnCopyProp();
-    INDEBUG(void optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName));
+    INDEBUG(void optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName));
 
     /**************************************************************************
      *               Early value propagation

src/coreclr/jit/copyprop.cpp

@@ -26,7 +26,7 @@
  * of the graph originating from the block. Refer SSA renaming for any additional info.
  * "curSsaName" tracks the currently live definitions.
  */
-void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName)
+void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName)
 {
     for (Statement* const stmt : block->Statements())
     {
@@ -42,7 +42,7 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap*
                     continue;
                 }
 
-                CopyPropSsaDefStack* stack = nullptr;
+                GenTreePtrStack* stack = nullptr;
                 curSsaName->Lookup(lclNum, &stack);
                 stack->Pop();
                 if (stack->Empty())
@@ -55,12 +55,12 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap*
 }
 
 #ifdef DEBUG
-void Compiler::optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName)
+void Compiler::optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName)
 {
     JITDUMP("{ ");
-    for (LclNumToLiveDefsMap::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
+    for (LclNumToGenTreePtrStack::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
     {
-        GenTreeLclVarCommon* lclVar    = iter.GetValue()->Top().GetDefNode()->AsLclVarCommon();
+        GenTreeLclVarCommon* lclVar    = iter.GetValue()->Top()->AsLclVarCommon();
         unsigned             ssaLclNum = optIsSsaLocal(lclVar);
         assert(ssaLclNum != BAD_VAR_NUM);
 
@@ -82,7 +82,7 @@ void Compiler::optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName)
  * Given the "lclVar" and "copyVar" compute if the copy prop will be beneficial.
  *
  */
-int Compiler::optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2)
+int Compiler::optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2)
 {
     int score = 0;
 
@@ -126,17 +126,16 @@ int Compiler::optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDs
 //    tree        -  The local tree to perform copy propagation on
 //    lclNum      -  The local number of said tree
 //    curSsaName  -  The map from lclNum to its recently live definitions as a stack
-//
-void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToLiveDefsMap* curSsaName)
+
+void Compiler::optCopyProp(Statement*               stmt,
+                           GenTreeLclVarCommon*     tree,
+                           unsigned                 lclNum,
+                           LclNumToGenTreePtrStack* curSsaName)
 {
     assert((lclNum != BAD_VAR_NUM) && (optIsSsaLocal(tree) == lclNum) && ((tree->gtFlags & GTF_VAR_DEF) == 0));
-    assert(tree->gtVNPair.BothDefined());
+    assert(tree->gtVNPair.GetConservative() != ValueNumStore::NoVN);
 
-    LclVarDsc* varDsc   = lvaGetDesc(lclNum);
-    ValueNum   lclDefVN = varDsc->GetPerSsaData(tree->GetSsaNum())->m_vnPair.GetConservative();
-    assert(lclDefVN != ValueNumStore::NoVN);
-
-    for (LclNumToLiveDefsMap::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
+    for (LclNumToGenTreePtrStack::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter)
     {
         unsigned newLclNum = iter.Get();
 
@@ -146,30 +145,35 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned
             continue;
         }
 
-        CopyPropSsaDef newLclDef    = iter.GetValue()->Top();
-        LclSsaVarDsc*  newLclSsaDef = newLclDef.GetSsaDef();
+        LclVarDsc* varDsc        = lvaGetDesc(lclNum);
+        LclVarDsc* newLclVarDsc  = lvaGetDesc(newLclNum);
+        GenTree*   newLclDefNode = iter.GetValue()->Top(); // Note that this "def" node can actually be a use (for
+                                                           // parameters and other use-before-def locals).
 
-        // Likewise, nothing to do if the most recent def is not available.
-        if (newLclSsaDef == nullptr)
+        // Do not copy propagate if the old and new lclVar have different 'doNotEnregister' settings.
+        // This is primarily to avoid copy propagating to IND(ADDR(LCL_VAR)) where the replacement lclVar
+        // is not marked 'lvDoNotEnregister'.
+        // However, in addition, it may not be profitable to propagate a 'doNotEnregister' lclVar to an
+        // existing use of an enregisterable lclVar.
+        if (varDsc->lvDoNotEnregister != newLclVarDsc->lvDoNotEnregister)
         {
             continue;
         }
 
-        ValueNum newLclDefVN = newLclSsaDef->m_vnPair.GetConservative();
-        assert(newLclDefVN != ValueNumStore::NoVN);
+        if ((gsShadowVarInfo != nullptr) && newLclVarDsc->lvIsParam &&
+            (gsShadowVarInfo[newLclNum].shadowCopy != BAD_VAR_NUM))
+        {
+            continue;
+        }
 
-        if (newLclDefVN != lclDefVN)
+        ValueNum newLclDefVN = GetUseAsgDefVNOrTreeVN(newLclDefNode);
+        if (newLclDefVN == ValueNumStore::NoVN)
         {
             continue;
         }
 
-        // Do not copy propagate if the old and new lclVar have different 'doNotEnregister' settings.
-        // This is primarily to avoid copy propagating to IND(ADDR(LCL_VAR)) where the replacement lclVar
-        // is not marked 'lvDoNotEnregister'.
-        // However, in addition, it may not be profitable to propagate a 'doNotEnregister' lclVar to an
-        // existing use of an enregisterable lclVar.
-        LclVarDsc* newLclVarDsc = lvaGetDesc(newLclNum);
-        if (varDsc->lvDoNotEnregister != newLclVarDsc->lvDoNotEnregister)
+        ValueNum lclDefVN = varDsc->GetPerSsaData(tree->GetSsaNum())->m_vnPair.GetConservative();
+        if (newLclDefVN != lclDefVN)
         {
             continue;
         }
@@ -178,7 +182,6 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned
         {
             continue;
         }
-
         // Check whether the newLclNum is live before being substituted. Otherwise, we could end
         // up in a situation where there must've been a phi node that got pruned because the variable
         // is not live anymore. For example,
@@ -194,7 +197,7 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned
         // 'c' with 'x.'
 
         // We compute liveness only on tracked variables. And all SSA locals are tracked.
-        assert(newLclVarDsc->lvTracked);
+        assert(lvaGetDesc(newLclNum)->lvTracked);
 
         // Because of this dependence on live variable analysis, CopyProp phase is immediately
         // after Liveness, SSA and VN.
@@ -203,6 +206,21 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned
             continue;
         }
 
+        unsigned newSsaNum = SsaConfig::RESERVED_SSA_NUM;
+        if (newLclDefNode->gtFlags & GTF_VAR_DEF)
+        {
+            newSsaNum = GetSsaNumForLocalVarDef(newLclDefNode);
+        }
+        else // parameters, this pointer etc.
+        {
+            newSsaNum = newLclDefNode->AsLclVarCommon()->GetSsaNum();
+        }
+
+        if (newSsaNum == SsaConfig::RESERVED_SSA_NUM)
+        {
+            continue;
+        }
+
         var_types newLclType = newLclVarDsc->TypeGet();
         if (!newLclVarDsc->lvNormalizeOnLoad())
         {
@@ -220,15 +238,12 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned
             JITDUMP("VN based copy assertion for ");
             printTreeID(tree);
             printf(" V%02d " FMT_VN " by ", lclNum, lclDefVN);
-            printTreeID(newLclDef.GetDefNode());
+            printTreeID(newLclDefNode);
             printf(" V%02d " FMT_VN ".\n", newLclNum, newLclDefVN);
             DISPNODE(tree);
         }
 #endif
 
-        unsigned newSsaNum = newLclVarDsc->GetSsaNumForSsaDef(newLclSsaDef);
-        assert(newSsaNum != SsaConfig::RESERVED_SSA_NUM);
-
         tree->AsLclVarCommon()->SetLclNum(newLclNum);
         tree->AsLclVarCommon()->SetSsaNum(newSsaNum);
         gtUpdateSideEffects(stmt, tree);
@@ -273,74 +288,6 @@ unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode)
     return lclNum;
 }
 
-//------------------------------------------------------------------------------
-// optCopyPropPushDef: Push the new live SSA def on the stack for "lclNode".
-//
-// Arguments:
-//    asg        - The assignment node for this def (will be "nullptr" for "use" defs)
-//    lclNode    - The local tree representing "the def" (that can actually be a use)
-//    lclNum     - The local's number (see "optIsSsaLocal")
-//    curSsaName - The map of local numbers to stacks of their defs
-//
-void Compiler::optCopyPropPushDef(GenTreeOp*           asg,
-                                  GenTreeLclVarCommon* lclNode,
-                                  unsigned             lclNum,
-                                  LclNumToLiveDefsMap* curSsaName)
-{
-    assert((lclNum != BAD_VAR_NUM) && (lclNum == optIsSsaLocal(lclNode)));
-
-    // Shadowed parameters are special: they will (at most) have one use, that is one on the RHS of an
-    // assignment to their shadow, and we must not substitute them anywhere. So we'll not push any defs.
-    if ((gsShadowVarInfo != nullptr) && lvaGetDesc(lclNum)->lvIsParam &&
-        (gsShadowVarInfo[lclNum].shadowCopy != BAD_VAR_NUM))
-    {
-        assert(!curSsaName->Lookup(lclNum));
-        return;
-    }
-
-    unsigned ssaDefNum = SsaConfig::RESERVED_SSA_NUM;
-    if (asg == nullptr)
-    {
-        // Parameters, this pointer etc.
-        assert((lclNode->gtFlags & GTF_VAR_DEF) == 0);
-        assert(lclNode->GetSsaNum() == SsaConfig::FIRST_SSA_NUM);
-        ssaDefNum = lclNode->GetSsaNum();
-    }
-    else
-    {
-        assert((lclNode->gtFlags & GTF_VAR_DEF) != 0);
-        ssaDefNum = GetSsaNumForLocalVarDef(lclNode);
-
-        // This will be "RESERVED_SSA_NUM" for promoted struct fields assigned using the parent struct.
-        // TODO-CQ: fix this.
-        assert((ssaDefNum != SsaConfig::RESERVED_SSA_NUM) || lvaGetDesc(lclNode)->CanBeReplacedWithItsField(this));
-    }
-
-    // The default is "not available".
-    LclSsaVarDsc* ssaDef = nullptr;
-    if (ssaDefNum != SsaConfig::RESERVED_SSA_NUM)
-    {
-        // This code preserves previous behavior. In principle, "ssaDefVN" should
-        // always be obtained directly from the SSA def descriptor and be valid.
-        ValueNum ssaDefVN = GetUseAsgDefVNOrTreeVN(lclNode);
-        assert(ssaDefVN != ValueNumStore::NoVN);
-
-        if (ssaDefVN != ValueNumStore::VNForVoid())
-        {
-            ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaDefNum);
-        }
-    }
-
-    CopyPropSsaDefStack* defStack;
-    if (!curSsaName->Lookup(lclNum, &defStack))
-    {
-        defStack = new (curSsaName->GetAllocator()) CopyPropSsaDefStack(curSsaName->GetAllocator());
-        curSsaName->Set(lclNum, defStack);
-    }
-
-    defStack->Push(CopyPropSsaDef(ssaDef, lclNode));
-}
-
 //------------------------------------------------------------------------------
 // optBlockCopyProp : Perform copy propagation using currently live definitions on the current block's
 //                    variables. Also as new definitions are encountered update the "curSsaName" which
@@ -349,8 +296,8 @@ void Compiler::optCopyPropPushDef(GenTreeOp*           asg,
 // Arguments:
 //    block       -  Block the tree belongs to
 //    curSsaName  -  The map from lclNum to its recently live definitions as a stack
-//
-void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName)
+
+void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName)
 {
 #ifdef DEBUG
     JITDUMP("Copy Assertion for " FMT_BB "\n", block->bbNum);
@@ -386,7 +333,13 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa
                     continue;
                 }
 
-                optCopyPropPushDef(tree->AsOp(), lclDefNode, lclNum, curSsaName);
+                GenTreePtrStack* stack;
+                if (!curSsaName->Lookup(lclNum, &stack))
+                {
+                    stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator());
+                }
+                stack->Push(lclDefNode);
+                curSsaName->Set(lclNum, stack, LclNumToGenTreePtrStack::Overwrite);
             }
             // TODO-CQ: propagate on LCL_FLDs too.
             else if (tree->OperIs(GT_LCL_VAR) && ((tree->gtFlags & GTF_VAR_DEF) == 0))
@@ -398,11 +351,19 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa
                     continue;
                 }
 
-                // If we encounter first use of a param or this pointer add it as a
-                // live definition. Since they are always live, we'll do it only once.
-                if ((lvaGetDesc(lclNum)->lvIsParam || (lclNum == info.compThisArg)) && !curSsaName->Lookup(lclNum))
+                // If we encounter first use of a param or this pointer add it as a live definition.
+                // Since they are always live, we'll do it only once.
+                if (lvaGetDesc(lclNum)->lvIsParam || (lclNum == info.compThisArg))
                 {
-                    optCopyPropPushDef(nullptr, tree->AsLclVarCommon(), lclNum, curSsaName);
+                    GenTreePtrStack* stack;
+                    if (!curSsaName->Lookup(lclNum, &stack))
+                    {
+                        assert(tree->AsLclVarCommon()->GetSsaNum() == SsaConfig::FIRST_SSA_NUM);
+
+                        stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator());
+                        stack->Push(tree);
+                        curSsaName->Set(lclNum, stack);
+                    }
                 }
 
                 // TODO-Review: EH successor/predecessor iteration seems broken.
@@ -461,7 +422,7 @@ void Compiler::optVnCopyProp()
     class CopyPropDomTreeVisitor : public DomTreeVisitor<CopyPropDomTreeVisitor>
     {
         // The map from lclNum to its recently live definitions as a stack.
-        LclNumToLiveDefsMap m_curSsaName;
+        LclNumToGenTreePtrStack m_curSsaName;
 
     public:
         CopyPropDomTreeVisitor(Compiler* compiler)