Change all "unmanaged" (no GC fields) sequential types to have sequential layout. (#61759)

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

Author: jkoritzinsky

src/coreclr/debug/ee/debugger.cpp

@@ -4360,7 +4360,7 @@ SIZE_T GetSetFrameHelper::GetValueClassSize(MetaSig* pSig)
     // - but we don't care if it's shared (since it will be the same size either way)
     _ASSERTE(!vcType.IsNull() && vcType.IsValueType());
 
-    return (vcType.GetMethodTable()->GetAlignedNumInstanceFieldBytes());
+    return (vcType.GetMethodTable()->GetNumInstanceFieldBytes());
 }
 
 //

src/coreclr/inc/readytorun.h

@@ -15,15 +15,17 @@
 #define READYTORUN_SIGNATURE 0x00525452 // 'RTR'
 
 // Keep these in sync with src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs
-#define READYTORUN_MAJOR_VERSION 0x0005
-#define READYTORUN_MINOR_VERSION 0x0004
+#define READYTORUN_MAJOR_VERSION 0x0006
+#define READYTORUN_MINOR_VERSION 0x0000
 
-#define MINIMUM_READYTORUN_MAJOR_VERSION 0x003
+#define MINIMUM_READYTORUN_MAJOR_VERSION 0x006
 
 // R2R Version 2.1 adds the InliningInfo section
 // R2R Version 2.2 adds the ProfileDataInfo section
 // R2R Version 3.0 changes calling conventions to correctly handle explicit structures to spec.
 //     R2R 3.0 is not backward compatible with 2.x.
+// R2R Version 6.0 changes managed layout for sequential types with any unmanaged non-blittable fields.
+//     R2R 6.0 is not backward compatible with 5.x or earlier.
 
 struct READYTORUN_CORE_HEADER
 {

src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs

@@ -14,8 +14,8 @@ internal struct ReadyToRunHeaderConstants
     {
         public const uint Signature = 0x00525452; // 'RTR'
 
-        public const ushort CurrentMajorVersion = 5;
-        public const ushort CurrentMinorVersion = 4;
+        public const ushort CurrentMajorVersion = 6;
+        public const ushort CurrentMinorVersion = 0;
     }
 
 #pragma warning disable 0169

src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs

@@ -103,7 +103,6 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp
                     type.Context.Target.GetWellKnownTypeSize(type),
                     type.Context.Target.GetWellKnownTypeAlignment(type),
                     0,
-                    alignUpInstanceByteSize: true,
                     out instanceByteSizeAndAlignment
                     );
 
@@ -291,8 +290,6 @@ protected virtual void FinalizeRuntimeSpecificStaticFieldLayout(TypeSystemContex
         {
         }
 
-        protected virtual bool AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(TypeDesc type) => true;
-
         protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType type, int numInstanceFields)
         {
             // Instance slice size is the total size of instance not including the base type.
@@ -312,7 +309,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
             foreach (var fieldAndOffset in layoutMetadata.Offsets)
             {
                 TypeDesc fieldType = fieldAndOffset.Field.FieldType;
-                var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable);
+                var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType.UnderlyingType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable);
                 if (!fieldLayoutAbiStable)
                     layoutAbiStable = false;
 
@@ -355,7 +352,6 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
                 instanceSize,
                 largestAlignmentRequired,
                 layoutMetadata.Size,
-                alignUpInstanceByteSize: AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(type),
                 out instanceByteSizeAndAlignment);
 
             ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
@@ -398,7 +394,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType
                 if (field.IsStatic)
                     continue;
 
-                var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable);
+                var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType.UnderlyingType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable);
                 if (!fieldLayoutAbiStable)
                     layoutAbiStable = false;
 
@@ -417,7 +413,6 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType
                 cumulativeInstanceFieldPos + offsetBias,
                 largestAlignmentRequirement,
                 layoutMetadata.Size,
-                alignUpInstanceByteSize: true,
                 out instanceByteSizeAndAlignment);
 
             ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
@@ -442,8 +437,8 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
             bool hasLayout = type.HasLayout();
             var layoutMetadata = type.GetClassLayout();
 
-            int packingSize = ComputePackingSize(type, layoutMetadata);
-            packingSize = Math.Min(context.Target.MaximumAutoLayoutPackingSize, packingSize);
+            // Auto-layout in CoreCLR does not respect packing size.
+            int packingSize = type.Context.Target.MaximumAlignment;
 
             var offsets = new FieldAndOffset[numInstanceFields];
             int fieldOrdinal = 0;
@@ -543,7 +538,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
             LayoutInt offsetBias = LayoutInt.Zero;
             if (!type.IsValueType && cumulativeInstanceFieldPos != LayoutInt.Zero && type.Context.Target.Architecture == TargetArchitecture.X86)
             {
-                offsetBias = new LayoutInt(type.Context.Target.PointerSize);
+                offsetBias = type.Context.Target.LayoutPointerSize;
                 cumulativeInstanceFieldPos -= offsetBias;
             }
 
@@ -650,29 +645,14 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
             // Place value class fields last
             for (int i = 0; i < instanceValueClassFieldsArr.Length; i++)
             {
-                // If the field has an indeterminate alignment, align the cumulative field offset to the indeterminate value
-                // Otherwise, align the cumulative field offset to the PointerSize
-                // This avoids issues with Universal Generic Field layouts whose fields may have Indeterminate sizes or alignments
+                // Align the cumulative field offset to the indeterminate value
                 var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(instanceValueClassFieldsArr[i].FieldType, hasLayout, packingSize, out bool fieldLayoutAbiStable);
                 if (!fieldLayoutAbiStable)
                     layoutAbiStable = false;
 
-                if (fieldSizeAndAlignment.Alignment.IsIndeterminate)
-                {
-                    cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, context.Target);
-                }
-                else
-                {
-                    LayoutInt AlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, context.Target.LayoutPointerSize);
-                    cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, AlignmentRequired, context.Target);
-                }
+                cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, context.Target);
                 offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos + offsetBias);
-
-                // If the field has an indeterminate size, align the cumulative field offset to the indeterminate value
-                // Otherwise, align the cumulative field offset to the aligned-instance field size
-                // This avoids issues with Universal Generic Field layouts whose fields may have Indeterminate sizes or alignments
-                LayoutInt alignedInstanceFieldBytes = fieldSizeAndAlignment.Size.IsIndeterminate ? fieldSizeAndAlignment.Size : GetAlignedNumInstanceFieldBytes(fieldSizeAndAlignment.Size);
-                cumulativeInstanceFieldPos = checked(cumulativeInstanceFieldPos + alignedInstanceFieldBytes);
+                cumulativeInstanceFieldPos = checked(cumulativeInstanceFieldPos + fieldSizeAndAlignment.Size);
 
                 fieldOrdinal++;
             }
@@ -687,11 +667,18 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
             }
             else if (cumulativeInstanceFieldPos.AsInt > context.Target.PointerSize)
             {
-                minAlign = context.Target.LayoutPointerSize;
-                if (requiresAlign8 && minAlign.AsInt == 4)
+                if (requiresAlign8)
                 {
                     minAlign = new LayoutInt(8);
                 }
+                else if (type.ContainsGCPointers)
+                {
+                    minAlign = context.Target.LayoutPointerSize;
+                }
+                else
+                {
+                    minAlign = largestAlignmentRequired;
+                }
             }
             else
             {
@@ -705,8 +692,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type,
                 cumulativeInstanceFieldPos + offsetBias,
                 minAlign,
                 classLayoutSize: 0,
-                alignUpInstanceByteSize: true,
-                out instanceByteSizeAndAlignment);
+                byteCount: out instanceByteSizeAndAlignment);
 
             ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
             computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment;
@@ -790,10 +776,10 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType,
             {
                 if (fieldType.IsValueType)
                 {
-                    DefType metadataType = (DefType)fieldType;
-                    result.Size = metadataType.InstanceFieldSize;
-                    result.Alignment = metadataType.InstanceFieldAlignment;
-                    layoutAbiStable = metadataType.LayoutAbiStable;
+                    DefType defType = (DefType)fieldType;
+                    result.Size = defType.InstanceFieldSize;
+                    result.Alignment = defType.InstanceFieldAlignment;
+                    layoutAbiStable = defType.LayoutAbiStable;
                 }
                 else
                 {
@@ -816,27 +802,24 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType,
                 result.Alignment = fieldType.Context.Target.LayoutPointerSize;
             }
 
+            // For non-auto layouts, we need to respect tighter packing requests for alignment.
             if (hasLayout)
             {
                 result.Alignment = LayoutInt.Min(result.Alignment, new LayoutInt(packingSize));
             }
-            else
-            {
-                result.Alignment = LayoutInt.Min(result.Alignment, fieldType.Context.Target.GetObjectAlignment(result.Alignment));
-            }
 
             return result;
         }
 
         private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata layoutMetadata)
         {
             if (layoutMetadata.PackingSize == 0)
-                return type.Context.Target.DefaultPackingSize;
+                return type.Context.Target.MaximumAlignment;
             else
                 return layoutMetadata.PackingSize;
         }
 
-        private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, bool alignUpInstanceByteSize, out SizeAndAlignment byteCount)
+        private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, out SizeAndAlignment byteCount)
         {
             SizeAndAlignment result;
 
@@ -864,9 +847,7 @@ private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt
             {
                 if (type.IsValueType)
                 {
-                    instanceSize = LayoutInt.AlignUp(instanceSize,
-                        alignUpInstanceByteSize ? alignment : LayoutInt.Min(alignment, target.LayoutPointerSize),
-                        target);
+                    instanceSize = LayoutInt.AlignUp(instanceSize, alignment, target);
                 }
             }
 

src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs

@@ -324,20 +324,5 @@ public int MaxHomogeneousAggregateElementCount
                 return 4;
             }
         }
-
-        public int MaximumAutoLayoutPackingSize
-        {
-            get
-            {
-                if (Abi == TargetAbi.CoreRT)
-                {
-                    if (Architecture == TargetArchitecture.X86)
-                    {
-                        return PointerSize;
-                    }
-                }
-                return MaximumAlignment;
-            }
-        }
     }
 }

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs

@@ -196,9 +196,9 @@ protected override ModuleFieldLayout CreateValueFromKey(EcmaModule module)
                         }
                     }
 
-                    if (nonGcBytes[StaticIndex.Regular] != 0 || 
+                    if (nonGcBytes[StaticIndex.Regular] != 0 ||
                         nonGcBytes[StaticIndex.ThreadLocal] != 0 ||
-                        gcBytes[StaticIndex.Regular] != 0 || 
+                        gcBytes[StaticIndex.Regular] != 0 ||
                         gcBytes[StaticIndex.ThreadLocal] != 0)
                     {
                         OffsetsForType offsetsForType = new OffsetsForType(LayoutInt.Indeterminate, LayoutInt.Indeterminate, LayoutInt.Indeterminate, LayoutInt.Indeterminate);
@@ -290,14 +290,14 @@ private void GetElementTypeInfoGeneric(
             }
 
             private void GetElementTypeInfo(
-                EcmaModule module, 
+                EcmaModule module,
                 FieldDesc fieldDesc,
-                EntityHandle valueTypeHandle, 
+                EntityHandle valueTypeHandle,
                 CorElementType elementType,
                 int pointerSize,
                 bool moduleLayout,
-                out int alignment, 
-                out int size, 
+                out int alignment,
+                out int size,
                 out bool isGcPointerField,
                 out bool isGcBoxedField)
             {
@@ -357,7 +357,7 @@ private void GetElementTypeInfo(
                         ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, fieldDesc.OwningType);
                         break;
 
-                    // Statics for valuetypes where the valuetype is defined in this module are handled here. 
+                    // Statics for valuetypes where the valuetype is defined in this module are handled here.
                     // Other valuetype statics utilize the pessimistic model below.
                     case CorElementType.ELEMENT_TYPE_VALUETYPE:
                         if (IsTypeByRefLike(valueTypeHandle, module.MetadataReader))
@@ -522,7 +522,7 @@ public FieldAndOffset[] CalculateTypeLayout(DefType defType, EcmaModule module,
                     offsetsForType.GcOffsets[StaticIndex.ThreadLocal],
                 };
 
-                LayoutInt[] gcPointerFieldOffsets = new LayoutInt[StaticIndex.Count] 
+                LayoutInt[] gcPointerFieldOffsets = new LayoutInt[StaticIndex.Count]
                 {
                     offsetsForType.GcOffsets[StaticIndex.Regular] + new LayoutInt(gcBoxedCount[StaticIndex.Regular] * pointerSize),
                     offsetsForType.GcOffsets[StaticIndex.ThreadLocal] + new LayoutInt(gcBoxedCount[StaticIndex.ThreadLocal] * pointerSize)
@@ -768,10 +768,10 @@ private class ModuleFieldLayout
             private ConcurrentDictionary<DefType, FieldAndOffset[]> _genericTypeToFieldMap;
 
             public ModuleFieldLayout(
-                EcmaModule module, 
-                StaticsBlock gcStatics, 
-                StaticsBlock nonGcStatics, 
-                StaticsBlock threadGcStatics, 
+                EcmaModule module,
+                StaticsBlock gcStatics,
+                StaticsBlock nonGcStatics,
+                StaticsBlock threadGcStatics,
                 StaticsBlock threadNonGcStatics,
                 IReadOnlyDictionary<TypeDefinitionHandle, OffsetsForType> typeOffsets)
             {
@@ -802,8 +802,7 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada
             {
                 return ComputeExplicitFieldLayout(type, numInstanceFields);
             }
-            else
-            if (type.IsEnum || MarshalUtils.IsBlittableType(type) || IsManagedSequentialType(type))
+            else if (type.IsSequentialLayout && !type.ContainsGCPointers)
             {
                 return ComputeSequentialFieldLayout(type, numInstanceFields);
             }
@@ -814,7 +813,7 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada
         }
 
         /// <summary>
-        /// This method decides whether the type needs aligned base offset in order to have layout resilient to 
+        /// This method decides whether the type needs aligned base offset in order to have layout resilient to
         /// base class layout changes.
         /// </summary>
         protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8, bool requiresAlignedBase)
@@ -826,44 +825,5 @@ protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref Layout
                 baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target);
             }
         }
-
-        protected override bool AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(TypeDesc type)
-        {
-            return MarshalUtils.IsBlittableType(type) || IsManagedSequentialType(type);
-        }
-
-        public static bool IsManagedSequentialType(TypeDesc type)
-        {
-            if (type.IsPointer)
-            {
-                return true;
-            }
-
-            if (!type.IsValueType)
-            {
-                return false;
-            }
-
-            MetadataType metadataType = (MetadataType)type;
-            if (metadataType.IsExplicitLayout || !metadataType.IsSequentialLayout)
-            {
-                return false;
-            }
-
-            if (type.IsPrimitive)
-            {
-                return true;
-            }
-
-            foreach (FieldDesc field in type.GetFields())
-            {
-                if (!field.IsStatic && !IsManagedSequentialType(field.FieldType.UnderlyingType))
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
     }
 }