Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 36 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
3c4ccbc
Change all "unmanaged" (no GC fields) sequential types to have sequen…
jkoritzinsky Nov 18, 2021
6b9e41d
Merge branch 'main' into managedseqential_unmanaged
jkoritzinsky Nov 18, 2021
b85447d
Fix inverted condition
jkoritzinsky Nov 19, 2021
0ff8540
Account for RequiresAlign8 for auto-layout nested structs in sequenti…
jkoritzinsky Nov 22, 2021
c6ee36d
Clean up R2R layout algorithm's handling of the new managed-sequentia…
jkoritzinsky Nov 29, 2021
99c0244
Update src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRu…
jkoritzinsky Nov 30, 2021
0ec572a
Try to remove the explicit layout quirk by following the behavior in …
jkoritzinsky Nov 30, 2021
e2589f3
Fix missed cast.
jkoritzinsky Nov 30, 2021
1744d82
Update src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayout…
jkoritzinsky Nov 30, 2021
bbaa220
AlignUp the right variable.
jkoritzinsky Nov 30, 2021
7afb6d2
Remove alignUpInstanceByteSize parameter as it's always true
jkoritzinsky Nov 30, 2021
f8f8390
Merge branch 'main' of github.com:dotnet/runtime into managedseqentia…
jkoritzinsky Nov 30, 2021
0e317fe
Make enums use sequential layout again since the auto-layout algorith…
jkoritzinsky Nov 30, 2021
f174cad
Remove HasLayoutMetadata branch and method since the only cases where…
jkoritzinsky Nov 30, 2021
6a90bb3
Handle enum alignment concerns while allowing enums themselves to hav…
jkoritzinsky Dec 3, 2021
880430a
Update the auto-layout algorithm to better track alignment requiremen…
jkoritzinsky Dec 8, 2021
83e7c80
Merge branch 'main' of github.com:dotnet/runtime into managedseqentia…
jkoritzinsky Dec 9, 2021
91df6da
Remove now-invalid alignment fixup
jkoritzinsky Dec 9, 2021
8196fbe
Handle the case of an auto-layout struct as a field in another struct.
jkoritzinsky Dec 9, 2021
da12b83
Store the custom alignment requirement for layoutkind.auto value type…
jkoritzinsky Dec 10, 2021
47c95bf
Update StructPacking test based on failures.
jkoritzinsky Dec 10, 2021
c309a69
Fix more of the struct packing test results
jkoritzinsky Dec 10, 2021
6cfe0c7
Don't double-count the adjustment for auto-layout reference type base…
jkoritzinsky Dec 10, 2021
f6af360
Fix GCSeries calculation for platforms that have the specialized alig…
jkoritzinsky Dec 10, 2021
66f80e8
Update check to specifically check if the base size is pointer sized.…
jkoritzinsky Dec 10, 2021
8501319
Update StructPacking.cs for arm and arm64 (we now respect alignment r…
jkoritzinsky Dec 10, 2021
1b9eda3
Fix default packing size calculations
jkoritzinsky Dec 10, 2021
2f74ce3
Try one more change to fix the last issue
jkoritzinsky Dec 11, 2021
c98ae95
Cleanup and update some corner cases of the managed-sequential case.
jkoritzinsky Dec 13, 2021
966af36
Fix nites.
jkoritzinsky Dec 13, 2021
ae9aa6c
Fix some copy-paste issues.
jkoritzinsky Dec 13, 2021
842aef2
Fix ->/. typo
jkoritzinsky Dec 13, 2021
d269851
Fix disqualifying from managed-sequential when RequiresAlign8 is true…
jkoritzinsky Dec 13, 2021
e423c3c
Update x86 tests that check sizeof for types that have changed size.
jkoritzinsky Dec 14, 2021
a1bc86e
Fix Align8 candidate calculation to not account for packing for auto-…
jkoritzinsky Dec 14, 2021
53e5644
Merge branch 'managedseqential_unmanaged' of github.com:jkoritzinsky/…
jkoritzinsky Dec 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/coreclr/inc/readytorun.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,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.
Expand Down Expand Up @@ -369,7 +367,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
instanceSize,
largestAlignmentRequired,
layoutMetadata.Size,
alignUpInstanceByteSize: AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(type),
alignUpInstanceByteSize: false,
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
out instanceByteSizeAndAlignment);

ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,9 @@ protected override ModuleFieldLayout CreateValueFromKey(EcmaModule module)
}
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
}

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);
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -803,7 +803,7 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada
return ComputeExplicitFieldLayout(type, numInstanceFields);
}
else
if (type.IsEnum || MarshalUtils.IsBlittableType(type) || IsManagedSequentialType(type))
if (type.IsSequentialLayout && !type.ContainsGCPointers)
{
return ComputeSequentialFieldLayout(type, numInstanceFields);
}
Expand All @@ -814,7 +814,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)
Expand All @@ -827,43 +827,26 @@ protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref Layout
}
}

protected override bool AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(TypeDesc type)
{
return MarshalUtils.IsBlittableType(type) || IsManagedSequentialType(type);
}

public static bool IsManagedSequentialType(TypeDesc type)
{
if (type.IsPointer)
if (type.IsPointer || type.IsFunctionPointer)
{
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 (!type.IsValueType)
{
if (!field.IsStatic && !IsManagedSequentialType(field.FieldType.UnderlyingType))
{
return false;
}
return false;
}

return true;
MetadataType metadataType = (MetadataType)type;

return metadataType.IsSequentialLayout && !metadataType.ContainsGCPointers;
}
}
}
7 changes: 6 additions & 1 deletion src/coreclr/vm/classlayoutinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,15 @@ namespace
}
else
{
#ifdef FEATURE_64BIT_ALIGNMENT
pManagedPlacementInfo->m_alignment = pNestedType.RequiresAlign8() ? 8 : TARGET_POINTER_SIZE;
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
#else
pManagedPlacementInfo->m_alignment = TARGET_POINTER_SIZE;
trylek marked this conversation as resolved.
Show resolved Hide resolved
#endif
}

return !pNestedType.GetMethodTable()->IsManagedSequential();
// Types that have GC Pointer fields (objects or byrefs) are disqualified from ManagedSequential layout.
return pNestedType.GetMethodTable()->ContainsPointers() != FALSE;
}

// No other type permitted for ManagedSequential.
Expand Down
19 changes: 10 additions & 9 deletions src/coreclr/vm/methodtablebuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8567,15 +8567,6 @@ MethodTableBuilder::HandleExplicitLayout(
SetHasOverLayedFields();
}

if (IsBlittable() || IsManagedSequential())
{
// Bug 849333: We shouldn't update "bmtFP->NumInstanceFieldBytes"
// for Blittable/ManagedSequential types. As this will break backward compatiblity
// for the size of types that return true for HasExplicitFieldOffsetLayout()
//
return;
}

FindPointerSeriesExplicit(instanceSliceSize, pFieldLayout);

// Fixup the offset to include parent as current offsets are relative to instance slice
Expand Down Expand Up @@ -8612,6 +8603,16 @@ MethodTableBuilder::HandleExplicitLayout(
numInstanceFieldBytes = S_UINT32(clstotalsize);
}
}
else
{
// align up to the alignment requirements of the members of this value type.
dwInstanceSliceOffset.AlignUp(GetLayoutInfo()->m_ManagedLargestAlignmentRequirementOfAllMembers);
if (dwInstanceSliceOffset.IsOverflow())
{
// addition overflow or cast truncation
BuildMethodTableThrowException(IDS_CLASSLOAD_GENERAL);
}
}
}

// The GC requires that all valuetypes containing orefs be sized to a multiple of TARGET_POINTER_SIZE.
Expand Down