[release/10.0-preview6] Record struct packing even when falling back to auto layout

src/coreclr/vm/class.h

@@ -460,6 +460,12 @@ class EEClassLayoutInfo
             m_ManagedLargestAlignmentRequirementOfAllMembers = alignment;
         }
 
+        void SetPackingSize(BYTE cbPackingSize)
+        {
+            LIMITED_METHOD_CONTRACT;
+            m_cbPackingSize = cbPackingSize;
+        }
+
         ULONG InitializeSequentialFieldLayout(
             FieldDesc* pFields,
             MethodTable** pByValueClassCache,
@@ -488,12 +494,6 @@ class EEClassLayoutInfo
                                 : (m_bFlags & ~e_ZERO_SIZED);
         }
 
-        void SetPackingSize(BYTE cbPackingSize)
-        {
-            LIMITED_METHOD_CONTRACT;
-            m_cbPackingSize = cbPackingSize;
-        }
-
         UINT32 SetInstanceBytesSize(UINT32 size)
         {
             LIMITED_METHOD_CONTRACT;

src/coreclr/vm/classlayoutinfo.cpp

@@ -891,25 +891,37 @@ EEClassNativeLayoutInfo* EEClassNativeLayoutInfo::CollectNativeLayoutFieldMetada
 
     pNativeLayoutInfo->m_numFields = numTotalInstanceFields;
 
-    BYTE parentAlignmentRequirement = 0;
+    // If there's no parent, pretend that the parent alignment requirement is 1.
+    BYTE parentAlignmentRequirement = 1;
     if (pParentLayoutInfo != nullptr)
     {
         parentAlignmentRequirement = pParentLayoutInfo->GetLargestAlignmentRequirement();
     }
 
+    BYTE packingSize = pMT->GetLayoutInfo()->GetPackingSize();
+    if (packingSize == 0)
+    {
+        packingSize = DEFAULT_PACKING_SIZE;
+    }
+
     BYTE fieldAlignmentRequirement = 0;
     // Now compute the native size of each field
     for (LayoutRawFieldInfo* pfwalk = pInfoArray; pfwalk->m_token != mdFieldDefNil; pfwalk++)
     {
         pfwalk->m_placement.m_size = pfwalk->m_nfd.NativeSize();
-        pfwalk->m_placement.m_alignment = pfwalk->m_nfd.AlignmentRequirement();
+        // Allow the packing size to override a looser alignment requirement.
+        pfwalk->m_placement.m_alignment = min(packingSize, (BYTE)pfwalk->m_nfd.AlignmentRequirement());
         if (pfwalk->m_placement.m_alignment > fieldAlignmentRequirement)
         {
             fieldAlignmentRequirement = (BYTE)pfwalk->m_placement.m_alignment;
         }
     }
 
-    pNativeLayoutInfo->m_alignmentRequirement = max(max<BYTE>(1, parentAlignmentRequirement), fieldAlignmentRequirement);
+    // Allow the packing size to require less alignment than the parent's alignment requirement.
+    BYTE initialAlignmentRequirement = min(packingSize, parentAlignmentRequirement);
+
+    // The alignment of the native layout is the stricter of the initial alignment requirement or the alignment requirements of the fields.
+    pNativeLayoutInfo->m_alignmentRequirement = max(initialAlignmentRequirement, fieldAlignmentRequirement);
 
     BOOL fExplicitOffsets = pMT->GetClass()->HasExplicitFieldOffsetLayout();
 
@@ -920,11 +932,6 @@ EEClassNativeLayoutInfo* EEClassNativeLayoutInfo::CollectNativeLayoutFieldMetada
     }
     else
     {
-        BYTE packingSize = pMT->GetLayoutInfo()->GetPackingSize();
-        if (packingSize == 0)
-        {
-            packingSize = DEFAULT_PACKING_SIZE;
-        }
         lastFieldEnd = CalculateOffsetsForSequentialLayout(pInfoArray, cInstanceFields, cbAdjustedParentLayoutNativeSize, packingSize);
     }
 

src/coreclr/vm/methodtablebuilder.cpp

@@ -8219,16 +8219,12 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable** pByValueClassCache)
 
     if (bmtLayout->layoutType == EEClassLayoutInfo::LayoutType::Sequential)
     {
-        if (hasNonTrivialParent && !pParentMT->IsManagedSequential())
+        // If the parent type is not Object, ValueType, or Sequential, or if this type has GC fields,
+        // we will use Auto layout instead of Sequential layout and set the packing size.
+        if ((hasNonTrivialParent && !pParentMT->IsManagedSequential()) || hasGCFields)
         {
-            // If the parent type is not Object, ValueType or Sequential, then we need to use Auto layout.
-            bmtLayout->layoutType = EEClassLayoutInfo::LayoutType::Auto;
-        }
-
-        if (hasGCFields)
-        {
-            // If this type has GC fields, we will use Auto layout instead of Sequential layout.
             bmtLayout->layoutType = EEClassLayoutInfo::LayoutType::Auto;
+            pLayoutInfo->SetPackingSize(bmtLayout->packingSize);
         }
     }
 

src/tests/Interop/StructPacking/StructPacking.cs

@@ -30,8 +30,8 @@ struct DefaultLayoutDefaultPacking<T> : ITestStructure
     public T _value;
 
     public int Size => Unsafe.SizeOf<DefaultLayoutDefaultPacking<T>>();
-    public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
-    public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
+    public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
+    public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
 }
 
 [StructLayout(LayoutKind.Sequential)]
@@ -41,8 +41,8 @@ struct SequentialLayoutDefaultPacking<T> : ITestStructure
     public T _value;
 
     public int Size => Unsafe.SizeOf<SequentialLayoutDefaultPacking<T>>();
-    public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
-    public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
+    public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
+    public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
 }
 
 [StructLayout(LayoutKind.Sequential, Pack = 1)]
@@ -52,8 +52,8 @@ struct SequentialLayoutMinPacking<T> : ITestStructure
     public T _value;
 
     public int Size => Unsafe.SizeOf<SequentialLayoutMinPacking<T>>();
-    public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
-    public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
+    public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
+    public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
 }
 
 [StructLayout(LayoutKind.Sequential, Pack = 128)]
@@ -63,8 +63,8 @@ struct SequentialLayoutMaxPacking<T> : ITestStructure
     public T _value;
 
     public int Size => Unsafe.SizeOf<SequentialLayoutMaxPacking<T>>();
-    public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
-    public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
+    public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
+    public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
 }
 
 [StructLayout(LayoutKind.Auto)]
@@ -74,8 +74,8 @@ struct AutoLayoutDefaultPacking<T> : ITestStructure
     public T _value;
 
     public int Size => Unsafe.SizeOf<AutoLayoutDefaultPacking<T>>();
-    public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
-    public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
+    public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
+    public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
 }
 
 [StructLayout(LayoutKind.Auto, Pack = 1)]
@@ -85,8 +85,8 @@ struct AutoLayoutMinPacking<T> : ITestStructure
     public T _value;
 
     public int Size => Unsafe.SizeOf<AutoLayoutMinPacking<T>>();
-    public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
-    public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
+    public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
+    public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
 }
 
 [StructLayout(LayoutKind.Auto, Pack = 128)]
@@ -96,11 +96,22 @@ struct AutoLayoutMaxPacking<T> : ITestStructure
     public T _value;
 
     public int Size => Unsafe.SizeOf<AutoLayoutMaxPacking<T>>();
-    public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
-    public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
+    public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
+    public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
 }
 
-public unsafe partial class Program
+[StructLayout(LayoutKind.Sequential, Pack = 1)]
+struct ManagedAutoUnmanagedSequentialLayoutMinPacking : ITestStructure
+{
+    public Action _value;
+    public byte _byte;
+
+    public int Size => Unsafe.SizeOf<ManagedAutoUnmanagedSequentialLayoutMinPacking>();
+    public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
+    public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
+}
+
+public unsafe partial class StructPacking
 {
     const int Pass = 100;
     const int Fail = 0;
@@ -136,6 +147,9 @@ public static int TestEntryPoint()
         succeeded &= TestMyVector128();
         succeeded &= TestMyVector256();
 
+        // Test data types that invalidate managed sequential layout
+        succeeded &= TestAction();
+
         return succeeded ? Pass : Fail;
     }
 
@@ -1629,6 +1643,45 @@ static bool TestMyVector256()
 
         return succeeded;
     }
+    static bool TestAction()
+    {
+        bool succeeded = true;
+
+        if (Environment.Is64BitProcess)
+        {
+            succeeded &= Test<ManagedAutoUnmanagedSequentialLayoutMinPacking>(
+                expectedSize: 16,
+                expectedOffsetByte: 8,
+                expectedOffsetValue: 0,
+                expectedNativeSize: 9
+            );
+
+            succeeded &= Test<ManagedAutoUnmanagedSequentialLayoutMinPacking>(
+                expectedSize: 16,
+                expectedOffsetByte: 8,
+                expectedOffsetValue: 0,
+                expectedNativeSize: 9
+            );
+        }
+        else
+        {
+            succeeded &= Test<ManagedAutoUnmanagedSequentialLayoutMinPacking>(
+                expectedSize: 8,
+                expectedOffsetByte: 4,
+                expectedOffsetValue: 0,
+                expectedNativeSize: 5
+            );
+
+            succeeded &= Test<ManagedAutoUnmanagedSequentialLayoutMinPacking>(
+                expectedSize: 8,
+                expectedOffsetByte: 4,
+                expectedOffsetValue: 0,
+                expectedNativeSize: 5
+            );
+        }
+
+        return succeeded;
+    }
 
     static bool Test<T>(int expectedSize, int expectedOffsetByte, int expectedOffsetValue) where T : ITestStructure
     {
@@ -1667,6 +1720,26 @@ static bool Test<T>(int expectedSize, int expectedOffsetByte, int expectedOffset
         return succeeded;
     }
 
+    static bool Test<T>(int expectedSize, int expectedOffsetByte, int expectedOffsetValue, int expectedNativeSize) where T : ITestStructure
+    {
+        bool succeeded = Test<T>(expectedSize, expectedOffsetByte, expectedOffsetValue);
+
+        int nativeSize = Marshal.SizeOf<T>();
+        if (nativeSize != expectedNativeSize)
+        {
+            Console.WriteLine($"Unexpected Native Size for {typeof(T)}.");
+            Console.WriteLine($"     Expected: {expectedNativeSize}; Actual: {nativeSize}");
+            succeeded = false;
+        }
+
+        if (!succeeded)
+        {
+            Console.WriteLine();
+        }
+
+        return succeeded;
+    }
+
     internal static int OffsetOf<T, U>(ref T origin, ref U target)
     {
         return Unsafe.ByteOffset(ref origin, ref Unsafe.As<U, T>(ref target)).ToInt32();