Fold primitive-typed access to promoted structs in local morph and forbid mismatched struct assignments

src/coreclr/jit/compiler.h

@@ -1009,8 +1009,6 @@ class LclVarDsc
         return structHnd;
     }
 
-    CORINFO_FIELD_HANDLE lvFieldHnd; // field handle for promoted struct fields
-
 private:
     ClassLayout* m_layout; // layout info for structs
 
@@ -3358,10 +3356,6 @@ class Compiler
             structPromotionInfo.typeHnd = NO_CLASS_HANDLE;
         }
 
-#ifdef DEBUG
-        void CheckRetypedAsScalar(CORINFO_FIELD_HANDLE fieldHnd, var_types requestedType);
-#endif // DEBUG
-
     private:
         bool CanPromoteStructVar(unsigned lclNum);
         bool ShouldPromoteStructVar(unsigned lclNum);
@@ -3376,12 +3370,6 @@ class Compiler
     private:
         Compiler*              compiler;
         lvaStructPromotionInfo structPromotionInfo;
-
-#ifdef DEBUG
-        typedef JitHashTable<CORINFO_FIELD_HANDLE, JitPtrKeyFuncs<CORINFO_FIELD_STRUCT_>, var_types>
-                                 RetypedAsScalarFieldsMap;
-        RetypedAsScalarFieldsMap retypedFieldsMap;
-#endif // DEBUG
     };
 
     StructPromotionHelper* structPromotionHelper;
@@ -5867,7 +5855,6 @@ class Compiler
 #endif
 
     PhaseStatus fgPromoteStructs();
-    void fgMorphStructField(GenTree* tree, GenTree* parent);
     void fgMorphLocalField(GenTree* tree, GenTree* parent);
 
     // Reset the refCount for implicit byrefs.

src/coreclr/jit/gentree.cpp

@@ -592,6 +592,7 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const
 {
     assert(varTypeIsStruct(TypeGet()));
 
+    CORINFO_CLASS_HANDLE structHnd = NO_CLASS_HANDLE;
     switch (OperGet())
     {
         case GT_LCL_VAR:
@@ -604,12 +605,31 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const
         case GT_BLK:
             return AsBlk()->GetLayout();
 
+        case GT_COMMA:
+            return AsOp()->gtOp2->GetLayout(compiler);
+
+#ifdef FEATURE_HW_INTRINSICS
+        case GT_HWINTRINSIC:
+            return AsHWIntrinsic()->GetLayout(compiler);
+#endif // FEATURE_HW_INTRINSICS
+
         case GT_MKREFANY:
-            return compiler->typGetObjLayout(compiler->impGetRefAnyClass());
+            structHnd = compiler->impGetRefAnyClass();
+            break;
+
+        case GT_FIELD:
+            compiler->eeGetFieldType(AsField()->gtFldHnd, &structHnd);
+            break;
+
+        case GT_CALL:
+            structHnd = AsCall()->gtRetClsHnd;
+            break;
 
         default:
             unreached();
     }
+
+    return compiler->typGetObjLayout(structHnd);
 }
 
 //-----------------------------------------------------------
@@ -22754,6 +22774,43 @@ bool GenTreeHWIntrinsic::OperIsMemoryLoadOrStore() const
     return OperIsMemoryLoad() || OperIsMemoryStore();
 }
 
+//------------------------------------------------------------------------
+// GetLayout: Get the layout for this TYP_STRUCT HWI node.
+//
+// Arguments:
+//    compiler - The Compiler instance
+//
+// Return Value:
+//    The struct layout for the node.
+//
+// Notes:
+//    Currently, this method synthesizes block layouts, since there is not
+//    enough space in the GenTreeHWIntrinsic node to store a proper layout
+//    pointer or number.
+//
+ClassLayout* GenTreeHWIntrinsic::GetLayout(Compiler* compiler) const
+{
+    assert(TypeIs(TYP_STRUCT));
+
+    switch (GetHWIntrinsicId())
+    {
+#ifdef TARGET_ARM64
+        case NI_AdvSimd_Arm64_LoadPairScalarVector64:
+        case NI_AdvSimd_Arm64_LoadPairScalarVector64NonTemporal:
+        case NI_AdvSimd_Arm64_LoadPairVector64:
+        case NI_AdvSimd_Arm64_LoadPairVector64NonTemporal:
+            return compiler->typGetBlkLayout(16);
+
+        case NI_AdvSimd_Arm64_LoadPairVector128:
+        case NI_AdvSimd_Arm64_LoadPairVector128NonTemporal:
+            return compiler->typGetBlkLayout(32);
+#endif // TARGET_ARM64
+
+        default:
+            unreached();
+    }
+}
+
 NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicId() const
 {
     NamedIntrinsic id             = gtHWIntrinsicId;

src/coreclr/jit/gentree.h

@@ -6250,6 +6250,8 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic
 
     unsigned GetResultOpNumForFMA(GenTree* use, GenTree* op1, GenTree* op2, GenTree* op3);
 
+    ClassLayout* GetLayout(Compiler* compiler) const;
+
     NamedIntrinsic GetHWIntrinsicId() const;
 
     //---------------------------------------------------------------------------------------

src/coreclr/jit/lclmorph.cpp

@@ -381,7 +381,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
     {
         GenTree* const node = *use;
 
-        if (node->OperIs(GT_FIELD))
+        if (node->OperIs(GT_IND, GT_FIELD))
         {
             MorphStructField(node, user);
         }
@@ -933,8 +933,6 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
                 indir->AsLclFld()->SetLayout(indirLayout);
                 lclNode = indir->AsLclVarCommon();
 
-                // Promoted locals aren't currently handled here so partial access can't be
-                // later be transformed into a LCL_VAR and the variable cannot be enregistered.
                 m_compiler->lvaSetVarDoNotEnregister(lclNum DEBUGARG(DoNotEnregisterReason::LocalField));
                 break;
 
@@ -1000,13 +998,6 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
 
         if (indir->TypeGet() != TYP_STRUCT)
         {
-            if (varDsc->lvPromoted)
-            {
-                // TODO-ADDR: support promoted locals here by moving the promotion morphing
-                // from pre-order to post-order.
-                return IndirTransform::None;
-            }
-
             if (indir->TypeGet() == varDsc->TypeGet())
             {
                 return IndirTransform::LclVar;
@@ -1082,23 +1073,105 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
     }
 
     //------------------------------------------------------------------------
-    // MorphStructField: Replaces a GT_FIELD based promoted/normed struct field access
-    //    (e.g. FIELD(ADDR(LCL_VAR))) with a GT_LCL_VAR that references the struct field.
+    // MorphStructField: Reduces indirect access to a promoted local (e.g.
+    //    FIELD(ADDR(LCL_VAR))) to a GT_LCL_VAR that references the struct field.
     //
     // Arguments:
-    //    node - the GT_FIELD node
-    //    user - the node that uses the field
+    //    indir - the GT_IND/GT_FIELD node
+    //    user  - the node that uses the field
     //
     // Notes:
-    //    This does not do anything if the field access does not denote
-    //    a promoted/normed struct field.
+    //    This does not do anything if the access does not denote a promoted
+    //    struct field.
     //
-    void MorphStructField(GenTree* node, GenTree* user)
+    void MorphStructField(GenTree* indir, GenTree* user)
     {
-        assert(node->OperIs(GT_FIELD));
-        // TODO-Cleanup: Move fgMorphStructField implementation here, it's not used anywhere else.
-        m_compiler->fgMorphStructField(node, user);
-        m_stmtModified |= node->OperIs(GT_LCL_VAR);
+        assert(indir->OperIs(GT_IND, GT_FIELD));
+
+        GenTree* objRef = indir->AsUnOp()->gtOp1;
+        GenTree* obj    = ((objRef != nullptr) && objRef->OperIs(GT_ADDR)) ? objRef->AsOp()->gtOp1 : nullptr;
+
+        // TODO-Bug: this code does not pay attention to "GTF_IND_VOLATILE".
+        if ((obj != nullptr) && obj->OperIs(GT_LCL_VAR) && varTypeIsStruct(obj))
+        {
+            const LclVarDsc* varDsc = m_compiler->lvaGetDesc(obj->AsLclVarCommon());
+
+            if (varDsc->lvPromoted)
+            {
+                unsigned fieldOffset = indir->OperIs(GT_FIELD) ? indir->AsField()->gtFldOffset : 0;
+                unsigned fieldLclNum = m_compiler->lvaGetFieldLocal(varDsc, fieldOffset);
+
+                if (fieldLclNum == BAD_VAR_NUM)
+                {
+                    // Access a promoted struct's field with an offset that doesn't correspond to any field.
+                    // It can happen if the struct was cast to another struct with different offsets.
+                    return;
+                }
+
+                const LclVarDsc* fieldDsc    = m_compiler->lvaGetDesc(fieldLclNum);
+                var_types        fieldType   = fieldDsc->TypeGet();
+                GenTree*         lclVarNode  = nullptr;
+                GenTreeFlags     lclVarFlags = indir->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE);
+
+                assert(fieldType != TYP_STRUCT); // promoted LCL_VAR can't have a struct type.
+                if ((indir->TypeGet() == fieldType) || ((user != nullptr) && user->OperIs(GT_ADDR)))
+                {
+                    lclVarNode = indir;
+
+                    if (user != nullptr)
+                    {
+                        if (user->OperIs(GT_ASG) && (user->AsOp()->gtOp1 == indir))
+                        {
+                            lclVarFlags |= GTF_VAR_DEF;
+                        }
+                        else if (user->OperIs(GT_ADDR))
+                        {
+                            // TODO-ADDR: delete this quirk.
+                            lclVarFlags &= ~GTF_DONT_CSE;
+                        }
+                    }
+                }
+                else // Here we will turn "FIELD/IND(ADDR(LCL_VAR<parent>))" into "OBJ/IND(ADDR(LCL_VAR<field>))".
+                {
+                    // This type mismatch is somewhat common due to how we retype fields of struct type that
+                    // recursively simplify down to a primitive. E. g. for "struct { struct { int a } A, B }",
+                    // the promoted local would look like "{ int a, B }", while the IR would contain "FIELD"
+                    // nodes for the outer struct "A".
+                    //
+                    if (indir->TypeIs(TYP_STRUCT))
+                    {
+                        // TODO-1stClassStructs: delete this once "IND<struct>" nodes are no more.
+                        if (indir->OperIs(GT_IND))
+                        {
+                            // We do not have a layout for this node.
+                            return;
+                        }
+
+                        ClassLayout* layout = indir->GetLayout(m_compiler);
+                        indir->SetOper(GT_OBJ);
+                        indir->AsBlk()->SetLayout(layout);
+                        indir->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid;
+#ifndef JIT32_GCENCODER
+                        indir->AsBlk()->gtBlkOpGcUnsafe = false;
+#endif // !JIT32_GCENCODER
+                    }
+                    else
+                    {
+                        indir->SetOper(GT_IND);
+                    }
+
+                    lclVarNode = obj;
+                }
+
+                lclVarNode->SetOper(GT_LCL_VAR);
+                lclVarNode->AsLclVarCommon()->SetLclNum(fieldLclNum);
+                lclVarNode->gtType  = fieldType;
+                lclVarNode->gtFlags = lclVarFlags;
+
+                JITDUMP("Replacing the field in promoted struct with local var V%02u\n", fieldLclNum);
+                m_stmtModified = true;
+            }
+        }
     }
 
     //------------------------------------------------------------------------

src/coreclr/jit/lclvars.cpp

@@ -1692,12 +1692,7 @@ bool Compiler::lvaFieldOffsetCmp::operator()(const lvaStructFieldInfo& field1, c
 // Arguments:
 //   compiler - pointer to a compiler to get access to an allocator, compHandle etc.
 //
-Compiler::StructPromotionHelper::StructPromotionHelper(Compiler* compiler)
-    : compiler(compiler)
-    , structPromotionInfo()
-#ifdef DEBUG
-    , retypedFieldsMap(compiler->getAllocator(CMK_DebugOnly))
-#endif // DEBUG
+Compiler::StructPromotionHelper::StructPromotionHelper(Compiler* compiler) : compiler(compiler), structPromotionInfo()
 {
 }
 
@@ -1730,27 +1725,6 @@ bool Compiler::StructPromotionHelper::TryPromoteStructVar(unsigned lclNum)
     return false;
 }
 
-#ifdef DEBUG
-//--------------------------------------------------------------------------------------------
-// CheckRetypedAsScalar - check that the fldType for this fieldHnd was retyped as requested type.
-//
-// Arguments:
-//   fieldHnd      - the field handle;
-//   requestedType - as which type the field was accessed;
-//
-// Notes:
-//   For example it can happen when such struct A { struct B { long c } } is compiled and we access A.B.c,
-//   it could look like "GT_FIELD struct B.c -> ADDR -> GT_FIELD struct A.B -> ADDR -> LCL_VAR A" , but
-//   "GT_FIELD struct A.B -> ADDR -> LCL_VAR A" can be promoted to "LCL_VAR long A.B" and then
-//   there is type mistmatch between "GT_FIELD struct B.c" and  "LCL_VAR long A.B".
-//
-void Compiler::StructPromotionHelper::CheckRetypedAsScalar(CORINFO_FIELD_HANDLE fieldHnd, var_types requestedType)
-{
-    assert(retypedFieldsMap.Lookup(fieldHnd));
-    assert(retypedFieldsMap[fieldHnd] == requestedType);
-}
-#endif // DEBUG
-
 //--------------------------------------------------------------------------------------------
 // CanPromoteStructType - checks if the struct type can be promoted.
 //
@@ -2309,9 +2283,6 @@ Compiler::lvaStructFieldInfo Compiler::StructPromotionHelper::GetFieldInfo(CORIN
             {
                 fieldInfo.fldType = compiler->getSIMDTypeForSize(simdSize);
                 fieldInfo.fldSize = simdSize;
-#ifdef DEBUG
-                retypedFieldsMap.Set(fieldInfo.fldHnd, fieldInfo.fldType, RetypedAsScalarFieldsMap::Overwrite);
-#endif // DEBUG
             }
         }
     }
@@ -2412,9 +2383,7 @@ bool Compiler::StructPromotionHelper::TryPromoteStructField(lvaStructFieldInfo&
     // (tracked by #10019).
     fieldInfo.fldType = fieldVarType;
     fieldInfo.fldSize = fieldSize;
-#ifdef DEBUG
-    retypedFieldsMap.Set(fieldInfo.fldHnd, fieldInfo.fldType, RetypedAsScalarFieldsMap::Overwrite);
-#endif // DEBUG
+
     return true;
 }
 
@@ -2496,7 +2465,6 @@ void Compiler::StructPromotionHelper::PromoteStructVar(unsigned lclNum)
         fieldVarDsc->lvType          = pFieldInfo->fldType;
         fieldVarDsc->lvExactSize     = pFieldInfo->fldSize;
         fieldVarDsc->lvIsStructField = true;
-        fieldVarDsc->lvFieldHnd      = pFieldInfo->fldHnd;
         fieldVarDsc->lvFldOffset     = pFieldInfo->fldOffset;
         fieldVarDsc->lvFldOrdinal    = pFieldInfo->fldOrdinal;
         fieldVarDsc->lvParentLcl     = lclNum;

src/coreclr/jit/lower.cpp

@@ -3340,8 +3340,7 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
                 bool constStructInit                  = retVal->IsConstInitVal();
                 bool implicitCastFromSameOrBiggerSize = (genTypeSize(retActualType) <= genTypeSize(retValActualType));
 
-                // This could happen if we have retyped op1 as a primitive type during struct promotion,
-                // check `retypedFieldsMap` for details.
+                // This could happen if we have retyped op1 as a primitive type during struct promotion.
                 bool actualTypesMatch = (retActualType == retValActualType);
 
                 assert(actualTypesMatch || constStructInit || implicitCastFromSameOrBiggerSize);