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 29 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
2 changes: 1 addition & 1 deletion src/coreclr/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

//
Expand Down
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 @@ -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
);

Expand Down Expand Up @@ -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.
Expand All @@ -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;

Expand Down Expand Up @@ -355,7 +352,6 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty
instanceSize,
largestAlignmentRequired,
layoutMetadata.Size,
alignUpInstanceByteSize: AlignUpInstanceByteSizeForExplicitFieldLayoutCompatQuirk(type),
out instanceByteSizeAndAlignment);

ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
Expand Down Expand Up @@ -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;

Expand All @@ -417,7 +413,6 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType
cumulativeInstanceFieldPos + offsetBias,
largestAlignmentRequirement,
layoutMetadata.Size,
alignUpInstanceByteSize: true,
out instanceByteSizeAndAlignment);

ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout();
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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++;
}
Expand All @@ -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
{
Expand All @@ -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;
Expand Down Expand Up @@ -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
{
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}
}

Expand Down
15 changes: 0 additions & 15 deletions src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
}
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 @@ -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);
}
Expand All @@ -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)
Expand All @@ -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;
}
}
}
Loading