Remove useless IndirectAssignmentAnnotation

This annotation is supposed to be used by SSA and VN but that does not actually happen because fgMorphCopyBlock also marks the destination local as address exposed.

Also, even if the local wasn't address exposed there are other parts of the JIT that should probably use this annotation but currently they do not:
* Copy propagation attempts to mirror SSA renaming but it ignores IndirectAssignmentAnnotation.
* Liveness also doesn't pay attention to IndirectAssignmentAnnotation. A comment in fgMorphCopyBlock even gives liveness as the reason why the destination local is address exposed.

7 files changed, 57 insertions, 230 deletions

diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp
index cec1dd361d..a1befb3a5d 100644
--- a/src/jit/compiler.cpp
+++ b/src/jit/compiler.cpp
@@ -2070,7 +2070,6 @@ void Compiler::compInit(ArenaAllocator* pAlloc, InlineInfo* inlineInfo)
 
     vnStore               = nullptr;
     m_opAsgnVarDefSsaNums = nullptr;
-    m_indirAssignMap      = nullptr;
     fgSsaPassesCompleted  = 0;
     fgVNPassesCompleted   = 0;
 
diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index a401c47aa8..f25e961225 100644
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -4011,58 +4011,6 @@ public:
     // rather than the "use" SSA number recorded in the tree "lcl".
     inline unsigned GetSsaNumForLocalVarDef(GenTree* lcl);
 
-    // Some assignments assign to a local "indirectly": they are part of a comma expression that takes the address
-    // of the local (or a field thereof), assigns this address to a temp, and uses an indirection of this temp as
-    // the LHS of the assignment.  This actually arises in exactly one situation.  At the source level we assign one
-    // struct local to another: "s1 = s2".  This becomes a copyblk.  If "s2" is promoted into  field variables "s2f0",
-    // ..."s2fn", then the copyblk will morph to a comma expression that takes the address of "s1" and does field-wise
-    // assignments:
-    //   (byref addrS1 = &s1,
-    //    *(addrS1 * offsetof(f0)) = s2f0,
-    //    ...
-    //    *(addrS1 * offsetof(fn)) = s2fn)
-    //
-    // It would be a shame, given the simple form at the source level, to be unable to track the values in the
-    // fields of "s1" after this.  But "s1" does not appear in the assignments that modify it.  How, then, to
-    // give it SSA names and value numbers?
-    //
-    // The solution is to use the side table described below to annotate each of the field-wise assignments at the
-    // end with an instance of the structure below, whose fields are described in the declaration.
-    struct IndirectAssignmentAnnotation
-    {
-        unsigned      m_lclNum;   // The local num that is being indirectly assigned.
-        FieldSeqNode* m_fieldSeq; // If the LHS of the struct assignment is itself a struct field dereference,
-                                  // as in "s0.g = s2", then "m_lclNum" would be "s0", and "m_fieldSeq" would
-                                  // be the singleton field sequence "g".  The individual assignments would
-                                  // further append the fields of "s.g" to that.
-        bool m_isEntire;          // True iff this assignment writes all of m_lclNum.  (This can occur if the
-                                  // structure has a single field).
-        unsigned m_defSsaNum;     // The new SSA number of "m_lclNum" after the assignment.
-        unsigned m_useSsaNum;     // Only valid if "m_isEntire" is false; if so, the SSA number of "m_lclNum" before the
-                                  // assignment.
-
-        IndirectAssignmentAnnotation(unsigned      lclNum,
-                                     FieldSeqNode* fldSeq,
-                                     bool          isEntire,
-                                     unsigned      defSsaNum = SsaConfig::RESERVED_SSA_NUM,
-                                     unsigned      useSsaNum = SsaConfig::RESERVED_SSA_NUM)
-            : m_lclNum(lclNum), m_fieldSeq(fldSeq), m_isEntire(isEntire), m_defSsaNum(defSsaNum), m_useSsaNum(useSsaNum)
-        {
-        }
-    };
-    typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, IndirectAssignmentAnnotation*> NodeToIndirAssignMap;
-    NodeToIndirAssignMap* m_indirAssignMap;
-    NodeToIndirAssignMap* GetIndirAssignMap()
-    {
-        if (m_indirAssignMap == nullptr)
-        {
-            // Create a CompAllocator that labels sub-structure with CMK_IndirAssignMap, and use that for allocation.
-            CompAllocator* ialloc = new (this, CMK_IndirAssignMap) CompAllocator(this, CMK_IndirAssignMap);
-            m_indirAssignMap      = new (ialloc) NodeToIndirAssignMap(ialloc);
-        }
-        return m_indirAssignMap;
-    }
-
     // Performs SSA conversion.
     void fgSsaBuild();
 
diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp
index 284b420120..46ec3f4d18 100644
--- a/src/jit/gentree.cpp
+++ b/src/jit/gentree.cpp
@@ -7992,19 +7992,6 @@ GenTree* Compiler::gtCloneExpr(
         // Copy any node annotations, if necessary.
         switch (tree->gtOper)
         {
-            case GT_ASG:
-            {
-                IndirectAssignmentAnnotation* pIndirAnnot = nullptr;
-                if (m_indirAssignMap != nullptr && GetIndirAssignMap()->Lookup(tree, &pIndirAnnot))
-                {
-                    IndirectAssignmentAnnotation* pNewIndirAnnot = new (this, CMK_Unknown)
-                        IndirectAssignmentAnnotation(pIndirAnnot->m_lclNum, pIndirAnnot->m_fieldSeq,
-                                                     pIndirAnnot->m_isEntire);
-                    GetIndirAssignMap()->Set(copy, pNewIndirAnnot);
-                }
-            }
-            break;
-
             case GT_STOREIND:
             case GT_IND:
             case GT_OBJ:
@@ -11534,20 +11521,6 @@ void Compiler::gtDispTree(GenTree*     tree,
         }
 #endif // FEATURE_PUT_STRUCT_ARG_STK
 
-        IndirectAssignmentAnnotation* pIndirAnnote;
-        if (tree->gtOper == GT_ASG && GetIndirAssignMap()->Lookup(tree, &pIndirAnnote))
-        {
-            printf("  indir assign of V%02d:", pIndirAnnote->m_lclNum);
-            if (pIndirAnnote->m_isEntire)
-            {
-                printf("d:%d", pIndirAnnote->m_defSsaNum);
-            }
-            else
-            {
-                printf("ud:%d->%d", pIndirAnnote->m_useSsaNum, pIndirAnnote->m_defSsaNum);
-            }
-        }
-
         if (tree->gtOper == GT_INTRINSIC)
         {
             switch (tree->gtIntrinsic.gtIntrinsicId)
diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp
index 3b3247ec00..3eb0aeeec6 100644
--- a/src/jit/morph.cpp
+++ b/src/jit/morph.cpp
@@ -11122,14 +11122,11 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
             asg = gtNewAssignNode(dest, src);
 
             // If we spilled the address, and we didn't do individual field assignments to promoted fields,
-            // and it was of a local, record the assignment as an indirect update of a local.
+            // and it was of a local, ensure that the destination local variable has been marked as address
+            // exposed. Neither liveness nor SSA are able to track this kind of indirect assignments.
             if (addrSpill && !destDoFldAsg && destLclNum != BAD_VAR_NUM)
             {
-                curFieldSeq   = GetFieldSeqStore()->Append(destFldSeq, curFieldSeq);
-                bool isEntire = (genTypeSize(var_types(lvaTable[destLclNum].lvType)) == genTypeSize(dest->TypeGet()));
-                IndirectAssignmentAnnotation* pIndirAnnot =
-                    new (this, CMK_Unknown) IndirectAssignmentAnnotation(destLclNum, curFieldSeq, isEntire);
-                GetIndirAssignMap()->Set(asg, pIndirAnnot);
+                noway_assert(lvaTable[destLclNum].lvAddrExposed);
             }
 
 #if LOCAL_ASSERTION_PROP
diff --git a/src/jit/ssabuilder.cpp b/src/jit/ssabuilder.cpp
index 9cbf805b10..073f83cf41 100644
--- a/src/jit/ssabuilder.cpp
+++ b/src/jit/ssabuilder.cpp
@@ -120,16 +120,6 @@ void Compiler::fgResetForSsa()
                     tree->gtLclVarCommon.SetSsaNum(SsaConfig::RESERVED_SSA_NUM);
                     continue;
                 }
-
-                Compiler::IndirectAssignmentAnnotation* pIndirAssign = nullptr;
-                if ((tree->OperGet() != GT_ASG) || !GetIndirAssignMap()->Lookup(tree, &pIndirAssign) ||
-                    (pIndirAssign == nullptr))
-                {
-                    continue;
-                }
-
-                pIndirAssign->m_defSsaNum = SsaConfig::RESERVED_SSA_NUM;
-                pIndirAssign->m_useSsaNum = SsaConfig::RESERVED_SSA_NUM;
             }
         }
     }
@@ -847,24 +837,9 @@ void SsaBuilder::InsertPhiFunctions(BasicBlock** postOrder, int count)
  */
 void SsaBuilder::AddDefPoint(GenTree* tree, BasicBlock* blk)
 {
-    Compiler::IndirectAssignmentAnnotation* pIndirAnnot;
-    // In the case of an "indirect assignment", where the LHS is IND of a byref to the local actually being assigned,
-    // we make the ASG tree the def point.
-    assert(tree->IsLocal() || IsIndirectAssign(tree, &pIndirAnnot));
-    unsigned lclNum;
-    unsigned defSsaNum;
-    if (tree->IsLocal())
-    {
-        lclNum    = tree->gtLclVarCommon.gtLclNum;
-        defSsaNum = m_pCompiler->GetSsaNumForLocalVarDef(tree);
-    }
-    else
-    {
-        bool b = m_pCompiler->GetIndirAssignMap()->Lookup(tree, &pIndirAnnot);
-        assert(b);
-        lclNum    = pIndirAnnot->m_lclNum;
-        defSsaNum = pIndirAnnot->m_defSsaNum;
-    }
+    assert(tree->IsLocal());
+    unsigned lclNum    = tree->gtLclVarCommon.gtLclNum;
+    unsigned defSsaNum = m_pCompiler->GetSsaNumForLocalVarDef(tree);
 #ifdef DEBUG
     // Record that there's a new SSA def.
     m_pCompiler->lvaTable[lclNum].lvNumSsaNames++;
@@ -875,12 +850,6 @@ void SsaBuilder::AddDefPoint(GenTree* tree, BasicBlock* blk)
     ssaDef->m_defLoc.m_tree = tree;
 }
 
-bool SsaBuilder::IsIndirectAssign(GenTree* tree, Compiler::IndirectAssignmentAnnotation** ppIndirAssign)
-{
-    return tree->OperGet() == GT_ASG && m_pCompiler->m_indirAssignMap != nullptr &&
-           m_pCompiler->GetIndirAssignMap()->Lookup(tree, ppIndirAssign);
-}
-
 /**
  * Rename the local variable tree node.
  *
@@ -973,97 +942,74 @@ void SsaBuilder::TreeRenameVariables(GenTree* tree, BasicBlock* block, SsaRename
         }
     }
 
-    Compiler::IndirectAssignmentAnnotation* pIndirAssign = nullptr;
-    if (!tree->IsLocal() && !IsIndirectAssign(tree, &pIndirAssign))
+    if (!tree->IsLocal())
     {
         return;
     }
 
-    if (pIndirAssign != nullptr)
+    unsigned lclNum = tree->gtLclVarCommon.gtLclNum;
+    // Is this a variable we exclude from SSA?
+    if (m_pCompiler->fgExcludeFromSsa(lclNum))
     {
-        unsigned lclNum = pIndirAssign->m_lclNum;
-        // Is this a variable we exclude from SSA?
-        if (m_pCompiler->fgExcludeFromSsa(lclNum))
+        tree->gtLclVarCommon.SetSsaNum(SsaConfig::RESERVED_SSA_NUM);
+        return;
+    }
+
+    if (tree->gtFlags & GTF_VAR_DEF)
+    {
+        if (tree->gtFlags & GTF_VAR_USEASG)
         {
-            pIndirAssign->m_defSsaNum = SsaConfig::RESERVED_SSA_NUM;
-            return;
+            // This the "x" in something like "x op= y"; it is both a use (first), then a def.
+            // The def will define a new SSA name, and record that in "x".  If we need the SSA
+            // name of the use, we record it in a map reserved for that purpose.
+            unsigned count = pRenameState->CountForUse(lclNum);
+            tree->gtLclVarCommon.SetSsaNum(count);
         }
-        // Otherwise...
-        if (!pIndirAssign->m_isEntire)
+
+        // Give a count and increment.
+        unsigned count = pRenameState->CountForDef(lclNum);
+        if (tree->gtFlags & GTF_VAR_USEASG)
+        {
+            m_pCompiler->GetOpAsgnVarDefSsaNums()->Set(tree, count);
+        }
+        else
         {
-            pIndirAssign->m_useSsaNum = pRenameState->CountForUse(lclNum);
+            tree->gtLclVarCommon.SetSsaNum(count);
         }
-        unsigned count            = pRenameState->CountForDef(lclNum);
-        pIndirAssign->m_defSsaNum = count;
         pRenameState->Push(block, lclNum, count);
         AddDefPoint(tree, block);
-    }
-    else
-    {
-        unsigned lclNum = tree->gtLclVarCommon.gtLclNum;
-        // Is this a variable we exclude from SSA?
-        if (m_pCompiler->fgExcludeFromSsa(lclNum))
+
+        // If necessary, add "lclNum/count" to the arg list of a phi def in any
+        // handlers for try blocks that "block" is within.  (But only do this for "real" definitions,
+        // not phi definitions.)
+        if (!isPhiDefn)
         {
-            tree->gtLclVarCommon.SetSsaNum(SsaConfig::RESERVED_SSA_NUM);
-            return;
+            AddDefToHandlerPhis(block, lclNum, count);
         }
-
-        if (tree->gtFlags & GTF_VAR_DEF)
+    }
+    else if (!isPhiDefn) // Phi args already have ssa numbers.
+    {
+        // This case is obviated by the short-term "early-out" above...but it's in the right direction.
+        // Is it a promoted struct local?
+        if (m_pCompiler->lvaTable[lclNum].lvPromoted)
         {
-            if (tree->gtFlags & GTF_VAR_USEASG)
-            {
-                // This the "x" in something like "x op= y"; it is both a use (first), then a def.
-                // The def will define a new SSA name, and record that in "x".  If we need the SSA
-                // name of the use, we record it in a map reserved for that purpose.
-                unsigned count = pRenameState->CountForUse(lclNum);
-                tree->gtLclVarCommon.SetSsaNum(count);
-            }
-
-            // Give a count and increment.
-            unsigned count = pRenameState->CountForDef(lclNum);
-            if (tree->gtFlags & GTF_VAR_USEASG)
+            assert(tree->TypeGet() == TYP_STRUCT);
+            LclVarDsc* varDsc = &m_pCompiler->lvaTable[lclNum];
+            // If has only a single field var, treat this as a use of that field var.
+            // Otherwise, we don't give SSA names to uses of promoted struct vars.
+            if (varDsc->lvFieldCnt == 1)
             {
-                m_pCompiler->GetOpAsgnVarDefSsaNums()->Set(tree, count);
+                lclNum = varDsc->lvFieldLclStart;
             }
             else
             {
-                tree->gtLclVarCommon.SetSsaNum(count);
-            }
-            pRenameState->Push(block, lclNum, count);
-            AddDefPoint(tree, block);
-
-            // If necessary, add "lclNum/count" to the arg list of a phi def in any
-            // handlers for try blocks that "block" is within.  (But only do this for "real" definitions,
-            // not phi definitions.)
-            if (!isPhiDefn)
-            {
-                AddDefToHandlerPhis(block, lclNum, count);
+                tree->gtLclVarCommon.SetSsaNum(SsaConfig::RESERVED_SSA_NUM);
+                return;
             }
         }
-        else if (!isPhiDefn) // Phi args already have ssa numbers.
-        {
-            // This case is obviated by the short-term "early-out" above...but it's in the right direction.
-            // Is it a promoted struct local?
-            if (m_pCompiler->lvaTable[lclNum].lvPromoted)
-            {
-                assert(tree->TypeGet() == TYP_STRUCT);
-                LclVarDsc* varDsc = &m_pCompiler->lvaTable[lclNum];
-                // If has only a single field var, treat this as a use of that field var.
-                // Otherwise, we don't give SSA names to uses of promoted struct vars.
-                if (varDsc->lvFieldCnt == 1)
-                {
-                    lclNum = varDsc->lvFieldLclStart;
-                }
-                else
-                {
-                    tree->gtLclVarCommon.SetSsaNum(SsaConfig::RESERVED_SSA_NUM);
-                    return;
-                }
-            }
-            // Give the count as top of stack.
-            unsigned count = pRenameState->CountForUse(lclNum);
-            tree->gtLclVarCommon.SetSsaNum(count);
-        }
+        // Give the count as top of stack.
+        unsigned count = pRenameState->CountForUse(lclNum);
+        tree->gtLclVarCommon.SetSsaNum(count);
     }
 }
 
diff --git a/src/jit/ssabuilder.h b/src/jit/ssabuilder.h
index 4be76f7851..20b8cd64f7 100644
--- a/src/jit/ssabuilder.h
+++ b/src/jit/ssabuilder.h
@@ -139,11 +139,6 @@ private:
     // information in m_defs.
     void AddDefPoint(GenTree* tree, BasicBlock* blk);
 
-    // Returns true, and sets "*ppIndirAssign", if "tree" has been recorded as an indirect assignment.
-    // (If the tree is an assignment, it's a definition only if it's labeled as an indirect definition, where
-    // we took the address of the local elsewhere in the extended tree.)
-    bool IsIndirectAssign(GenTree* tree, Compiler::IndirectAssignmentAnnotation** ppIndirAssign);
-
 #ifdef DEBUG
     void Print(BasicBlock** postOrder, int count);
 #endif
diff --git a/src/jit/valuenum.cpp b/src/jit/valuenum.cpp
index 4948be0ddb..2a516fbebc 100644
--- a/src/jit/valuenum.cpp
+++ b/src/jit/valuenum.cpp
@@ -6189,9 +6189,6 @@ void Compiler::fgValueNumberTree(GenTree* tree, bool evalAsgLhsInd)
                     // If it is a PtrToLoc, lib and cons VNs will be the same.
                     if (argIsVNFunc)
                     {
-                        IndirectAssignmentAnnotation* pIndirAnnot =
-                            nullptr; // This will be used if "tree" is an "indirect assignment",
-                                     // explained below.
                         if (funcApp.m_func == VNF_PtrToLoc)
                         {
                             assert(arg->gtVNPair.BothEqual()); // If it's a PtrToLoc, lib/cons shouldn't differ.
@@ -6204,11 +6201,9 @@ void Compiler::fgValueNumberTree(GenTree* tree, bool evalAsgLhsInd)
                             {
                                 FieldSeqNode* fieldSeq = vnStore->FieldSeqVNToFieldSeq(funcApp.m_args[1]);
 
-                                // Either "arg" is the address of (part of) a local itself, or the assignment is an
-                                // "indirect assignment", where an outer comma expression assigned the address of a
-                                // local to a temp, and that temp is our lhs, and we recorded this in a table when we
-                                // made the indirect assignment...or else we have a "rogue" PtrToLoc, one that should
-                                // have made the local in question address-exposed.  Assert on that.
+                                // Either "arg" is the address of (part of) a local itself, or else we have
+                                // a "rogue" PtrToLoc, one that should have made the local in question
+                                // address-exposed.  Assert on that.
                                 GenTreeLclVarCommon* lclVarTree   = nullptr;
                                 bool                 isEntire     = false;
                                 unsigned             lclDefSsaNum = SsaConfig::RESERVED_SSA_NUM;
@@ -6246,32 +6241,6 @@ void Compiler::fgValueNumberTree(GenTree* tree, bool evalAsgLhsInd)
                                     }
                                     lvaTable[lclNum].GetPerSsaData(lclDefSsaNum)->m_vnPair = newLhsVNPair;
                                 }
-                                else if (m_indirAssignMap != nullptr && GetIndirAssignMap()->Lookup(tree, &pIndirAnnot))
-                                {
-                                    // The local #'s should agree.
-                                    assert(lclNum == pIndirAnnot->m_lclNum);
-                                    assert(pIndirAnnot->m_defSsaNum != SsaConfig::RESERVED_SSA_NUM);
-                                    lclDefSsaNum = pIndirAnnot->m_defSsaNum;
-                                    // Does this assignment write the entire width of the local?
-                                    if (genTypeSize(lhs->TypeGet()) == genTypeSize(var_types(lvaTable[lclNum].lvType)))
-                                    {
-                                        assert(pIndirAnnot->m_useSsaNum == SsaConfig::RESERVED_SSA_NUM);
-                                        assert(pIndirAnnot->m_isEntire);
-                                        newLhsVNPair = rhsVNPair;
-                                    }
-                                    else
-                                    {
-                                        assert(pIndirAnnot->m_useSsaNum != SsaConfig::RESERVED_SSA_NUM);
-                                        assert(!pIndirAnnot->m_isEntire);
-                                        assert(pIndirAnnot->m_fieldSeq == fieldSeq);
-                                        ValueNumPair oldLhsVNPair =
-                                            lvaTable[lclNum].GetPerSsaData(pIndirAnnot->m_useSsaNum)->m_vnPair;
-                                        newLhsVNPair =
-                                            vnStore->VNPairApplySelectorsAssign(oldLhsVNPair, fieldSeq, rhsVNPair,
-                                                                                lhs->TypeGet(), compCurBB);
-                                    }
-                                    lvaTable[lclNum].GetPerSsaData(lclDefSsaNum)->m_vnPair = newLhsVNPair;
-                                }
                                 else
                                 {
                                     unreached(); // "Rogue" PtrToLoc, as discussed above.