Refactor Sequential/Explicit layout to happen in the same phase where we do Auto layout

src/coreclr/vm/class.cpp

@@ -246,11 +246,6 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField
         bmtEnumFields.dwNumInstanceFields = 1;
     }
 
-    // We shouldn't have to fill this in b/c we're not allowed to EnC value classes, or
-    // anything else with layout info associated with it.
-    // Provide 2, 1 placeholder and 1 for the actual field - see BuildMethodTableThrowing().
-    LayoutRawFieldInfo layoutRawFieldInfos[2];
-
     // If not NULL, it means there are some by-value fields, and this contains an entry for each instance or static field,
     // which is NULL if not a by value field, and points to the EEClass of the field if a by value field.  Instance fields
     // come first, statics come second.
@@ -288,7 +283,6 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField
         GCX_PREEMP();
         unsigned totalDeclaredFieldSize = 0;
         builder.InitializeFieldDescs(pFD,
-                                 layoutRawFieldInfos,
                                  &bmtInternal,
                                  &genericsInfo,
                                  &bmtMetaData,

src/coreclr/vm/class.h

@@ -72,7 +72,6 @@ class   EnCFieldDesc;
 class   FieldDesc;
 class   NativeFieldDescriptor;
 class   EEClassNativeLayoutInfo;
-struct  LayoutRawFieldInfo;
 class   MetaSig;
 class   MethodDesc;
 class   MethodDescChunk;
@@ -126,7 +125,7 @@ class ExplicitFieldTrust
 };
 
 //----------------------------------------------------------------------------------------------
-// This class is a helper for HandleExplicitLayout. To make it harder to introduce security holes
+// This class is a helper for ValidateExplicitLayout. To make it harder to introduce security holes
 // into this function, we will manage all updates to the class's trust level through the ExplicitClassTrust
 // class. This abstraction enforces the rule that the overall class is only as trustworthy as
 // the least trustworthy field.
@@ -175,7 +174,7 @@ class ExplicitClassTrust : private ExplicitFieldTrust
 };
 
 //----------------------------------------------------------------------------------------------
-// This class is a helper for HandleExplicitLayout. To make it harder to introduce security holes
+// This class is a helper for ValidateExplicitLayout. To make it harder to introduce security holes
 // into this function, this class will collect trust information about individual fields to be later
 // aggregated into the overall class level.
 //
@@ -334,39 +333,22 @@ class SparseVTableMap
 //=======================================================================
 class EEClassLayoutInfo
 {
-    static VOID CollectLayoutFieldMetadataThrowing(
-       mdTypeDef cl,                // cl of the NStruct being loaded
-       BYTE packingSize,            // packing size (from @dll.struct)
-       BYTE nlType,                 // nltype (from @dll.struct)
-       BOOL fExplicitOffsets,       // explicit offsets?
-       MethodTable *pParentMT,       // the loaded superclass
-       ULONG cTotalFields,              // total number of fields (instance and static)
-       HENUMInternal *phEnumField,  // enumerator for fields
-       Module* pModule,             // Module that defines the scope, loader and heap (for allocate FieldMarshalers)
-       const SigTypeContext *pTypeContext,          // Type parameters for NStruct being loaded
-       EEClassLayoutInfo *pEEClassLayoutInfoOut,  // caller-allocated structure to fill in.
-       LayoutRawFieldInfo *pInfoArrayOut, // caller-allocated array to fill in.  Needs room for cTotalFields+1 elements
-       LoaderAllocator * pAllocator,
-       AllocMemTracker    *pamTracker
-    );
-
-    friend class ClassLoader;
-    friend class EEClass;
-    friend class MethodTableBuilder;
-        UINT32      m_cbManagedSize;
-
     public:
-        BYTE        m_ManagedLargestAlignmentRequirementOfAllMembers;
-
+        enum class LayoutType : BYTE
+        {
+            Auto = 0, // Make sure Auto is the default value as the default-constructed value represents the "auto layout" case
+            Sequential,
+            Explicit
+        };
     private:
         enum {
             // TRUE if the GC layout of the class is bit-for-bit identical
             // to its unmanaged counterpart with the runtime marshalling system
             // (i.e. no internal reference fields, no ansi-unicode char conversions required, etc.)
             // Used to optimize marshaling.
             e_BLITTABLE                       = 0x01,
-            // Is this type also sequential in managed memory?
-            e_MANAGED_SEQUENTIAL              = 0x02,
+            // unused                         = 0x02,
+
             // When a sequential/explicit type has no fields, it is conceptually
             // zero-sized, but actually is 1 byte in length. This holds onto this
             // fact and allows us to revert the 1 byte of padding when another
@@ -380,28 +362,27 @@ class EEClassLayoutInfo
             e_IS_OR_HAS_INT128_FIELD          = 0x20,
         };
 
-        BYTE        m_bFlags;
+        LayoutType m_LayoutType;
+
+        BYTE       m_ManagedLargestAlignmentRequirementOfAllMembers;
+
+        BYTE       m_bFlags;
 
         // Packing size in bytes (1, 2, 4, 8 etc.)
-        BYTE        m_cbPackingSize;
+        BYTE       m_cbPackingSize;
 
     public:
-        UINT32 GetManagedSize() const
-        {
-            LIMITED_METHOD_CONTRACT;
-            return m_cbManagedSize;
-        }
 
         BOOL IsBlittable() const
         {
             LIMITED_METHOD_CONTRACT;
             return (m_bFlags & e_BLITTABLE) == e_BLITTABLE;
         }
 
-        BOOL IsManagedSequential() const
+        LayoutType GetLayoutType() const
         {
             LIMITED_METHOD_CONTRACT;
-            return (m_bFlags & e_MANAGED_SEQUENTIAL) == e_MANAGED_SEQUENTIAL;
+            return m_LayoutType;
         }
 
         // If true, this says that the type was originally zero-sized
@@ -433,54 +414,112 @@ class EEClassLayoutInfo
             return (m_bFlags & e_IS_OR_HAS_INT128_FIELD) == e_IS_OR_HAS_INT128_FIELD;
         }
 
+        BYTE GetAlignmentRequirement() const
+        {
+            LIMITED_METHOD_CONTRACT;
+            return m_ManagedLargestAlignmentRequirementOfAllMembers;
+        }
+
         BYTE GetPackingSize() const
         {
             LIMITED_METHOD_CONTRACT;
             return m_cbPackingSize;
         }
 
-    private:
         void SetIsBlittable(BOOL isBlittable)
         {
             LIMITED_METHOD_CONTRACT;
             m_bFlags = isBlittable ? (m_bFlags | e_BLITTABLE)
                                    : (m_bFlags & ~e_BLITTABLE);
         }
 
-        void SetIsManagedSequential(BOOL isManagedSequential)
+        void SetHasAutoLayoutField(BOOL hasAutoLayoutField)
         {
             LIMITED_METHOD_CONTRACT;
-            m_bFlags = isManagedSequential ? (m_bFlags | e_MANAGED_SEQUENTIAL)
-                                           : (m_bFlags & ~e_MANAGED_SEQUENTIAL);
+            m_bFlags = hasAutoLayoutField ? (m_bFlags | e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT)
+                                       : (m_bFlags & ~e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT);
         }
 
-        void SetIsZeroSized(BOOL isZeroSized)
+        void SetIsInt128OrHasInt128Fields(BOOL hasInt128Field)
         {
             LIMITED_METHOD_CONTRACT;
-            m_bFlags = isZeroSized ? (m_bFlags | e_ZERO_SIZED)
-                                   : (m_bFlags & ~e_ZERO_SIZED);
+            m_bFlags = hasInt128Field ? (m_bFlags | e_IS_OR_HAS_INT128_FIELD)
+                                       : (m_bFlags & ~e_IS_OR_HAS_INT128_FIELD);
         }
 
         void SetHasExplicitSize(BOOL hasExplicitSize)
         {
             LIMITED_METHOD_CONTRACT;
             m_bFlags = hasExplicitSize ? (m_bFlags | e_HAS_EXPLICIT_SIZE)
-                                       : (m_bFlags & ~e_HAS_EXPLICIT_SIZE);
+                                    : (m_bFlags & ~e_HAS_EXPLICIT_SIZE);
         }
 
-        void SetHasAutoLayoutField(BOOL hasAutoLayoutField)
+        void SetAlignmentRequirement(BYTE alignment)
         {
             LIMITED_METHOD_CONTRACT;
-            m_bFlags = hasAutoLayoutField ? (m_bFlags | e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT)
-                                       : (m_bFlags & ~e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT);
+            m_ManagedLargestAlignmentRequirementOfAllMembers = alignment;
         }
 
-        void SetIsInt128OrHasInt128Fields(BOOL hasInt128Field)
+        ULONG InitializeSequentialFieldLayout(
+            FieldDesc* pFields,
+            MethodTable** pByValueClassCache,
+            ULONG cFields,
+            BYTE packingSize,
+            ULONG classSizeInMetadata,
+            MethodTable* pParentMT
+        );
+
+        ULONG InitializeExplicitFieldLayout(
+            FieldDesc* pFields,
+            MethodTable** pByValueClassCache,
+            ULONG cFields,
+            BYTE packingSize,
+            ULONG classSizeInMetadata,
+            MethodTable* pParentMT,
+            Module* pModule,
+            mdTypeDef cl
+        );
+
+    private:
+        void SetIsZeroSized(BOOL isZeroSized)
         {
             LIMITED_METHOD_CONTRACT;
-            m_bFlags = hasInt128Field ? (m_bFlags | e_IS_OR_HAS_INT128_FIELD)
-                                       : (m_bFlags & ~e_IS_OR_HAS_INT128_FIELD);
+            m_bFlags = isZeroSized ? (m_bFlags | e_ZERO_SIZED)
+                                : (m_bFlags & ~e_ZERO_SIZED);
+        }
+
+        void SetPackingSize(BYTE cbPackingSize)
+        {
+            LIMITED_METHOD_CONTRACT;
+            m_cbPackingSize = cbPackingSize;
         }
+
+        UINT32 SetInstanceBytesSize(UINT32 size)
+        {
+            LIMITED_METHOD_CONTRACT;
+            // Bump the managed size of the structure up to 1.
+            SetIsZeroSized(size == 0 ? TRUE : FALSE);
+            return size == 0 ? 1 : size;
+        }
+
+        void SetLayoutType(LayoutType layoutType)
+        {
+            LIMITED_METHOD_CONTRACT;
+            m_LayoutType = layoutType;
+        }
+    public:
+        enum class NestedFieldFlags
+        {
+            support_use_as_flags = -1,
+            None = 0x0,
+            NonBlittable = 0x1,
+            GCPointer = 0x2,
+            Align8 = 0x4,
+            AutoLayout = 0x8,
+            Int128 = 0x10,
+        };
+
+        static NestedFieldFlags GetNestedFieldFlags(Module* pModule, FieldDesc *pFD, ULONG cFields, CorNativeLinkType nlType, MethodTable** pByValueClassCache);
 };
 
 //
@@ -1964,7 +2003,7 @@ inline BOOL EEClass::IsBlittable()
 inline BOOL EEClass::IsManagedSequential()
 {
     LIMITED_METHOD_CONTRACT;
-    return HasLayout() && GetLayoutInfo()->IsManagedSequential();
+    return HasLayout() && GetLayoutInfo()->GetLayoutType() == EEClassLayoutInfo::LayoutType::Sequential;
 }
 
 inline BOOL EEClass::HasExplicitSize()

src/coreclr/vm/classcompat.h

@@ -42,7 +42,6 @@ class   EEClass;
 class   LayoutEEClass;
 class   EnCFieldDesc;
 class   FieldDesc;
-struct  LayoutRawFieldInfo;
 class   MetaSig;
 class   MethodDesc;
 class   MethodDescChunk;