From 2e12cf3fa59d3c479a6688285345d1a181547e53 Mon Sep 17 00:00:00 2001 From: Scott Ferguson Date: Wed, 8 Dec 2021 14:23:26 -0500 Subject: [PATCH 1/2] Have CoreCLR follow sequential layout with GC refs By default CoreCLR will ignore the LayoutKind.Sequential on types containing GC references since those types are not blittable and no code outside of the runtime can access those types and will use the auto layout algorithm to change the field layout. But Unity uses a lot of custom icalls that rely on the managed layout matching the native layout. This PR adds an IsSequentialWithRefs flag on MethodTable to indicate the types with sequential layout, but we GC refs. These types: 1. Have the LayoutKind.Sequential flag 2. The type is not blittable and not managed sequential (not blittable but has no GC refs). These are already handled specially by the runtime. 3. Has a parent that is System.Object, System.ValueType, is sequential with refs, is blittable, or is managed sequential This requires only very small changes in the runtime because the explicit layout paths do the work we need already. 1. classlayoutinfo.cpp already calculates field size and sequential offsets so we only need to mark the types as sequential with refs there. 2. In methodtablebuilder.cpp we need to set the field offset to the value calculated in 1. 3. In methodtablebuilder.cpp we run HandleExplicitLayout() which will calculate the type's size (it also, needlessly in our case, validates the field offsets) 4. In methodtablebuilder.cpp we run HandleGCForExplicitLayout() which calculates the GCDesc for the type. The only other needed change was to change the definition of MetadataEnumResult in managedmdimport.hpp. This type is used in an icall that assumes the auto-layout algorithm - interestingly it has nothing to do with GC refs, but the fact that the auto-layout algorithm places larger than pointer sized structs on pointer sized boundaries. This PR also updates CrossGen2 so follows the same layout algorithm. The runtime has a lot of checks that validate that the R2R image match the current runtime layout, size, and GC desc. --- src/coreclr/clr.featuredefines.props | 2 + src/coreclr/clrdefinitions.cmake | 3 + src/coreclr/clrfeatures.cmake | 5 ++ .../Common/MetadataFieldLayoutAlgorithm.cs | 28 ++++++--- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 60 ++++++++++++++++++- .../ILCompiler.ReadyToRun.csproj | 4 ++ .../TestMetadataFieldLayoutAlgorithm.cs | 2 +- .../ILCompiler.TypeSystem.ReadyToRun.csproj | 3 + src/coreclr/vm/class.h | 30 ++++++++++ src/coreclr/vm/classlayoutinfo.cpp | 10 ++++ src/coreclr/vm/managedmdimport.hpp | 2 + src/coreclr/vm/methodtablebuilder.cpp | 33 +++++++++- src/coreclr/vm/methodtablebuilder.h | 3 + 13 files changed, 174 insertions(+), 11 deletions(-) diff --git a/src/coreclr/clr.featuredefines.props b/src/coreclr/clr.featuredefines.props index 511db52edc102..386c2e67e3f3f 100644 --- a/src/coreclr/clr.featuredefines.props +++ b/src/coreclr/clr.featuredefines.props @@ -11,6 +11,7 @@ true true true + true @@ -64,5 +65,6 @@ $(DefineConstants);PROFILING_SUPPORTED $(DefineConstants);FEATURE_PROFAPI_ATTACH_DETACH + $(DefineConstants);FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS diff --git a/src/coreclr/clrdefinitions.cmake b/src/coreclr/clrdefinitions.cmake index ffadad3552100..08e4c5b77fc7c 100644 --- a/src/coreclr/clrdefinitions.cmake +++ b/src/coreclr/clrdefinitions.cmake @@ -219,6 +219,9 @@ if (CLR_CMAKE_TARGET_WIN32 AND CLR_CMAKE_TARGET_ARCH_AMD64) add_definitions(-DFEATURE_SPECIAL_USER_MODE_APC) endif() +if (FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS) + add_definitions(-DFEATURE_SEQUENTIAL_LAYOUT_WITH_REFS) +endif() # Use this function to enable building with a specific target OS and architecture set of defines # This is known to work for the set of defines used by the JIT and gcinfo, it is not likely correct for diff --git a/src/coreclr/clrfeatures.cmake b/src/coreclr/clrfeatures.cmake index f82ff1aa4e73e..cfd7b0d909843 100644 --- a/src/coreclr/clrfeatures.cmake +++ b/src/coreclr/clrfeatures.cmake @@ -39,3 +39,8 @@ endif() if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS) set(FEATURE_OBJCMARSHAL 1) endif() + +# should default this to 0 once we figure out how to enable in build scripting +if(NOT DEFINED FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS) + set(FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS 1) +endif(NOT DEFINED FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 362fd1763d230..24e8f29c6b963 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -104,6 +104,7 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp type.Context.Target.GetWellKnownTypeAlignment(type), 0, alignUpInstanceByteSize: true, + ignoreLayoutAlignment: false, out instanceByteSizeAndAlignment ); @@ -302,7 +303,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty LayoutInt instanceSize = cumulativeInstanceFieldPos + offsetBias; var layoutMetadata = type.GetClassLayout(); - int packingSize = ComputePackingSize(type, layoutMetadata); + int packingSize = ComputePackingSize(type, layoutMetadata, false); LayoutInt largestAlignmentRequired = LayoutInt.One; var offsets = new FieldAndOffset[numInstanceFields]; @@ -356,6 +357,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty largestAlignmentRequired, layoutMetadata.Size, alignUpInstanceByteSize: AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(type), + ignoreLayoutAlignment: false, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); @@ -376,7 +378,7 @@ private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, Layo return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target); } - protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields) + protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields, bool isSequentialWithRefs) { var offsets = new FieldAndOffset[numInstanceFields]; @@ -390,7 +392,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType LayoutInt largestAlignmentRequirement = LayoutInt.One; int fieldOrdinal = 0; - int packingSize = ComputePackingSize(type, layoutMetadata); + int packingSize = ComputePackingSize(type, layoutMetadata, ignoreLayoutPacking: isSequentialWithRefs); bool layoutAbiStable = true; foreach (var field in type.GetFields()) @@ -418,6 +420,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType largestAlignmentRequirement, layoutMetadata.Size, alignUpInstanceByteSize: true, + ignoreLayoutAlignment: isSequentialWithRefs, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); @@ -442,7 +445,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, bool hasLayout = type.HasLayout(); var layoutMetadata = type.GetClassLayout(); - int packingSize = ComputePackingSize(type, layoutMetadata); + int packingSize = ComputePackingSize(type, layoutMetadata, false); packingSize = Math.Min(context.Target.MaximumAutoLayoutPackingSize, packingSize); var offsets = new FieldAndOffset[numInstanceFields]; @@ -706,6 +709,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, minAlign, classLayoutSize: 0, alignUpInstanceByteSize: true, + ignoreLayoutAlignment: false, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); @@ -828,15 +832,15 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, return result; } - private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata layoutMetadata) + private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata layoutMetadata, bool ignoreLayoutPacking) { - if (layoutMetadata.PackingSize == 0) + if (layoutMetadata.PackingSize == 0 || ignoreLayoutPacking) return type.Context.Target.DefaultPackingSize; 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, bool alignUpInstanceByteSize, bool ignoreLayoutAlignment, out SizeAndAlignment byteCount) { SizeAndAlignment result; @@ -862,6 +866,16 @@ private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt } else { +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + if (ignoreLayoutAlignment) + { + // Even if we want to ignore layout alignment, the GC requires types with GC refs to be + // sized to a multiple of LayoutPointerSize + if (type.ContainsGCPointers) + instanceSize = LayoutInt.AlignUp(instanceSize, target.LayoutPointerSize, target); + } + else +#endif if (type.IsValueType) { instanceSize = LayoutInt.AlignUp(instanceSize, diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index 76e237cec5c1c..9cdebdea70f25 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -803,9 +803,9 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada return ComputeExplicitFieldLayout(type, numInstanceFields); } else - if (type.IsEnum || MarshalUtils.IsBlittableType(type) || IsManagedSequentialType(type)) + if (ShouldLayoutSequential(type, out var isSequentialWithRefs)) { - return ComputeSequentialFieldLayout(type, numInstanceFields); + return ComputeSequentialFieldLayout(type, numInstanceFields, isSequentialWithRefs); } else { @@ -865,5 +865,61 @@ public static bool IsManagedSequentialType(TypeDesc type) return true; } + + private static bool ShouldLayoutSequential(TypeDesc type, out bool isSequentialWithRefs) + { + isSequentialWithRefs = false; + + var name = type.GetDisplayName(); + + if (type.IsEnum) + { + return true; + } + + if (MarshalUtils.IsBlittableType(type)) + { + return true; + } + + if (IsManagedSequentialType(type)) + { + return true; + } + +#if !FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + return false; + } +#else + isSequentialWithRefs = HasValidSequentialLayout(type); + return isSequentialWithRefs; + } + + private static bool HasValidSequentialLayout(TypeDesc type) + { + var metadataType = type as MetadataType; + if (metadataType == null) + { + return false; + } + + if (!metadataType.IsSequentialLayout) + { + return false; + } + + if (!metadataType.HasBaseType) + { + return false; + } + + if (metadataType.BaseType.IsObject || metadataType.BaseType.IsWellKnownType(WellKnownType.ValueType)) + { + return true; + } + + return HasValidSequentialLayout(type.BaseType); + } +#endif } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj index bce5efe4e0905..aee3add894c1f 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj @@ -17,6 +17,10 @@ Debug;Release;Checked + + + + diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/TestMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/TestMetadataFieldLayoutAlgorithm.cs index fdd8382a5bdcf..05b03614f404d 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/TestMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/TestMetadataFieldLayoutAlgorithm.cs @@ -39,7 +39,7 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada } else if (type.IsSequentialLayout || type.IsEnum) { - return ComputeSequentialFieldLayout(type, numInstanceFields); + return ComputeSequentialFieldLayout(type, numInstanceFields, false); } else { diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj index 86b9c06a6cf0d..61688aa2950d6 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun/ILCompiler.TypeSystem.ReadyToRun.csproj @@ -19,6 +19,9 @@ Debug;Release;Checked + + + Internal.TypeSystem.Strings.resources diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 1817658119d17..174cb0ab74ea7 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -376,6 +376,11 @@ class EEClassLayoutInfo e_ZERO_SIZED = 0x04, // The size of the struct is explicitly specified in the meta-data. e_HAS_EXPLICIT_SIZE = 0x08 + +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + // The type has GC references, but respect sequential layout + , e_SEQUENTIAL_WITH_REFS = 0x10 +#endif }; BYTE m_bFlags; @@ -419,6 +424,14 @@ class EEClassLayoutInfo return (m_bFlags & e_HAS_EXPLICIT_SIZE) == e_HAS_EXPLICIT_SIZE; } +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + BOOL IsSequentialWithRefs() const + { + LIMITED_METHOD_CONTRACT; + return (m_bFlags & e_SEQUENTIAL_WITH_REFS) == e_SEQUENTIAL_WITH_REFS; + } +#endif + BYTE GetPackingSize() const { LIMITED_METHOD_CONTRACT; @@ -453,6 +466,13 @@ class EEClassLayoutInfo m_bFlags = hasExplicitSize ? (m_bFlags | e_HAS_EXPLICIT_SIZE) : (m_bFlags & ~e_HAS_EXPLICIT_SIZE); } + + void SetIsSequentialWithRefs(BOOL isSequenialWithRefs) + { + LIMITED_METHOD_CONTRACT; + m_bFlags = isSequenialWithRefs ? (m_bFlags | e_SEQUENTIAL_WITH_REFS) + : (m_bFlags & ~e_SEQUENTIAL_WITH_REFS); + } }; // @@ -1382,6 +1402,10 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! BOOL HasExplicitSize(); +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + BOOL IsSequentialWithRefs(); +#endif + static void GetBestFitMapping(MethodTable * pMT, BOOL *pfBestFitMapping, BOOL *pfThrowOnUnmappableChar); /* @@ -2062,6 +2086,12 @@ inline BOOL EEClass::HasExplicitSize() return HasLayout() && GetLayoutInfo()->HasExplicitSize(); } +inline BOOL EEClass::IsSequentialWithRefs() +{ + LIMITED_METHOD_CONTRACT; + return HasLayout() && GetLayoutInfo()->IsSequentialWithRefs(); +} + //========================================================================== // These routines manage the prestub (a bootstrapping stub that all // FunctionDesc's are initialized with.) diff --git a/src/coreclr/vm/classlayoutinfo.cpp b/src/coreclr/vm/classlayoutinfo.cpp index 749620609dfdd..a5cff1dd099e5 100644 --- a/src/coreclr/vm/classlayoutinfo.cpp +++ b/src/coreclr/vm/classlayoutinfo.cpp @@ -710,6 +710,16 @@ VOID EEClassLayoutInfo::CollectLayoutFieldMetadataThrowing( } pEEClassLayoutInfoOut->SetIsManagedSequential(!fDisqualifyFromManagedSequential); + +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + BOOL fIsSequentialWithRefs = FALSE; + if (fDisqualifyFromManagedSequential && !fExplicitOffsets) + { + fIsSequentialWithRefs = !fHasNonTrivialParent || (fParentHasLayout && (pParentLayoutInfo->IsSequentialWithRefs() || pParentLayoutInfo->IsManagedSequential() || pParentLayoutInfo->IsBlittable())); + } + + pEEClassLayoutInfoOut->SetIsSequentialWithRefs(fIsSequentialWithRefs); +#endif } void EEClassNativeLayoutInfo::InitializeNativeLayoutFieldMetadataThrowing(MethodTable* pMT) diff --git a/src/coreclr/vm/managedmdimport.hpp b/src/coreclr/vm/managedmdimport.hpp index ffc1efb2b7a64..08476ccfe10cb 100644 --- a/src/coreclr/vm/managedmdimport.hpp +++ b/src/coreclr/vm/managedmdimport.hpp @@ -29,7 +29,9 @@ typedef struct I4Array * largeResult; int length; #ifdef HOST_64BIT +#ifndef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS int padding; +#endif #endif int smallResult[16]; } MetadataEnumResult; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index d5feeae4425e2..9dffe5de6c552 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1734,6 +1734,13 @@ MethodTableBuilder::BuildMethodTableThrowing( HandleExplicitLayout(pByValueClassCache); } } +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + else if (IsSequentialWithRefs() && !bmtGenerics->fContainsGenericVariables) + { + _ASSERTE(HasLayout()); + HandleExplicitLayout(pByValueClassCache); + } +#endif else { _ASSERTE(!IsBlittable()); @@ -1828,6 +1835,10 @@ MethodTableBuilder::BuildMethodTableThrowing( if (HasExplicitFieldOffsetLayout()) // Perform relevant GC calculations for tdexplicit HandleGCForExplicitLayout(); +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + else if (IsSequentialWithRefs()) + HandleGCForExplicitLayout(); +#endif else // Perform relevant GC calculations for value classes HandleGCForValueClasses(pByValueClassCache); @@ -4252,6 +4263,15 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, IfFailThrow(pFD->SetOffset(pLayoutFieldInfo->m_placement.m_offset)); } +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + else if (!fIsStatic && IsSequentialWithRefs()) + { + (DWORD_PTR &)pFD->m_pMTOfEnclosingClass = + (*pByValueClassCache)[dwCurrentDeclaredField]->GetNumInstanceFieldBytes(); + + IfFailThrow(pFD->SetOffset(pLayoutFieldInfo->m_placement.m_offset)); + } +#endif else { // static value class fields hold a handle, which is ptr sized @@ -4274,6 +4294,10 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, IfFailThrow(pFD->SetOffset(pLayoutFieldInfo->m_placement.m_offset)); else if (IsManagedSequential() && !fIsStatic) IfFailThrow(pFD->SetOffset(pLayoutFieldInfo->m_placement.m_offset)); +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + else if (IsSequentialWithRefs() && !fIsStatic) + IfFailThrow(pFD->SetOffset(pLayoutFieldInfo->m_placement.m_offset)); +#endif else if (bCurrentFieldIsGCPointer) pFD->SetOffset(FIELD_OFFSET_UNPLACED_GC_PTR); else @@ -8258,7 +8282,14 @@ DWORD MethodTableBuilder::GetFieldSize(FieldDesc *pFD) // We should only be calling this while this class is being built. _ASSERTE(GetHalfBakedMethodTable() == 0); - BAD_FORMAT_NOTHROW_ASSERT(! pFD->IsByValue() || HasExplicitFieldOffsetLayout()); + + +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + BAD_FORMAT_NOTHROW_ASSERT(! pFD->IsByValue() || HasExplicitFieldOffsetLayout() || IsSequentialWithRefs() ); +#else + BAD_FORMAT_NOTHROW_ASSERT(! pFD->IsByValue() || HasExplicitFieldOffsetLayout() ); +#endif + if (pFD->IsByValue()) return (DWORD)(DWORD_PTR&)(pFD->m_pMTOfEnclosingClass); diff --git a/src/coreclr/vm/methodtablebuilder.h b/src/coreclr/vm/methodtablebuilder.h index 42cae0b22c1e9..9d46519de6dfc 100644 --- a/src/coreclr/vm/methodtablebuilder.h +++ b/src/coreclr/vm/methodtablebuilder.h @@ -230,6 +230,9 @@ class MethodTableBuilder BOOL HasExplicitFieldOffsetLayout() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->HasExplicitFieldOffsetLayout(); } BOOL IsManagedSequential() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->IsManagedSequential(); } BOOL HasExplicitSize() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->HasExplicitSize(); } +#ifdef FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS + BOOL IsSequentialWithRefs() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->IsSequentialWithRefs(); } +#endif #ifdef _DEBUG LPCUTF8 GetDebugClassName() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->GetDebugClassName(); } From 9bd5682eeba99d3d6bf9fd3bff3be87072de2546 Mon Sep 17 00:00:00 2001 From: Scott Ferguson Date: Thu, 9 Dec 2021 15:49:15 -0500 Subject: [PATCH 2/2] Make our changes to crossgen2 extra obvious --- .../Common/MetadataFieldLayoutAlgorithm.cs | 36 +++++++++++++++++++ .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 12 ++++--- ...ompiler.TypeSystem.ReadyToRun.Tests.csproj | 3 ++ .../TestMetadataFieldLayoutAlgorithm.cs | 4 +++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 24e8f29c6b963..7bbebe81a8e60 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -104,7 +104,9 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp type.Context.Target.GetWellKnownTypeAlignment(type), 0, alignUpInstanceByteSize: true, +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS ignoreLayoutAlignment: false, +#endif out instanceByteSizeAndAlignment ); @@ -303,7 +305,11 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty LayoutInt instanceSize = cumulativeInstanceFieldPos + offsetBias; var layoutMetadata = type.GetClassLayout(); +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS int packingSize = ComputePackingSize(type, layoutMetadata, false); +#else + int packingSize = ComputePackingSize(type, layoutMetadata); +#endif LayoutInt largestAlignmentRequired = LayoutInt.One; var offsets = new FieldAndOffset[numInstanceFields]; @@ -357,7 +363,9 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty largestAlignmentRequired, layoutMetadata.Size, alignUpInstanceByteSize: AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(type), +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS ignoreLayoutAlignment: false, +#endif out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); @@ -378,7 +386,11 @@ private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, Layo return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target); } +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields, bool isSequentialWithRefs) +#else + protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields) +#endif { var offsets = new FieldAndOffset[numInstanceFields]; @@ -392,7 +404,11 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType LayoutInt largestAlignmentRequirement = LayoutInt.One; int fieldOrdinal = 0; +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS int packingSize = ComputePackingSize(type, layoutMetadata, ignoreLayoutPacking: isSequentialWithRefs); +#else + int packingSize = ComputePackingSize(type, layoutMetadata); +#endif bool layoutAbiStable = true; foreach (var field in type.GetFields()) @@ -420,7 +436,9 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType largestAlignmentRequirement, layoutMetadata.Size, alignUpInstanceByteSize: true, +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS ignoreLayoutAlignment: isSequentialWithRefs, +#endif out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); @@ -445,7 +463,11 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, bool hasLayout = type.HasLayout(); var layoutMetadata = type.GetClassLayout(); +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS int packingSize = ComputePackingSize(type, layoutMetadata, false); +#else + int packingSize = ComputePackingSize(type, layoutMetadata); +#endif packingSize = Math.Min(context.Target.MaximumAutoLayoutPackingSize, packingSize); var offsets = new FieldAndOffset[numInstanceFields]; @@ -709,7 +731,9 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, minAlign, classLayoutSize: 0, alignUpInstanceByteSize: true, +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS ignoreLayoutAlignment: false, +#endif out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); @@ -832,15 +856,27 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, return result; } +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata layoutMetadata, bool ignoreLayoutPacking) +#else + private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata layoutMetadata) +#endif { +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS if (layoutMetadata.PackingSize == 0 || ignoreLayoutPacking) +#else + if (layoutMetadata.PackingSize == 0) +#endif return type.Context.Target.DefaultPackingSize; else return layoutMetadata.PackingSize; } +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, bool alignUpInstanceByteSize, bool ignoreLayoutAlignment, out SizeAndAlignment byteCount) +#else + private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, bool alignUpInstanceByteSize, out SizeAndAlignment byteCount) +#endif { SizeAndAlignment result; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index 9cdebdea70f25..ecfad235739b3 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -803,10 +803,17 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada return ComputeExplicitFieldLayout(type, numInstanceFields); } else +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS if (ShouldLayoutSequential(type, out var isSequentialWithRefs)) { return ComputeSequentialFieldLayout(type, numInstanceFields, isSequentialWithRefs); } +#else + if (type.IsEnum || MarshalUtils.IsBlittableType(type) || IsManagedSequentialType(type)) + { + return ComputeSequentialFieldLayout(type, numInstanceFields); + } +#endif else { return ComputeAutoFieldLayout(type, numInstanceFields); @@ -866,6 +873,7 @@ public static bool IsManagedSequentialType(TypeDesc type) return true; } +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS private static bool ShouldLayoutSequential(TypeDesc type, out bool isSequentialWithRefs) { isSequentialWithRefs = false; @@ -887,10 +895,6 @@ private static bool ShouldLayoutSequential(TypeDesc type, out bool isSequentialW return true; } -#if !FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS - return false; - } -#else isSequentialWithRefs = HasValidSequentialLayout(type); return isSequentialWithRefs; } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj index 558daa29f76b7..22b9f5d7c7ea8 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj @@ -16,6 +16,9 @@ true + + + diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/TestMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/TestMetadataFieldLayoutAlgorithm.cs index 05b03614f404d..f23289a48ab9b 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/TestMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/TestMetadataFieldLayoutAlgorithm.cs @@ -39,7 +39,11 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada } else if (type.IsSequentialLayout || type.IsEnum) { +#if FEATURE_SEQUENTIAL_LAYOUT_WITH_REFS return ComputeSequentialFieldLayout(type, numInstanceFields, false); +#else + return ComputeSequentialFieldLayout(type, numInstanceFields); +#endif } else {