Include dependently promoted fields in SSA (#77238)

* Remove quirks

* Early prop fix for call asgs

* Include all tracked locals in SSA

* Remove a (now incorrect) comment

* Fix a typo

* Encoding fixes

 1) The definition of SIMPLE_NUM_COUNT was wrong.
 2) SsaNumInfo::Composite, in the compact case, did not clear the old value.
 3) SsaNumInfo::Composite, in the outlined case, did not copy the already
    (compactly) encoded simple names.

* Fix store numbering

The load path needs to use the offset relative to
the store's target location.

* Clarify 'fieldValueOffset' calculations

Author: SingleAccretion

src/coreclr/jit/compiler.h

@@ -2815,7 +2815,7 @@ class Compiler
     bool gtIsStaticFieldPtrToBoxedStruct(var_types fieldNodeType, CORINFO_FIELD_HANDLE fldHnd);
 
     bool gtStoreDefinesField(
-        LclVarDsc* fieldVarDsc, ssize_t offset, unsigned size, ssize_t* pFieldStoreOffset, unsigned* pFileStoreSize);
+        LclVarDsc* fieldVarDsc, ssize_t offset, unsigned size, ssize_t* pFieldStoreOffset, unsigned* pFieldStoreSize);
 
     // Return true if call is a recursive call; return false otherwise.
     // Note when inlining, this looks for calls back to the root method.

src/coreclr/jit/copyprop.cpp

@@ -326,20 +326,12 @@ void Compiler::optCopyPropPushDef(GenTree* defNode, GenTreeLclVarCommon* lclNode
         LclVarDsc* varDsc = lvaGetDesc(lclNum);
         assert(varDsc->lvPromoted);
 
-        if (varDsc->CanBeReplacedWithItsField(this))
+        for (unsigned index = 0; index < varDsc->lvFieldCnt; index++)
         {
-            // TODO-CQ: remove this zero-diff quirk.
-            pushDef(varDsc->lvFieldLclStart, SsaConfig::RESERVED_SSA_NUM);
-        }
-        else
-        {
-            for (unsigned index = 0; index < varDsc->lvFieldCnt; index++)
+            unsigned ssaNum = lclNode->GetSsaNum(this, index);
+            if (ssaNum != SsaConfig::RESERVED_SSA_NUM)
             {
-                unsigned ssaNum = lclNode->GetSsaNum(this, index);
-                if (ssaNum != SsaConfig::RESERVED_SSA_NUM)
-                {
-                    pushDef(varDsc->lvFieldLclStart + index, ssaNum);
-                }
+                pushDef(varDsc->lvFieldLclStart + index, ssaNum);
             }
         }
     }

src/coreclr/jit/earlyprop.cpp

@@ -364,13 +364,10 @@ GenTree* Compiler::optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropK
     LclSsaVarDsc* ssaVarDsc = lvaTable[lclNum].GetPerSsaData(ssaNum);
     GenTreeOp*    ssaDefAsg = ssaVarDsc->GetAssignment();
 
-    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
+    // Incoming parameters or live-in variables don't have actual definition tree node for
+    // their FIRST_SSA_NUM. Definitions induced by calls do not record the store node. See
+    // SsaBuilder::RenameDef.
+    if (ssaDefAsg != nullptr)
     {
         assert(ssaDefAsg->OperIs(GT_ASG));
 
@@ -569,8 +566,19 @@ GenTree* Compiler::optFindNullCheckToFold(GenTree* tree, LocalNumberToNullCheckT
             return nullptr;
         }
 
-        GenTree* defRHS = defLoc->GetAssignment()->gtGetOp2();
+        GenTree* defNode = defLoc->GetAssignment();
+        if (defNode == nullptr)
+        {
+            return nullptr;
+        }
+
+        GenTree* defLHS = defNode->gtGetOp1();
+        if (!defLHS->OperIs(GT_LCL_VAR) || (defLHS->AsLclVar()->GetLclNum() != lclNum))
+        {
+            return nullptr;
+        }
 
+        GenTree* defRHS = defNode->gtGetOp2();
         if (defRHS->OperGet() != GT_COMMA)
         {
             return nullptr;

src/coreclr/jit/gentree.cpp

@@ -17626,15 +17626,15 @@ bool Compiler::gtIsStaticFieldPtrToBoxedStruct(var_types fieldNodeType, CORINFO_
 //    size              - Size of the store in bytes
 //    pFieldStoreOffset - [out] parameter for the store's offset relative
 //                        to the field local itself
-//    pFileStoreSize    - [out] parameter for the amount of the field's
+//    pFieldStoreSize   - [out] parameter for the amount of the field's
 //                        local's bytes affected by the store
 //
 // Return Value:
 //    If the given store affects the given field local, "true, "false"
 //    otherwise.
 //
 bool Compiler::gtStoreDefinesField(
-    LclVarDsc* fieldVarDsc, ssize_t offset, unsigned size, ssize_t* pFieldStoreOffset, unsigned* pFileStoreSize)
+    LclVarDsc* fieldVarDsc, ssize_t offset, unsigned size, ssize_t* pFieldStoreOffset, unsigned* pFieldStoreSize)
 {
     ssize_t  fieldOffset = fieldVarDsc->lvFldOffset;
     unsigned fieldSize   = genTypeSize(fieldVarDsc); // No TYP_STRUCT field locals.
@@ -17644,7 +17644,7 @@ bool Compiler::gtStoreDefinesField(
     if ((fieldOffset < storeEndOffset) && (offset < fieldEndOffset))
     {
         *pFieldStoreOffset = (offset < fieldOffset) ? 0 : (offset - fieldOffset);
-        *pFileStoreSize    = static_cast<unsigned>(min(storeEndOffset, fieldEndOffset) - max(offset, fieldOffset));
+        *pFieldStoreSize   = static_cast<unsigned>(min(storeEndOffset, fieldEndOffset) - max(offset, fieldOffset));
 
         return true;
     }
@@ -23790,10 +23790,10 @@ unsigned* SsaNumInfo::GetOutlinedNumSlot(Compiler* compiler, unsigned index) con
             return SsaNumInfo(COMPOSITE_ENCODING_BIT | ssaNumEncoded);
         }
 
-        return SsaNumInfo(ssaNumEncoded | baseNum.m_value);
+        return SsaNumInfo(ssaNumEncoded | (baseNum.m_value & ~(SIMPLE_NUM_MASK << (index * BITS_PER_SIMPLE_NUM))));
     }
 
-    if (!baseNum.IsInvalid())
+    if (!baseNum.IsInvalid() && !baseNum.HasCompactFormat())
     {
         *baseNum.GetOutlinedNumSlot(compiler, index) = ssaNum;
         return baseNum;
@@ -23807,11 +23807,23 @@ unsigned* SsaNumInfo::GetOutlinedNumSlot(Compiler* compiler, unsigned index) con
     }
 
     // Allocate a new chunk for the field numbers. Once allocated, it cannot be expanded.
-    int                            count              = compiler->lvaGetDesc(parentLclNum)->lvFieldCnt;
-    JitExpandArrayStack<unsigned>* table              = compiler->m_outlinedCompositeSsaNums;
-    int                            outIdx             = table->Size();
-    unsigned*                      pLastSlot          = &table->GetRef(outIdx + count - 1); // This will grow the table.
-    pLastSlot[-(count - 1) + static_cast<int>(index)] = ssaNum;
+    int                            count      = compiler->lvaGetDesc(parentLclNum)->lvFieldCnt;
+    JitExpandArrayStack<unsigned>* table      = compiler->m_outlinedCompositeSsaNums;
+    int                            outIdx     = table->Size();
+    unsigned*                      pLastSlot  = &table->GetRef(outIdx + count - 1); // This will grow the table.
+    unsigned*                      pFirstSlot = pLastSlot - count + 1;
+
+    // Copy over all of the already encoded numbers.
+    if (!baseNum.IsInvalid())
+    {
+        for (int i = 0; i < SIMPLE_NUM_COUNT; i++)
+        {
+            pFirstSlot[i] = baseNum.GetNum(compiler, i);
+        }
+    }
+
+    // Copy the one being set last to overwrite any previous values.
+    pFirstSlot[index] = ssaNum;
 
     // Split the index if it does not fit into a small encoding.
     if ((outIdx & ~OUTLINED_INDEX_LOW_MASK) != 0)

src/coreclr/jit/gentree.h

@@ -3538,7 +3538,7 @@ class SsaNumInfo final
     static const int BITS_PER_SIMPLE_NUM     = 8;
     static const int MAX_SIMPLE_NUM          = (1 << (BITS_PER_SIMPLE_NUM - 1)) - 1;
     static const int SIMPLE_NUM_MASK         = MAX_SIMPLE_NUM;
-    static const int SIMPLE_NUM_COUNT        = sizeof(int) / BITS_PER_SIMPLE_NUM;
+    static const int SIMPLE_NUM_COUNT        = (sizeof(int) * BITS_PER_BYTE) / BITS_PER_SIMPLE_NUM;
     static const int COMPOSITE_ENCODING_BIT  = 1 << 31;
     static const int OUTLINED_ENCODING_BIT   = 1 << 15;
     static const int OUTLINED_INDEX_LOW_MASK = OUTLINED_ENCODING_BIT - 1;

src/coreclr/jit/morphblock.cpp

@@ -814,8 +814,6 @@ void MorphCopyBlockHelper::TrySpecialCases()
     {
         assert(m_dst->OperIs(GT_LCL_VAR));
 
-        // This will exclude field locals (if any) from SSA: we do not have a way to
-        // associate multiple SSA definitions (SSA numbers) with one store.
         m_dstVarDsc->lvIsMultiRegRet = true;
 
         JITDUMP("Not morphing a multireg node return\n");

src/coreclr/jit/ssabuilder.cpp

@@ -1584,7 +1584,7 @@ void SsaBuilder::Build()
     // Mark all variables that will be tracked by SSA
     for (unsigned lclNum = 0; lclNum < m_pCompiler->lvaCount; lclNum++)
     {
-        m_pCompiler->lvaTable[lclNum].lvInSsa = IncludeInSsa(lclNum);
+        m_pCompiler->lvaTable[lclNum].lvInSsa = m_pCompiler->lvaGetDesc(lclNum)->lvTracked;
     }
 
     // Insert phi functions.
@@ -1641,45 +1641,6 @@ void SsaBuilder::SetupBBRoot()
     }
 }
 
-//------------------------------------------------------------------------
-// IncludeInSsa: Check if the specified variable can be included in SSA.
-//
-// Arguments:
-//    lclNum - the variable number
-//
-// Return Value:
-//    true if the variable is included in SSA
-//
-bool SsaBuilder::IncludeInSsa(unsigned lclNum)
-{
-    LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum);
-
-    if (!varDsc->lvTracked)
-    {
-        return false; // SSA is only done for tracked variables
-    }
-    // lvPromoted structs are never tracked...
-    assert(!varDsc->lvPromoted);
-
-    if (varDsc->lvIsStructField &&
-        (m_pCompiler->lvaGetParentPromotionType(lclNum) != Compiler::PROMOTION_TYPE_INDEPENDENT))
-    {
-        // SSA must exclude struct fields that are not independent
-        // - because we don't model the struct assignment properly when multiple fields can be assigned by one struct
-        //   assignment.
-        // - SSA doesn't allow a single node to contain multiple SSA definitions.
-        // - and PROMOTION_TYPE_DEPENDEDNT fields  are never candidates for a register.
-        //
-        return false;
-    }
-    else if (varDsc->lvIsStructField && m_pCompiler->lvaGetDesc(varDsc->lvParentLcl)->lvIsMultiRegRet)
-    {
-        return false;
-    }
-    // otherwise this variable is included in SSA
-    return true;
-}
-
 #ifdef DEBUG
 
 //------------------------------------------------------------------------

src/coreclr/jit/ssabuilder.h

@@ -20,8 +20,6 @@ class SsaBuilder
         m_pCompiler->EndPhase(phase);
     }
 
-    bool IncludeInSsa(unsigned lclNum);
-
 public:
     // Constructor
     SsaBuilder(Compiler* pCompiler);

src/coreclr/jit/valuenum.cpp

@@ -4966,8 +4966,12 @@ void Compiler::fgValueNumberLocalStore(GenTree*             storeNode,
                     // Avoid redundant bitcasts for the common case of a full definition.
                     fieldStoreType = fieldVarDsc->TypeGet();
                 }
+
+                // Calculate offset of this field's value, relative to the entire one.
+                ssize_t      fieldOffset      = fieldVarDsc->lvFldOffset;
+                ssize_t      fieldValueOffset = (fieldOffset < offset) ? 0 : (fieldOffset - offset);
                 ValueNumPair fieldStoreValue =
-                    vnStore->VNPairForLoad(value, storeSize, fieldStoreType, offset, fieldStoreSize);
+                    vnStore->VNPairForLoad(value, storeSize, fieldStoreType, fieldValueOffset, fieldStoreSize);
 
                 processDef(fieldLclNum, lclDefNode->GetSsaNum(this, index), fieldStoreOffset, fieldStoreSize,
                            fieldStoreValue);
@@ -8503,11 +8507,6 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
 
             rhsVNPair.SetBoth(initObjVN);
         }
-        else if (lhs->OperIs(GT_LCL_VAR) && lhsVarDsc->CanBeReplacedWithItsField(this))
-        {
-            // TODO-CQ: remove this zero-diff quirk.
-            rhsVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lvaGetDesc(lhsVarDsc->lvFieldLclStart)->TypeGet()));
-        }
         else
         {
             assert(tree->OperIsCopyBlkOp());