From d7cc48ca529c88cc7af7e27fb29ff237a81df694 Mon Sep 17 00:00:00 2001 From: Tomas Date: Mon, 29 Mar 2021 15:54:20 +0200 Subject: [PATCH 01/15] Fix for field layout verification across version bubble boundary When verifying field offset consistency, we shouldn't be checking base class size when the base class doesn't belong to the current version bubble. I have fixed this by adding a special case for the existing fixup encoding indicated by base class size being zero; please let me know if you find it preferable to create a new fixup type for this purpose instead. I have also created a simple regression test that was previously failing when run against the framework compiled with CG2 assembly by assembly. Thanks Tomas --- .../ReadyToRun/FieldFixupSignature.cs | 18 +++++-- src/coreclr/vm/jitinterface.cpp | 8 ++++ .../coreclr/GitHub_49982/test49982.cs | 48 +++++++++++++++++++ .../coreclr/GitHub_49982/test49982.csproj | 19 ++++++++ 4 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 src/tests/Regressions/coreclr/GitHub_49982/test49982.cs create mode 100644 src/tests/Regressions/coreclr/GitHub_49982/test49982.csproj diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs index 62b5f5ce8c18ed..3f7e2a143948d2 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs @@ -41,22 +41,30 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) EcmaModule targetModule = factory.SignatureContext.GetTargetModule(_fieldDesc); SignatureContext innerContext = dataBuilder.EmitFixup(factory, _fixupKind, targetModule, factory.SignatureContext); + uint baseOffset = 0; + uint fieldOffset = (uint)_fieldDesc.Offset.AsInt; if (_fixupKind == ReadyToRunFixupKind.Verify_FieldOffset) { TypeDesc baseType = _fieldDesc.OwningType.BaseType; - if ((_fieldDesc.OwningType.BaseType != null) && !_fieldDesc.IsStatic && !_fieldDesc.OwningType.IsValueType) + if ((_fieldDesc.OwningType.BaseType != null) + && !_fieldDesc.IsStatic + && !_fieldDesc.OwningType.IsValueType) { - dataBuilder.EmitUInt((uint)_fieldDesc.OwningType.FieldBaseOffset().AsInt); + baseOffset = (uint)_fieldDesc.OwningType.FieldBaseOffset().AsInt; + if (!factory.CompilationModuleGroup.VersionsWithType(_fieldDesc.OwningType.BaseType)) + { + fieldOffset -= baseOffset; + baseOffset = 0; + } } - else - dataBuilder.EmitUInt(0); + dataBuilder.EmitUInt(baseOffset); } if ((_fixupKind == ReadyToRunFixupKind.Check_FieldOffset) || (_fixupKind == ReadyToRunFixupKind.Verify_FieldOffset)) { - dataBuilder.EmitUInt((uint)_fieldDesc.Offset.AsInt); + dataBuilder.EmitUInt(fieldOffset); } dataBuilder.EmitFieldSignature(_fieldDesc, innerContext); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 2a3b251ec5621e..2423c4887d61e5 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -14107,6 +14107,14 @@ BOOL LoadDynamicInfoEntry(Module *currentModule, actualBaseOffset = ReadyToRunInfo::GetFieldBaseOffset(pEnclosingMT); } + if (baseOffset == 0) + { + // Relative verification of just the field offset when the base class + // is outside of the current R2R version bubble + actualFieldOffset -= actualBaseOffset; + actualBaseOffset = 0; + } + if ((fieldOffset != actualFieldOffset) || (baseOffset != actualBaseOffset)) { // Verification failures are failfast events diff --git a/src/tests/Regressions/coreclr/GitHub_49982/test49982.cs b/src/tests/Regressions/coreclr/GitHub_49982/test49982.cs new file mode 100644 index 00000000000000..ec3ae4343df4ce --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_49982/test49982.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.ComponentModel; +using System.Collections.Generic; +using System.Net; +using System.Net.Sockets; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +class Program +{ + private class MockEndPoint : EndPoint + { + } + + private sealed class ExtendedSocketException : SocketException + { + private readonly EndPoint? _endPoint; + + public ExtendedSocketException(EndPoint? endPoint) + : base(0) + { + _endPoint = endPoint; + } + + public bool EndPointEquals(EndPoint? endPoint) + { + return _endPoint == endPoint; + } + } + + static bool TestExtendedSocketException() + { + EndPoint endPoint = new MockEndPoint(); + ExtendedSocketException extendedSocketException = new ExtendedSocketException(endPoint); + Console.WriteLine("ExtendedSocketException: {0}", extendedSocketException.GetType()); + return extendedSocketException.EndPointEquals(endPoint); + } + + static int Main() + { + Console.WriteLine("Extended socket exception:"); + return TestExtendedSocketException() ? 100 : 1; + } +} diff --git a/src/tests/Regressions/coreclr/GitHub_49982/test49982.csproj b/src/tests/Regressions/coreclr/GitHub_49982/test49982.csproj new file mode 100644 index 00000000000000..27de21752249f1 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_49982/test49982.csproj @@ -0,0 +1,19 @@ + + + Exe + BuildAndRun + 1 + enable + 9.0 + true + + + true + + + + + + + + From ff09d66f31afb43d190bf6d861c486112d708fbd Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 30 Mar 2021 01:41:41 +0200 Subject: [PATCH 02/15] Fix base type alignment calculation on ARM --- .../Common/MetadataFieldLayoutAlgorithm.cs | 24 +++++-------------- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 2 +- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 1f57b5f1a8c4b2..158f43389f4a5a 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -396,12 +396,12 @@ private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, Layo } } - protected static ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields) + protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields) { var offsets = new FieldAndOffset[numInstanceFields]; // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = ComputeBytesUsedInParentType(type); + LayoutInt cumulativeInstanceFieldPos = CalculateFieldOffsetForType(type, requiresAlign8: false); var layoutMetadata = type.GetClassLayout(); @@ -451,8 +451,6 @@ protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutI protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, int numInstanceFields) { - // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = ComputeBytesUsedInParentType(type); TypeSystemContext context = type.Context; var layoutMetadata = type.GetClassLayout(); @@ -552,19 +550,8 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, largestAlignmentRequired = context.Target.GetObjectAlignment(largestAlignmentRequired); bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; - if (!type.IsValueType) - { - DefType baseType = type.BaseType; - if (baseType != null && !baseType.IsObject) - { - if (!requiresAlign8 && baseType.RequiresAlign8()) - { - requiresAlign8 = true; - } - - AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8); - } - } + // For types inheriting from another type, field offsets continue on from where they left off + LayoutInt cumulativeInstanceFieldPos = CalculateFieldOffsetForType(type, requiresAlign8); // We've finished placing the fields into their appropriate arrays // The next optimization may place non-GC Pointers, so repurpose our @@ -775,13 +762,14 @@ private static bool IsByValueClass(TypeDesc type) return type.IsValueType && !type.IsPrimitive && !type.IsEnum; } - private static LayoutInt ComputeBytesUsedInParentType(DefType type) + private LayoutInt CalculateFieldOffsetForType(DefType type, bool requiresAlign8) { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; if (!type.IsValueType && type.HasBaseType) { cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned; + AlignBaseOffsetIfNecessary((MetadataType)type, ref cumulativeInstanceFieldPos, requiresAlign8 || type.BaseType.RequiresAlign8()); } return cumulativeInstanceFieldPos; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index be653fdcab1cdf..f1c1c17ae43529 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -835,7 +835,7 @@ protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref Layout LayoutInt alignment = new LayoutInt(type.Context.Target.PointerSize); - if (requiresAlign8) + if (requiresAlign8 || type.Context.Target.Architecture == TargetArchitecture.ARM) { alignment = new LayoutInt(8); } From 66d46082c428b8c721a38d55df3db86942e5e606 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 30 Mar 2021 20:51:49 +0200 Subject: [PATCH 03/15] Unify calculation of field base offset in Crossgen2 --- .../Common/MetadataFieldLayoutAlgorithm.cs | 10 +++++----- .../Compiler/ReadyToRunCompilerContext.cs | 2 ++ .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 17 +++-------------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 158f43389f4a5a..c4288cf7f2165d 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -401,7 +401,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType var offsets = new FieldAndOffset[numInstanceFields]; // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = CalculateFieldOffsetForType(type, requiresAlign8: false); + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type); var layoutMetadata = type.GetClassLayout(); @@ -445,7 +445,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType return computedLayout; } - protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) + protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset) { } @@ -551,7 +551,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = CalculateFieldOffsetForType(type, requiresAlign8); + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type); // We've finished placing the fields into their appropriate arrays // The next optimization may place non-GC Pointers, so repurpose our @@ -762,14 +762,14 @@ private static bool IsByValueClass(TypeDesc type) return type.IsValueType && !type.IsPrimitive && !type.IsEnum; } - private LayoutInt CalculateFieldOffsetForType(DefType type, bool requiresAlign8) + public LayoutInt CalculateFieldBaseOffset(TypeDesc type) { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; if (!type.IsValueType && type.HasBaseType) { cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned; - AlignBaseOffsetIfNecessary((MetadataType)type, ref cumulativeInstanceFieldPos, requiresAlign8 || type.BaseType.RequiresAlign8()); + AlignBaseOffsetIfNecessary((MetadataType)type, ref cumulativeInstanceFieldPos); } return cumulativeInstanceFieldPos; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs index 5b8c09852f7ec3..b573c0625a2023 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs @@ -68,6 +68,8 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) } } + public LayoutInt CalculateFieldBaseOffset(TypeDesc type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type); + public void SetCompilationGroup(ReadyToRunCompilationModuleGroupBase compilationModuleGroup) { _r2rFieldLayoutAlgorithm.SetCompilationGroup(compilationModuleGroup); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index f1c1c17ae43529..5379a6df0835f7 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -20,13 +20,7 @@ public static class ReadyToRunTypeExtensions { public static LayoutInt FieldBaseOffset(this TypeDesc type) { - LayoutInt baseOffset = type.BaseType.InstanceByteCount; - if (type.RequiresAlign8()) - { - baseOffset = LayoutInt.AlignUp(baseOffset, new LayoutInt(8), type.Context.Target); - } - - return baseOffset; + return ((ReadyToRunCompilerContext)type.Context).CalculateFieldBaseOffset(type); } } @@ -823,7 +817,7 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada /// This method decides whether the type needs aligned base offset in order to have layout resilient to /// base class layout changes. /// - protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) + protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset) { DefType baseType = type.BaseType; @@ -833,12 +827,7 @@ protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref Layout return; } - LayoutInt alignment = new LayoutInt(type.Context.Target.PointerSize); - - if (requiresAlign8 || type.Context.Target.Architecture == TargetArchitecture.ARM) - { - alignment = new LayoutInt(8); - } + LayoutInt alignment = new LayoutInt(type.Context.Target.Architecture == TargetArchitecture.ARM ? 8 : type.Context.Target.PointerSize); baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target); } From 1334047e48fc5dd25be206576cd92b805abf5011 Mon Sep 17 00:00:00 2001 From: Tomas Date: Wed, 31 Mar 2021 13:09:12 +0200 Subject: [PATCH 04/15] More fixes for base type size vs. field offset base --- .../Common/MetadataFieldLayoutAlgorithm.cs | 42 ++++++++++++++----- .../ReadyToRun/FieldFixupSignature.cs | 2 +- .../Compiler/ReadyToRunCompilerContext.cs | 2 + .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 7 +++- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index c4288cf7f2165d..f1d3f866df2260 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -461,7 +461,6 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, var offsets = new FieldAndOffset[numInstanceFields]; int fieldOrdinal = 0; - // Iterate over the instance fields and keep track of the number of fields of each category // For the non-GC Pointer fields, we will keep track of the number of fields by log2(size) int maxLog2Size = CalculateLog2(TargetDetails.MaximumPrimitiveSize); @@ -551,7 +550,8 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type); + LayoutInt offsetBias = OffsetBias(context); + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type) + offsetBias; // We've finished placing the fields into their appropriate arrays // The next optimization may place non-GC Pointers, so repurpose our @@ -625,7 +625,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, // Place the field j = instanceNonGCPointerFieldsCount[i]; FieldDesc field = instanceNonGCPointerFieldsArr[i][j]; - PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); instanceNonGCPointerFieldsCount[i]++; } @@ -642,14 +642,14 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, { for (int j = 0; j < instanceGCPointerFieldsArr.Length; j++) { - PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); } } // The start index will be the index that may have been increased in the previous optimization for (int j = instanceNonGCPointerFieldsCount[i]; j < instanceNonGCPointerFieldsArr[i].Length; j++) { - PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); } } @@ -672,7 +672,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt AlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, context.Target.LayoutPointerSize); cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, AlignmentRequired, context.Target); } - offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos); + 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 @@ -707,7 +707,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos - offsetBias, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -720,12 +720,12 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, return computedLayout; } - private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal) + private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal, LayoutInt offsetBias) { var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, packingSize, out bool _); instanceFieldPos = AlignUpInstanceFieldOffset(field.OwningType, instanceFieldPos, fieldSizeAndAlignment.Alignment, field.Context.Target); - offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos); + offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos - offsetBias); instanceFieldPos = checked(instanceFieldPos + fieldSizeAndAlignment.Size); fieldOrdinal++; @@ -762,19 +762,41 @@ private static bool IsByValueClass(TypeDesc type) return type.IsValueType && !type.IsPrimitive && !type.IsEnum; } + public LayoutInt CalculateBaseTypeSize(TypeDesc type) + { + LayoutInt baseTypeSize = LayoutInt.Zero; + + if (!type.IsValueType && type.HasBaseType) + { + LayoutInt offsetBias = OffsetBias(type.Context); + baseTypeSize = type.BaseType.InstanceByteCount + offsetBias; + AlignBaseOffsetIfNecessary((MetadataType)type, ref baseTypeSize); + baseTypeSize -= offsetBias; + } + + return baseTypeSize; + } + public LayoutInt CalculateFieldBaseOffset(TypeDesc type) { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; if (!type.IsValueType && type.HasBaseType) { - cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned; + LayoutInt offsetBias = OffsetBias(type.Context); + cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned + offsetBias; AlignBaseOffsetIfNecessary((MetadataType)type, ref cumulativeInstanceFieldPos); + cumulativeInstanceFieldPos -= offsetBias; } return cumulativeInstanceFieldPos; } + public static LayoutInt OffsetBias(TypeSystemContext context) + { + return context.Target.Architecture == TargetArchitecture.ARM ? new LayoutInt(context.Target.PointerSize) : LayoutInt.Zero; + } + private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, int packingSize, out bool layoutAbiStable) { SizeAndAlignment result; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs index 3f7e2a143948d2..620f20a12ef403 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs @@ -51,7 +51,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) && !_fieldDesc.IsStatic && !_fieldDesc.OwningType.IsValueType) { - baseOffset = (uint)_fieldDesc.OwningType.FieldBaseOffset().AsInt; + baseOffset = (uint)_fieldDesc.OwningType.BaseTypeSize().AsInt; if (!factory.CompilationModuleGroup.VersionsWithType(_fieldDesc.OwningType.BaseType)) { fieldOffset -= baseOffset; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs index b573c0625a2023..6e93c7d886839d 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs @@ -68,6 +68,8 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) } } + public LayoutInt CalculateBaseTypeSize(TypeDesc type) => _r2rFieldLayoutAlgorithm.CalculateBaseTypeSize(type); + public LayoutInt CalculateFieldBaseOffset(TypeDesc type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type); public void SetCompilationGroup(ReadyToRunCompilationModuleGroupBase compilationModuleGroup) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index 5379a6df0835f7..ebfd7872c15038 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -18,6 +18,11 @@ namespace ILCompiler { public static class ReadyToRunTypeExtensions { + public static LayoutInt BaseTypeSize(this TypeDesc type) + { + return ((ReadyToRunCompilerContext)type.Context).CalculateBaseTypeSize(type); + } + public static LayoutInt FieldBaseOffset(this TypeDesc type) { return ((ReadyToRunCompilerContext)type.Context).CalculateFieldBaseOffset(type); @@ -820,7 +825,7 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset) { DefType baseType = type.BaseType; - + if (!_compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)baseType, derivedType: type)) { // The type is defined in the module that's currently being compiled and the type layout doesn't depend on other modules From a0de1c2b0b5fdd17ee7ba9b9228cf2d423d64e09 Mon Sep 17 00:00:00 2001 From: Tomas Date: Wed, 31 Mar 2021 23:14:18 +0200 Subject: [PATCH 05/15] Additional ARM-specific field layout verification fixes --- .../Common/MetadataFieldLayoutAlgorithm.cs | 29 +++++-------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index f1d3f866df2260..25bab3c510975f 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -381,19 +381,7 @@ protected static ComputedInstanceFieldLayout ComputeExplicitFieldLayout(Metadata private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, LayoutInt cumulativeInstanceFieldPos, LayoutInt alignment, TargetDetails target, bool armAlignFromStartOfFields = false) { - if (!typeWithField.IsValueType && (target.Architecture == TargetArchitecture.X86 || (armAlignFromStartOfFields && target.Architecture == TargetArchitecture.ARM)) && cumulativeInstanceFieldPos != new LayoutInt(0)) - { - // Alignment of fields is relative to the start of the field list, not the start of the object - // - // The code in the VM is written as if this is the rule for all architectures, but for ARM 32bit platforms - // there is an additional adjustment via dwOffsetBias that aligns fields based on the start of the object - cumulativeInstanceFieldPos = cumulativeInstanceFieldPos - new LayoutInt(target.PointerSize); - return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target) + new LayoutInt(target.PointerSize); - } - else - { - return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target); - } + return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target); } protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields) @@ -421,10 +409,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType largestAlignmentRequirement = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequirement); - cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, type.Context.Target, - armAlignFromStartOfFields: true // In what appears to have been a bug in the design of the arm32 type layout code - // this portion of the layout algorithm does not layout from the start of the object - ); + cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, type.Context.Target); offsets[fieldOrdinal] = new FieldAndOffset(field, cumulativeInstanceFieldPos); cumulativeInstanceFieldPos = checked(cumulativeInstanceFieldPos + fieldSizeAndAlignment.Size); @@ -550,7 +535,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt offsetBias = OffsetBias(context); + LayoutInt offsetBias = OffsetBias(type); LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type) + offsetBias; // We've finished placing the fields into their appropriate arrays @@ -768,7 +753,7 @@ public LayoutInt CalculateBaseTypeSize(TypeDesc type) if (!type.IsValueType && type.HasBaseType) { - LayoutInt offsetBias = OffsetBias(type.Context); + LayoutInt offsetBias = OffsetBias(type); baseTypeSize = type.BaseType.InstanceByteCount + offsetBias; AlignBaseOffsetIfNecessary((MetadataType)type, ref baseTypeSize); baseTypeSize -= offsetBias; @@ -783,7 +768,7 @@ public LayoutInt CalculateFieldBaseOffset(TypeDesc type) if (!type.IsValueType && type.HasBaseType) { - LayoutInt offsetBias = OffsetBias(type.Context); + LayoutInt offsetBias = OffsetBias(type); cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned + offsetBias; AlignBaseOffsetIfNecessary((MetadataType)type, ref cumulativeInstanceFieldPos); cumulativeInstanceFieldPos -= offsetBias; @@ -792,9 +777,9 @@ public LayoutInt CalculateFieldBaseOffset(TypeDesc type) return cumulativeInstanceFieldPos; } - public static LayoutInt OffsetBias(TypeSystemContext context) + public static LayoutInt OffsetBias(TypeDesc type) { - return context.Target.Architecture == TargetArchitecture.ARM ? new LayoutInt(context.Target.PointerSize) : LayoutInt.Zero; + return !type.IsValueType && type.Context.Target.Architecture == TargetArchitecture.ARM ? new LayoutInt(type.Context.Target.PointerSize) : LayoutInt.Zero; } private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, int packingSize, out bool layoutAbiStable) From 95fe35fe3b653d09df046434adba4255077cc333 Mon Sep 17 00:00:00 2001 From: Tomas Date: Thu, 1 Apr 2021 16:46:48 +0200 Subject: [PATCH 06/15] Revert failed attempt at refactoring ARM layout; include MVID fix --- .../Common/MetadataFieldLayoutAlgorithm.cs | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 25bab3c510975f..8755773188bdf7 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -381,7 +381,19 @@ protected static ComputedInstanceFieldLayout ComputeExplicitFieldLayout(Metadata private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, LayoutInt cumulativeInstanceFieldPos, LayoutInt alignment, TargetDetails target, bool armAlignFromStartOfFields = false) { - return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target); + if (!typeWithField.IsValueType && (target.Architecture == TargetArchitecture.X86 || (armAlignFromStartOfFields && target.Architecture == TargetArchitecture.ARM)) && cumulativeInstanceFieldPos != new LayoutInt(0)) + { + // Alignment of fields is relative to the start of the field list, not the start of the object + // + // The code in the VM is written as if this is the rule for all architectures, but for ARM 32bit platforms + // there is an additional adjustment via dwOffsetBias that aligns fields based on the start of the object + cumulativeInstanceFieldPos = cumulativeInstanceFieldPos - new LayoutInt(target.PointerSize); + return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target) + new LayoutInt(target.PointerSize); + } + else + { + return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target); + } } protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields) @@ -409,7 +421,10 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType largestAlignmentRequirement = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequirement); - cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, type.Context.Target); + cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, type.Context.Target, + armAlignFromStartOfFields: true // In what appears to have been a bug in the design of the arm32 type layout code + // this portion of the layout algorithm does not layout from the start of the object + ); offsets[fieldOrdinal] = new FieldAndOffset(field, cumulativeInstanceFieldPos); cumulativeInstanceFieldPos = checked(cumulativeInstanceFieldPos + fieldSizeAndAlignment.Size); @@ -535,8 +550,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt offsetBias = OffsetBias(type); - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type) + offsetBias; + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type); // We've finished placing the fields into their appropriate arrays // The next optimization may place non-GC Pointers, so repurpose our @@ -610,7 +624,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, // Place the field j = instanceNonGCPointerFieldsCount[i]; FieldDesc field = instanceNonGCPointerFieldsArr[i][j]; - PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); + PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); instanceNonGCPointerFieldsCount[i]++; } @@ -627,14 +641,14 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, { for (int j = 0; j < instanceGCPointerFieldsArr.Length; j++) { - PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); + PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); } } // The start index will be the index that may have been increased in the previous optimization for (int j = instanceNonGCPointerFieldsCount[i]; j < instanceNonGCPointerFieldsArr[i].Length; j++) { - PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); + PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); } } @@ -657,7 +671,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt AlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, context.Target.LayoutPointerSize); cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, AlignmentRequired, context.Target); } - offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos - offsetBias); + offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos); // 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 @@ -692,7 +706,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos - offsetBias, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -705,12 +719,12 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, return computedLayout; } - private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal, LayoutInt offsetBias) + private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal) { var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, packingSize, out bool _); instanceFieldPos = AlignUpInstanceFieldOffset(field.OwningType, instanceFieldPos, fieldSizeAndAlignment.Alignment, field.Context.Target); - offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos - offsetBias); + offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos); instanceFieldPos = checked(instanceFieldPos + fieldSizeAndAlignment.Size); fieldOrdinal++; From 970283bdae77ca7cb07f6d615f8490502f15efd3 Mon Sep 17 00:00:00 2001 From: Tomas Date: Thu, 1 Apr 2021 16:52:28 +0200 Subject: [PATCH 07/15] Stricter typing (TypeDesc -> MetadataType) per Michal's PR feedback --- .../TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs | 8 ++++---- .../DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs | 2 +- .../Compiler/ReadyToRunCompilerContext.cs | 4 ++-- .../Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs | 4 ++-- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 8755773188bdf7..1a3411f1c51447 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -761,7 +761,7 @@ private static bool IsByValueClass(TypeDesc type) return type.IsValueType && !type.IsPrimitive && !type.IsEnum; } - public LayoutInt CalculateBaseTypeSize(TypeDesc type) + public LayoutInt CalculateBaseTypeSize(MetadataType type) { LayoutInt baseTypeSize = LayoutInt.Zero; @@ -769,14 +769,14 @@ public LayoutInt CalculateBaseTypeSize(TypeDesc type) { LayoutInt offsetBias = OffsetBias(type); baseTypeSize = type.BaseType.InstanceByteCount + offsetBias; - AlignBaseOffsetIfNecessary((MetadataType)type, ref baseTypeSize); + AlignBaseOffsetIfNecessary(type, ref baseTypeSize); baseTypeSize -= offsetBias; } return baseTypeSize; } - public LayoutInt CalculateFieldBaseOffset(TypeDesc type) + public LayoutInt CalculateFieldBaseOffset(MetadataType type) { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; @@ -784,7 +784,7 @@ public LayoutInt CalculateFieldBaseOffset(TypeDesc type) { LayoutInt offsetBias = OffsetBias(type); cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned + offsetBias; - AlignBaseOffsetIfNecessary((MetadataType)type, ref cumulativeInstanceFieldPos); + AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos); cumulativeInstanceFieldPos -= offsetBias; } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs index 620f20a12ef403..d171dc37edab18 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs @@ -51,7 +51,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) && !_fieldDesc.IsStatic && !_fieldDesc.OwningType.IsValueType) { - baseOffset = (uint)_fieldDesc.OwningType.BaseTypeSize().AsInt; + baseOffset = (uint)((MetadataType)_fieldDesc.OwningType).BaseTypeSize().AsInt; if (!factory.CompilationModuleGroup.VersionsWithType(_fieldDesc.OwningType.BaseType)) { fieldOffset -= baseOffset; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs index 6e93c7d886839d..d331ba118c7675 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs @@ -68,9 +68,9 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) } } - public LayoutInt CalculateBaseTypeSize(TypeDesc type) => _r2rFieldLayoutAlgorithm.CalculateBaseTypeSize(type); + public LayoutInt CalculateBaseTypeSize(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateBaseTypeSize(type); - public LayoutInt CalculateFieldBaseOffset(TypeDesc type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type); + public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type); public void SetCompilationGroup(ReadyToRunCompilationModuleGroupBase compilationModuleGroup) { diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index ebfd7872c15038..638cabe7ab20ed 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -18,12 +18,12 @@ namespace ILCompiler { public static class ReadyToRunTypeExtensions { - public static LayoutInt BaseTypeSize(this TypeDesc type) + public static LayoutInt BaseTypeSize(this MetadataType type) { return ((ReadyToRunCompilerContext)type.Context).CalculateBaseTypeSize(type); } - public static LayoutInt FieldBaseOffset(this TypeDesc type) + public static LayoutInt FieldBaseOffset(this MetadataType type) { return ((ReadyToRunCompilerContext)type.Context).CalculateFieldBaseOffset(type); } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index cd262716689e60..56a3cce130244e 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -2336,7 +2336,7 @@ private void EncodeFieldBaseOffset(FieldDesc field, CORINFO_FIELD_INFO* pResult, } // ENCODE_FIELD_BASE_OFFSET - int fieldBaseOffset = pMT.FieldBaseOffset().AsInt; + int fieldBaseOffset = ((MetadataType)pMT).FieldBaseOffset().AsInt; Debug.Assert(pResult->offset >= (uint)fieldBaseOffset); pResult->offset -= (uint)fieldBaseOffset; pResult->fieldAccessor = CORINFO_FIELD_ACCESSOR.CORINFO_FIELD_INSTANCE_WITH_BASE; From a180e11078790692632f013c3e15b9187aa53e6b Mon Sep 17 00:00:00 2001 From: Tomas Date: Fri, 2 Apr 2021 22:59:16 +0200 Subject: [PATCH 08/15] Fix instance size calculation for sequential / explicit layout --- .../Common/MetadataFieldLayoutAlgorithm.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 1a3411f1c51447..f0b1622f5d7278 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -363,7 +363,7 @@ protected static ComputedInstanceFieldLayout ComputeExplicitFieldLayout(Metadata } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeInstanceSize(type, instanceSize, largestAlignmentRequired, layoutMetadata.Size, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeExplicitInstanceSize(type, instanceSize, largestAlignmentRequired, layoutMetadata.Size, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -432,7 +432,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, largestAlignmentRequirement, layoutMetadata.Size, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeExplicitInstanceSize(type, cumulativeInstanceFieldPos, largestAlignmentRequirement, layoutMetadata.Size, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -845,15 +845,19 @@ private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata lay return layoutMetadata.PackingSize; } - private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, out SizeAndAlignment byteCount) + private static SizeAndAlignment ComputeExplicitInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, out SizeAndAlignment byteCount) { - SizeAndAlignment result; - - // Pad the length of structs to be 1 if they are empty so we have no zero-length structures - if (type.IsValueType && instanceSize == LayoutInt.Zero) + SizeAndAlignment result = ComputeInstanceSize(type, instanceSize, alignment, classLayoutSize, out byteCount); + if (!type.IsValueType && type.HasBaseType && byteCount.Size == type.BaseType.InstanceByteCount) { - instanceSize = LayoutInt.One; + byteCount.Size += LayoutInt.One; } + return result; + } + + private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, out SizeAndAlignment byteCount) + { + SizeAndAlignment result; TargetDetails target = type.Context.Target; From d1967511c537e04094319e0cffd7f4e926a2c031 Mon Sep 17 00:00:00 2001 From: Tomas Date: Sat, 3 Apr 2021 01:25:22 +0200 Subject: [PATCH 09/15] Restore incorrectly removed logic for non-zero struct sizes --- .../TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index f0b1622f5d7278..b3e3f4ff3e5003 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -859,6 +859,12 @@ private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt { SizeAndAlignment result; + // Pad the length of structs to be 1 if they are empty so we have no zero-length structures + if (type.IsValueType && instanceSize == LayoutInt.Zero) + { + instanceSize = LayoutInt.One; + } + TargetDetails target = type.Context.Target; if (classLayoutSize != 0) From cdb4b0c0e780b1eb781ccef0d8cfa8dcd0eacd8f Mon Sep 17 00:00:00 2001 From: Tomas Date: Sat, 3 Apr 2021 20:28:30 +0200 Subject: [PATCH 10/15] Simplify and fix alignment logic on x86 --- .../Common/MetadataFieldLayoutAlgorithm.cs | 64 +++++++++---------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index b3e3f4ff3e5003..31e56bd8f504ed 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -379,29 +379,21 @@ protected static ComputedInstanceFieldLayout ComputeExplicitFieldLayout(Metadata return computedLayout; } - private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, LayoutInt cumulativeInstanceFieldPos, LayoutInt alignment, TargetDetails target, bool armAlignFromStartOfFields = false) + private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, LayoutInt cumulativeInstanceFieldPos, LayoutInt alignment, TargetDetails target) { - if (!typeWithField.IsValueType && (target.Architecture == TargetArchitecture.X86 || (armAlignFromStartOfFields && target.Architecture == TargetArchitecture.ARM)) && cumulativeInstanceFieldPos != new LayoutInt(0)) - { - // Alignment of fields is relative to the start of the field list, not the start of the object - // - // The code in the VM is written as if this is the rule for all architectures, but for ARM 32bit platforms - // there is an additional adjustment via dwOffsetBias that aligns fields based on the start of the object - cumulativeInstanceFieldPos = cumulativeInstanceFieldPos - new LayoutInt(target.PointerSize); - return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target) + new LayoutInt(target.PointerSize); - } - else - { - return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target); - } + return LayoutInt.AlignUp(cumulativeInstanceFieldPos, alignment, target); } protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType type, int numInstanceFields) { var offsets = new FieldAndOffset[numInstanceFields]; + LayoutInt offsetBias = OffsetBias(type); + // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type); + // For reference types, we calculate field alignment as if the address after the method table pointer + // has offset 0 (on 32-bit platforms, this location is guaranteed to be 8-aligned). + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type) - offsetBias; var layoutMetadata = type.GetClassLayout(); @@ -421,18 +413,15 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType largestAlignmentRequirement = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequirement); - cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, type.Context.Target, - armAlignFromStartOfFields: true // In what appears to have been a bug in the design of the arm32 type layout code - // this portion of the layout algorithm does not layout from the start of the object - ); - offsets[fieldOrdinal] = new FieldAndOffset(field, cumulativeInstanceFieldPos); + cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, fieldSizeAndAlignment.Alignment, type.Context.Target); + offsets[fieldOrdinal] = new FieldAndOffset(field, cumulativeInstanceFieldPos + offsetBias); cumulativeInstanceFieldPos = checked(cumulativeInstanceFieldPos + fieldSizeAndAlignment.Size); fieldOrdinal++; } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeExplicitInstanceSize(type, cumulativeInstanceFieldPos, largestAlignmentRequirement, layoutMetadata.Size, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeExplicitInstanceSize(type, cumulativeInstanceFieldPos + offsetBias, largestAlignmentRequirement, layoutMetadata.Size, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -549,8 +538,14 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, largestAlignmentRequired = context.Target.GetObjectAlignment(largestAlignmentRequired); bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; + LayoutInt offsetBias = OffsetBias(type); + // For types inheriting from another type, field offsets continue on from where they left off LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type); + if (!cumulativeInstanceFieldPos.IsIndeterminate) + { + cumulativeInstanceFieldPos -= offsetBias; + } // We've finished placing the fields into their appropriate arrays // The next optimization may place non-GC Pointers, so repurpose our @@ -624,7 +619,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, // Place the field j = instanceNonGCPointerFieldsCount[i]; FieldDesc field = instanceNonGCPointerFieldsArr[i][j]; - PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); instanceNonGCPointerFieldsCount[i]++; } @@ -641,14 +636,14 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, { for (int j = 0; j < instanceGCPointerFieldsArr.Length; j++) { - PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); } } // The start index will be the index that may have been increased in the previous optimization for (int j = instanceNonGCPointerFieldsCount[i]; j < instanceNonGCPointerFieldsArr[i].Length; j++) { - PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); } } @@ -671,7 +666,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt AlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, context.Target.LayoutPointerSize); cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, AlignmentRequired, context.Target); } - offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos); + 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 @@ -706,7 +701,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos + offsetBias, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -719,12 +714,12 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, return computedLayout; } - private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal) + private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal, LayoutInt offsetBias) { var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, packingSize, out bool _); instanceFieldPos = AlignUpInstanceFieldOffset(field.OwningType, instanceFieldPos, fieldSizeAndAlignment.Alignment, field.Context.Target); - offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos); + offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos + offsetBias); instanceFieldPos = checked(instanceFieldPos + fieldSizeAndAlignment.Size); fieldOrdinal++; @@ -780,12 +775,15 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type) { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; - if (!type.IsValueType && type.HasBaseType) + if (!type.IsValueType) { LayoutInt offsetBias = OffsetBias(type); - cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned + offsetBias; - AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos); - cumulativeInstanceFieldPos -= offsetBias; + if (type.HasBaseType) + { + cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned - offsetBias; + AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos); + } + cumulativeInstanceFieldPos += offsetBias; } return cumulativeInstanceFieldPos; @@ -793,7 +791,7 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type) public static LayoutInt OffsetBias(TypeDesc type) { - return !type.IsValueType && type.Context.Target.Architecture == TargetArchitecture.ARM ? new LayoutInt(type.Context.Target.PointerSize) : LayoutInt.Zero; + return !type.IsValueType ? new LayoutInt(type.Context.Target.PointerSize) : LayoutInt.Zero; } private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, int packingSize, out bool layoutAbiStable) From db2146ee5d882753bac904d48ccda9862af1dd12 Mon Sep 17 00:00:00 2001 From: Tomas Date: Sun, 4 Apr 2021 00:00:28 +0200 Subject: [PATCH 11/15] Put back align8 support; cleanup empty class management --- .../Common/MetadataFieldLayoutAlgorithm.cs | 43 +++++-------------- .../ReadyToRun/FieldFixupSignature.cs | 2 +- .../Compiler/ReadyToRunCompilerContext.cs | 4 +- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 9 +--- 4 files changed, 15 insertions(+), 43 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 31e56bd8f504ed..bfb68c2a533f56 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -363,7 +363,7 @@ protected static ComputedInstanceFieldLayout ComputeExplicitFieldLayout(Metadata } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeExplicitInstanceSize(type, instanceSize, largestAlignmentRequired, layoutMetadata.Size, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeInstanceSize(type, instanceSize, largestAlignmentRequired, layoutMetadata.Size, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -393,7 +393,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType // For types inheriting from another type, field offsets continue on from where they left off // For reference types, we calculate field alignment as if the address after the method table pointer // has offset 0 (on 32-bit platforms, this location is guaranteed to be 8-aligned). - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type) - offsetBias; + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false) - offsetBias; var layoutMetadata = type.GetClassLayout(); @@ -421,7 +421,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeExplicitInstanceSize(type, cumulativeInstanceFieldPos + offsetBias, largestAlignmentRequirement, layoutMetadata.Size, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos + offsetBias, largestAlignmentRequirement, layoutMetadata.Size, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -434,7 +434,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType return computedLayout; } - protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset) + protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) { } @@ -541,7 +541,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt offsetBias = OffsetBias(type); // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type); + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8); if (!cumulativeInstanceFieldPos.IsIndeterminate) { cumulativeInstanceFieldPos -= offsetBias; @@ -756,22 +756,7 @@ private static bool IsByValueClass(TypeDesc type) return type.IsValueType && !type.IsPrimitive && !type.IsEnum; } - public LayoutInt CalculateBaseTypeSize(MetadataType type) - { - LayoutInt baseTypeSize = LayoutInt.Zero; - - if (!type.IsValueType && type.HasBaseType) - { - LayoutInt offsetBias = OffsetBias(type); - baseTypeSize = type.BaseType.InstanceByteCount + offsetBias; - AlignBaseOffsetIfNecessary(type, ref baseTypeSize); - baseTypeSize -= offsetBias; - } - - return baseTypeSize; - } - - public LayoutInt CalculateFieldBaseOffset(MetadataType type) + public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8) { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; @@ -781,7 +766,11 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type) if (type.HasBaseType) { cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned - offsetBias; - AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos); + if (!type.BaseType.IsObject && type.BaseType.IsZeroSizedReferenceType) + { + cumulativeInstanceFieldPos += LayoutInt.One; + } + AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8); } cumulativeInstanceFieldPos += offsetBias; } @@ -843,16 +832,6 @@ private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata lay return layoutMetadata.PackingSize; } - private static SizeAndAlignment ComputeExplicitInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, out SizeAndAlignment byteCount) - { - SizeAndAlignment result = ComputeInstanceSize(type, instanceSize, alignment, classLayoutSize, out byteCount); - if (!type.IsValueType && type.HasBaseType && byteCount.Size == type.BaseType.InstanceByteCount) - { - byteCount.Size += LayoutInt.One; - } - return result; - } - private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt instanceSize, LayoutInt alignment, int classLayoutSize, out SizeAndAlignment byteCount) { SizeAndAlignment result; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs index d171dc37edab18..bc394db179335c 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs @@ -51,7 +51,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) && !_fieldDesc.IsStatic && !_fieldDesc.OwningType.IsValueType) { - baseOffset = (uint)((MetadataType)_fieldDesc.OwningType).BaseTypeSize().AsInt; + baseOffset = (uint)((MetadataType)_fieldDesc.OwningType).FieldBaseOffset().AsInt; if (!factory.CompilationModuleGroup.VersionsWithType(_fieldDesc.OwningType.BaseType)) { fieldOffset -= baseOffset; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs index d331ba118c7675..8d0a1fff5c29bc 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs @@ -68,9 +68,7 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) } } - public LayoutInt CalculateBaseTypeSize(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateBaseTypeSize(type); - - public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type); + public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type, type.RequiresAlign8()); public void SetCompilationGroup(ReadyToRunCompilationModuleGroupBase compilationModuleGroup) { diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index 638cabe7ab20ed..c723c0f8525ff6 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -18,11 +18,6 @@ namespace ILCompiler { public static class ReadyToRunTypeExtensions { - public static LayoutInt BaseTypeSize(this MetadataType type) - { - return ((ReadyToRunCompilerContext)type.Context).CalculateBaseTypeSize(type); - } - public static LayoutInt FieldBaseOffset(this MetadataType type) { return ((ReadyToRunCompilerContext)type.Context).CalculateFieldBaseOffset(type); @@ -822,7 +817,7 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada /// This method decides whether the type needs aligned base offset in order to have layout resilient to /// base class layout changes. /// - protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset) + protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) { DefType baseType = type.BaseType; @@ -832,7 +827,7 @@ protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref Layout return; } - LayoutInt alignment = new LayoutInt(type.Context.Target.Architecture == TargetArchitecture.ARM ? 8 : type.Context.Target.PointerSize); + LayoutInt alignment = new LayoutInt(requiresAlign8 ? 8 : type.Context.Target.PointerSize); baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target); } From 2c9bbd40fdd4a61f93e9615c0cc10a64f8f9b775 Mon Sep 17 00:00:00 2001 From: Tomas Date: Sun, 4 Apr 2021 02:51:26 +0200 Subject: [PATCH 12/15] Fix OffsetBias for object; improve handling of indeterminate types --- .../Common/MetadataFieldLayoutAlgorithm.cs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index bfb68c2a533f56..0a0a33464757c7 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -666,7 +666,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt AlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, context.Target.LayoutPointerSize); cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, AlignmentRequired, context.Target); } - offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos + offsetBias); + offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos.IsIndeterminate ? cumulativeInstanceFieldPos : 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 @@ -701,7 +701,9 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos + offsetBias, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeInstanceSize(type, + cumulativeInstanceFieldPos.IsIndeterminate ? cumulativeInstanceFieldPos : cumulativeInstanceFieldPos + offsetBias, + minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -719,7 +721,7 @@ private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAn var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, packingSize, out bool _); instanceFieldPos = AlignUpInstanceFieldOffset(field.OwningType, instanceFieldPos, fieldSizeAndAlignment.Alignment, field.Context.Target); - offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos + offsetBias); + offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos.IsIndeterminate ? instanceFieldPos : instanceFieldPos + offsetBias); instanceFieldPos = checked(instanceFieldPos + fieldSizeAndAlignment.Size); fieldOrdinal++; @@ -760,19 +762,20 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8 { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; - if (!type.IsValueType) + if (!type.IsValueType && type.HasBaseType) { - LayoutInt offsetBias = OffsetBias(type); - if (type.HasBaseType) + cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned; + if (!type.BaseType.InstanceByteCountUnaligned.IsIndeterminate) { + LayoutInt offsetBias = OffsetBias(type); cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned - offsetBias; - if (!type.BaseType.IsObject && type.BaseType.IsZeroSizedReferenceType) + if (type.BaseType.IsZeroSizedReferenceType && ((MetadataType)type.BaseType).IsExplicitLayout) { cumulativeInstanceFieldPos += LayoutInt.One; } AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8); + cumulativeInstanceFieldPos += offsetBias; } - cumulativeInstanceFieldPos += offsetBias; } return cumulativeInstanceFieldPos; @@ -780,7 +783,7 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8 public static LayoutInt OffsetBias(TypeDesc type) { - return !type.IsValueType ? new LayoutInt(type.Context.Target.PointerSize) : LayoutInt.Zero; + return !type.IsValueType && type.HasBaseType ? new LayoutInt(type.Context.Target.PointerSize) : LayoutInt.Zero; } private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, int packingSize, out bool layoutAbiStable) From 8db3511dd6216c204e2073968cc6fa6defe81781 Mon Sep 17 00:00:00 2001 From: Tomas Date: Sun, 4 Apr 2021 17:43:26 +0200 Subject: [PATCH 13/15] More fixes regarding FieldBaseOffset vs. the actual field base offset --- .../Common/MetadataFieldLayoutAlgorithm.cs | 12 +++++++----- .../Compiler/ReadyToRunCompilerContext.cs | 7 ++++++- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 13 ++++--------- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 0a0a33464757c7..9f7e5d51cc2e05 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -393,7 +393,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType // For types inheriting from another type, field offsets continue on from where they left off // For reference types, we calculate field alignment as if the address after the method table pointer // has offset 0 (on 32-bit platforms, this location is guaranteed to be 8-aligned). - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false) - offsetBias; + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false, requiresAlignedBase: false) - offsetBias; var layoutMetadata = type.GetClassLayout(); @@ -434,7 +434,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType return computedLayout; } - protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) + protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8, bool requiresAlignedBase) { } @@ -541,7 +541,9 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt offsetBias = OffsetBias(type); // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8); + // Base alignment is not always required, it's only applied when there's a version bubble boundary + // between base type and the current type. + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8, requiresAlignedBase: false); if (!cumulativeInstanceFieldPos.IsIndeterminate) { cumulativeInstanceFieldPos -= offsetBias; @@ -758,7 +760,7 @@ private static bool IsByValueClass(TypeDesc type) return type.IsValueType && !type.IsPrimitive && !type.IsEnum; } - public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8) + public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8, bool requiresAlignedBase) { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; @@ -773,7 +775,7 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8 { cumulativeInstanceFieldPos += LayoutInt.One; } - AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8); + AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8, requiresAlignedBase); cumulativeInstanceFieldPos += offsetBias; } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs index 8d0a1fff5c29bc..4c8f9770332ca7 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs @@ -68,7 +68,12 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) } } - public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type, type.RequiresAlign8()); + /// + /// This is a rough equivalent of the CoreCLR runtime method ReadyToRunInfo::GetFieldBaseOffset. + /// In contrast to the auto field layout algorithm, this method unconditionally applies alignment + /// between base and derived class (even when they reside in the same version bubble). + /// + public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type, type.RequiresAlign8(), requiresAlignedBase: true); public void SetCompilationGroup(ReadyToRunCompilationModuleGroupBase compilationModuleGroup) { diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index c723c0f8525ff6..4f754f895e35ca 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -817,18 +817,13 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada /// This method decides whether the type needs aligned base offset in order to have layout resilient to /// base class layout changes. /// - protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) + protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8, bool requiresAlignedBase) { - DefType baseType = type.BaseType; - - if (!_compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)baseType, derivedType: type)) + if (requiresAlignedBase || _compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)type.BaseType, derivedType: type)) { - // The type is defined in the module that's currently being compiled and the type layout doesn't depend on other modules - return; + LayoutInt alignment = new LayoutInt(requiresAlign8 ? 8 : type.Context.Target.PointerSize); + baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target); } - - LayoutInt alignment = new LayoutInt(requiresAlign8 ? 8 : type.Context.Target.PointerSize); - baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target); } public static bool IsManagedSequentialType(TypeDesc type) From 943cd4dfc8e81f2752fbc95e157b657843a46cc3 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 6 Apr 2021 00:07:45 +0200 Subject: [PATCH 14/15] More consistency fixes for x86 and arm --- .../Common/MetadataFieldLayoutAlgorithm.cs | 40 ++++++------------- .../CompilationModuleGroup.ReadyToRun.cs | 6 +++ .../ReadyToRun/FieldFixupSignature.cs | 5 ++- .../ReadyToRunCompilationModuleGroupBase.cs | 5 ++- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 2 +- 5 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 9f7e5d51cc2e05..bee59791d60a69 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -308,9 +308,11 @@ protected static ComputedInstanceFieldLayout ComputeExplicitFieldLayout(Metadata { // Instance slice size is the total size of instance not including the base type. // It is calculated as the field whose offset and size add to the greatest value. + LayoutInt offsetBias = !type.IsValueType ? new LayoutInt(type.Context.Target.PointerSize) : LayoutInt.Zero; LayoutInt cumulativeInstanceFieldPos = type.HasBaseType && !type.IsValueType ? type.BaseType.InstanceByteCount : LayoutInt.Zero; LayoutInt instanceSize = cumulativeInstanceFieldPos; + cumulativeInstanceFieldPos -= offsetBias; var layoutMetadata = type.GetClassLayout(); @@ -333,7 +335,7 @@ protected static ComputedInstanceFieldLayout ComputeExplicitFieldLayout(Metadata if (fieldAndOffset.Offset == FieldAndOffset.InvalidOffset) ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadBadFormat, type); - LayoutInt computedOffset = fieldAndOffset.Offset + cumulativeInstanceFieldPos; + LayoutInt computedOffset = fieldAndOffset.Offset + cumulativeInstanceFieldPos + offsetBias; // GC pointers MUST be aligned. // We treat byref-like structs as GC pointers too. @@ -388,11 +390,10 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType { var offsets = new FieldAndOffset[numInstanceFields]; - LayoutInt offsetBias = OffsetBias(type); - // For types inheriting from another type, field offsets continue on from where they left off // For reference types, we calculate field alignment as if the address after the method table pointer // has offset 0 (on 32-bit platforms, this location is guaranteed to be 8-aligned). + LayoutInt offsetBias = !type.IsValueType ? new LayoutInt(type.Context.Target.PointerSize) : LayoutInt.Zero; LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false, requiresAlignedBase: false) - offsetBias; var layoutMetadata = type.GetClassLayout(); @@ -538,16 +539,10 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, largestAlignmentRequired = context.Target.GetObjectAlignment(largestAlignmentRequired); bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; - LayoutInt offsetBias = OffsetBias(type); - // For types inheriting from another type, field offsets continue on from where they left off // Base alignment is not always required, it's only applied when there's a version bubble boundary // between base type and the current type. LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8, requiresAlignedBase: false); - if (!cumulativeInstanceFieldPos.IsIndeterminate) - { - cumulativeInstanceFieldPos -= offsetBias; - } // We've finished placing the fields into their appropriate arrays // The next optimization may place non-GC Pointers, so repurpose our @@ -621,7 +616,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, // Place the field j = instanceNonGCPointerFieldsCount[i]; FieldDesc field = instanceNonGCPointerFieldsArr[i][j]; - PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); + PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); instanceNonGCPointerFieldsCount[i]++; } @@ -638,14 +633,14 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, { for (int j = 0; j < instanceGCPointerFieldsArr.Length; j++) { - PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); + PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); } } // The start index will be the index that may have been increased in the previous optimization for (int j = instanceNonGCPointerFieldsCount[i]; j < instanceNonGCPointerFieldsArr[i].Length; j++) { - PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); + PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); } } @@ -668,7 +663,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt AlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, context.Target.LayoutPointerSize); cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, AlignmentRequired, context.Target); } - offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos.IsIndeterminate ? cumulativeInstanceFieldPos : cumulativeInstanceFieldPos + offsetBias); + offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos); // 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 @@ -703,9 +698,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeInstanceSize(type, - cumulativeInstanceFieldPos.IsIndeterminate ? cumulativeInstanceFieldPos : cumulativeInstanceFieldPos + offsetBias, - minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -718,12 +711,12 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, return computedLayout; } - private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal, LayoutInt offsetBias) + private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal) { var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, packingSize, out bool _); instanceFieldPos = AlignUpInstanceFieldOffset(field.OwningType, instanceFieldPos, fieldSizeAndAlignment.Alignment, field.Context.Target); - offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos.IsIndeterminate ? instanceFieldPos : instanceFieldPos + offsetBias); + offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos); instanceFieldPos = checked(instanceFieldPos + fieldSizeAndAlignment.Size); fieldOrdinal++; @@ -769,25 +762,18 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8 cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned; if (!type.BaseType.InstanceByteCountUnaligned.IsIndeterminate) { - LayoutInt offsetBias = OffsetBias(type); - cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned - offsetBias; - if (type.BaseType.IsZeroSizedReferenceType && ((MetadataType)type.BaseType).IsExplicitLayout) + cumulativeInstanceFieldPos = type.BaseType.InstanceByteCountUnaligned; + if (type.BaseType.IsZeroSizedReferenceType && ((MetadataType)type.BaseType).HasLayout()) { cumulativeInstanceFieldPos += LayoutInt.One; } AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8, requiresAlignedBase); - cumulativeInstanceFieldPos += offsetBias; } } return cumulativeInstanceFieldPos; } - public static LayoutInt OffsetBias(TypeDesc type) - { - return !type.IsValueType && type.HasBaseType ? new LayoutInt(type.Context.Target.PointerSize) : LayoutInt.Zero; - } - private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, int packingSize, out bool layoutAbiStable) { SizeAndAlignment result; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/CompilationModuleGroup.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/CompilationModuleGroup.ReadyToRun.cs index 2312ecc1c4a755..e2ec6059d5dc77 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/CompilationModuleGroup.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/CompilationModuleGroup.ReadyToRun.cs @@ -81,6 +81,12 @@ public bool EnforceOwningType(EcmaModule module) /// public abstract bool IsInputBubble { get; } + /// + /// Returns true when the base type and derived type don't reside in the same version bubble + /// in which case the runtime aligns the field base offset. + /// + public abstract bool NeedsAlignmentBetweenBaseTypeAndDerived(MetadataType baseType, MetadataType derivedType); + /// /// List of input modules to use for the compilation. /// diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs index bc394db179335c..f993b381d8ceae 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs @@ -51,8 +51,9 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) && !_fieldDesc.IsStatic && !_fieldDesc.OwningType.IsValueType) { - baseOffset = (uint)((MetadataType)_fieldDesc.OwningType).FieldBaseOffset().AsInt; - if (!factory.CompilationModuleGroup.VersionsWithType(_fieldDesc.OwningType.BaseType)) + MetadataType owningType = (MetadataType)_fieldDesc.OwningType; + baseOffset = (uint)owningType.FieldBaseOffset().AsInt; + if (factory.CompilationModuleGroup.NeedsAlignmentBetweenBaseTypeAndDerived((MetadataType)baseType, owningType)) { fieldOffset -= baseOffset; baseOffset = 0; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs index 2e0a760e9725a3..92ef5346c770fd 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs @@ -267,8 +267,11 @@ private bool ModuleMatchesCompilationUnitIndex(ModuleDesc module1, ModuleDesc mo return ModuleToCompilationUnitIndex(module1) == ModuleToCompilationUnitIndex(module2); } - public bool NeedsAlignmentBetweenBaseTypeAndDerived(MetadataType baseType, MetadataType derivedType) + public override bool NeedsAlignmentBetweenBaseTypeAndDerived(MetadataType baseType, MetadataType derivedType) { + if (baseType.IsObject) + return false; + if (!ModuleMatchesCompilationUnitIndex(derivedType.Module, baseType.Module) || TypeLayoutCompilationUnits(baseType).HasMultipleCompilationUnits) { diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index 4f754f895e35ca..af4ac9a6e455fb 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -821,7 +821,7 @@ protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref Layout { if (requiresAlignedBase || _compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)type.BaseType, derivedType: type)) { - LayoutInt alignment = new LayoutInt(requiresAlign8 ? 8 : type.Context.Target.PointerSize); + LayoutInt alignment = new LayoutInt(requiresAlign8 || type.BaseType.RequiresAlign8() ? 8 : type.Context.Target.PointerSize); baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target); } } From bf43f05e39ff6de782dced67ac48a6e3cc557147 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 6 Apr 2021 16:59:56 +0200 Subject: [PATCH 15/15] Two more fixes for x86 - don't 8-align base & offset bias fix --- .../Common/MetadataFieldLayoutAlgorithm.cs | 20 ++++++++++++------- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 3 ++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index bee59791d60a69..f7372ba0d6edd9 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -543,6 +543,12 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, // Base alignment is not always required, it's only applied when there's a version bubble boundary // between base type and the current type. LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8, requiresAlignedBase: false); + LayoutInt offsetBias = LayoutInt.Zero; + if (!type.IsValueType && cumulativeInstanceFieldPos != LayoutInt.Zero && type.Context.Target.Architecture == TargetArchitecture.X86) + { + offsetBias = new LayoutInt(type.Context.Target.PointerSize); + cumulativeInstanceFieldPos -= offsetBias; + } // We've finished placing the fields into their appropriate arrays // The next optimization may place non-GC Pointers, so repurpose our @@ -616,7 +622,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, // Place the field j = instanceNonGCPointerFieldsCount[i]; FieldDesc field = instanceNonGCPointerFieldsArr[i][j]; - PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(field, packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); instanceNonGCPointerFieldsCount[i]++; } @@ -633,14 +639,14 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, { for (int j = 0; j < instanceGCPointerFieldsArr.Length; j++) { - PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(instanceGCPointerFieldsArr[j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); } } // The start index will be the index that may have been increased in the previous optimization for (int j = instanceNonGCPointerFieldsCount[i]; j < instanceNonGCPointerFieldsArr[i].Length; j++) { - PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal); + PlaceInstanceField(instanceNonGCPointerFieldsArr[i][j], packingSize, offsets, ref cumulativeInstanceFieldPos, ref fieldOrdinal, offsetBias); } } @@ -663,7 +669,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt AlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, context.Target.LayoutPointerSize); cumulativeInstanceFieldPos = AlignUpInstanceFieldOffset(type, cumulativeInstanceFieldPos, AlignmentRequired, context.Target); } - offsets[fieldOrdinal] = new FieldAndOffset(instanceValueClassFieldsArr[i], cumulativeInstanceFieldPos); + 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 @@ -698,7 +704,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, } SizeAndAlignment instanceByteSizeAndAlignment; - var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); + var instanceSizeAndAlignment = ComputeInstanceSize(type, cumulativeInstanceFieldPos + offsetBias, minAlign, 0/* specified field size unused */, out instanceByteSizeAndAlignment); ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout(); computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; @@ -711,12 +717,12 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, return computedLayout; } - private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal) + private static void PlaceInstanceField(FieldDesc field, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal, LayoutInt offsetBias) { var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, packingSize, out bool _); instanceFieldPos = AlignUpInstanceFieldOffset(field.OwningType, instanceFieldPos, fieldSizeAndAlignment.Alignment, field.Context.Target); - offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos); + offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos + offsetBias); instanceFieldPos = checked(instanceFieldPos + fieldSizeAndAlignment.Size); fieldOrdinal++; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index af4ac9a6e455fb..1cc09466ee216e 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -821,7 +821,8 @@ protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref Layout { if (requiresAlignedBase || _compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)type.BaseType, derivedType: type)) { - LayoutInt alignment = new LayoutInt(requiresAlign8 || type.BaseType.RequiresAlign8() ? 8 : type.Context.Target.PointerSize); + bool use8Align = (requiresAlign8 || type.BaseType.RequiresAlign8()) && type.Context.Target.Architecture != TargetArchitecture.X86; + LayoutInt alignment = new LayoutInt(use8Align ? 8 : type.Context.Target.PointerSize); baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target); } }