From 32a2976da3e14af083f5acb963d6eae12ff1db01 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 14 Oct 2020 17:13:50 -0700 Subject: [PATCH] Arm32 Crossgen2 initial support (#43243) - Fix type layout bugs - Sequential or Explicit layout classes without explicit field offsets on arm32 should align their fields based on the start of the field list of the object - The field base offset used for R2R calculation on Arm32 should respect the RequiresAlign8 flag - Computing true for requiresAlign8 in auto field layout should set the alignment of a class to 8 during auto layout - if a class derives from an type which requires 8 byte alignment, set the derived to require higher alignment - Align the EH info table on 4 byte boundaries - Set the thumb bit on the arm32 personality routine RVA in XData - Enable Crossgen2 smoke test for arm - Adjust architecture specific type layout tests to match CoreCLR behavior - Fix alignment of Export functions within PE file --- .../src/tools/Common/Compiler/TypeExtensions.cs | 27 ----------------- .../Common/MetadataFieldLayoutAlgorithm.cs | 28 ++++++++++++++--- .../Common/TypeSystem/Common/TypeSystemHelpers.cs | 35 ++++++++++++++++++++++ .../ReadyToRun/ExceptionInfoLookupTableNode.cs | 2 +- .../ReadyToRun/FieldFixupSignature.cs | 4 ++- .../ReadyToRun/MethodGCInfoNode.cs | 8 ++++- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 24 +++++++++------ .../JitInterface/CorInfoImpl.ReadyToRun.cs | 5 ++-- .../ObjectWriter/SectionBuilder.cs | 3 ++ .../ArchitectureSpecificFieldLayoutTests.cs | 6 ++-- .../readytorun/crossgen2/crossgen2smoke.csproj | 3 -- 11 files changed, 94 insertions(+), 51 deletions(-) diff --git a/src/coreclr/src/tools/Common/Compiler/TypeExtensions.cs b/src/coreclr/src/tools/Common/Compiler/TypeExtensions.cs index beacedf..efaee58 100644 --- a/src/coreclr/src/tools/Common/Compiler/TypeExtensions.cs +++ b/src/coreclr/src/tools/Common/Compiler/TypeExtensions.cs @@ -213,33 +213,6 @@ namespace ILCompiler return type.IsMdArray || elementType.IsPointer || elementType.IsFunctionPointer; } - /// - /// Determines whether an object of type '' requires 8-byte alignment on - /// 32bit ARM or 32bit Wasm architectures. - /// - public static bool RequiresAlign8(this TypeDesc type) - { - if (type.Context.Target.Architecture != TargetArchitecture.ARM && type.Context.Target.Architecture != TargetArchitecture.Wasm32) - { - return false; - } - - if (type.IsArray) - { - var elementType = ((ArrayType)type).ElementType; - if ((elementType.IsValueType) && ((DefType)elementType).InstanceByteAlignment.AsInt > 4) - { - return true; - } - } - else if (type.IsDefType && ((DefType)type).InstanceByteAlignment.AsInt > 4) - { - return true; - } - - return false; - } - public static TypeDesc MergeTypesToCommonParent(TypeDesc ta, TypeDesc tb) { if (ta == tb) diff --git a/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index df5eee1..f322786 100644 --- a/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/src/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -379,9 +379,9 @@ namespace Internal.TypeSystem return computedLayout; } - private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, LayoutInt cumulativeInstanceFieldPos, LayoutInt alignment, TargetDetails target) + private static LayoutInt AlignUpInstanceFieldOffset(TypeDesc typeWithField, LayoutInt cumulativeInstanceFieldPos, LayoutInt alignment, TargetDetails target, bool armAlignFromStartOfFields = false) { - if (!typeWithField.IsValueType && target.Architecture == TargetArchitecture.X86 && cumulativeInstanceFieldPos != new LayoutInt(0)) + 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 // @@ -421,7 +421,10 @@ namespace Internal.TypeSystem 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); @@ -548,7 +551,20 @@ namespace Internal.TypeSystem largestAlignmentRequired = context.Target.GetObjectAlignment(largestAlignmentRequired); bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; - AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8); + + if (!type.IsValueType) + { + DefType baseType = type.BaseType; + if (baseType != null && !baseType.IsObject) + { + if (!requiresAlign8 && baseType.RequiresAlign8()) + { + requiresAlign8 = true; + } + + AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8); + } + } // We've finished placing the fields into their appropriate arrays // The next optimization may place non-GC Pointers, so repurpose our @@ -691,6 +707,10 @@ namespace Internal.TypeSystem else if (cumulativeInstanceFieldPos.AsInt > context.Target.PointerSize) { minAlign = context.Target.LayoutPointerSize; + if (requiresAlign8 && minAlign.AsInt == 4) + { + minAlign = new LayoutInt(8); + } } else { diff --git a/src/coreclr/src/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs b/src/coreclr/src/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs index cca5a2d..10d541d 100644 --- a/src/coreclr/src/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs +++ b/src/coreclr/src/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs @@ -438,5 +438,40 @@ namespace Internal.TypeSystem return false; } + + /// + /// Determines whether an object of type '' requires 8-byte alignment on + /// 32bit ARM or 32bit Wasm architectures. + /// + public static bool RequiresAlign8(this TypeDesc type) + { + if (type.Context.Target.Architecture != TargetArchitecture.ARM && type.Context.Target.Architecture != TargetArchitecture.Wasm32) + { + return false; + } + + if (type.IsArray) + { + var elementType = ((ArrayType)type).ElementType; + if (elementType.IsValueType) + { + var alignment = ((DefType)elementType).InstanceByteAlignment; + if (!alignment.IsIndeterminate && alignment.AsInt > 4) + { + return true; + } + } + } + else if (type.IsDefType) + { + var alignment = ((DefType)type).InstanceByteAlignment; + if (!alignment.IsIndeterminate && alignment.AsInt > 4) + { + return true; + } + } + + return false; + } } } diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ExceptionInfoLookupTableNode.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ExceptionInfoLookupTableNode.cs index d340b62..4fa4e5e 100644 --- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ExceptionInfoLookupTableNode.cs +++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ExceptionInfoLookupTableNode.cs @@ -48,7 +48,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) { - return new ObjectData(_ehInfoBuilder.ToArray(), Array.Empty(), alignment: 1, definedSymbols: new ISymbolDefinitionNode[] { this }); + return new ObjectData(_ehInfoBuilder.ToArray(), Array.Empty(), alignment: 4, definedSymbols: new ISymbolDefinitionNode[] { this }); } protected override string GetName(NodeFactory context) diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs index eb5431f..01098a7 100644 --- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs +++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs @@ -46,7 +46,9 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun { TypeDesc baseType = _fieldDesc.OwningType.BaseType; if ((_fieldDesc.OwningType.BaseType != null) && !_fieldDesc.IsStatic && !_fieldDesc.OwningType.IsValueType) - dataBuilder.EmitUInt((uint)_fieldDesc.OwningType.BaseType.InstanceByteCount.AsInt); + { + dataBuilder.EmitUInt((uint)_fieldDesc.OwningType.FieldBaseOffset().AsInt); + } else dataBuilder.EmitUInt(0); } diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodGCInfoNode.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodGCInfoNode.cs index 18f7a17..8af0429 100644 --- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodGCInfoNode.cs +++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodGCInfoNode.cs @@ -90,7 +90,13 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun { bool isFilterFunclet = (_methodNode.FrameInfos[frameInfoIndex].Flags & FrameInfoFlags.Filter) != 0; ISymbolNode personalityRoutine = (isFilterFunclet ? factory.FilterFuncletPersonalityRoutine : factory.PersonalityRoutine); - dataBuilder.EmitReloc(personalityRoutine, RelocType.IMAGE_REL_BASED_ADDR32NB); + int codeDelta = 0; + if (targetArch == TargetArchitecture.ARM) + { + // THUMB_CODE + codeDelta = 1; + } + dataBuilder.EmitReloc(personalityRoutine, RelocType.IMAGE_REL_BASED_ADDR32NB, codeDelta); } if (frameInfoIndex == 0 && _methodNode.GCInfo != null) diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index a9695ef..d89c632 100644 --- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -16,6 +16,20 @@ using Internal.CorConstants; namespace ILCompiler { + 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; + } + } + internal class ReadyToRunMetadataFieldLayoutAlgorithm : MetadataFieldLayoutAlgorithm { /// @@ -811,16 +825,8 @@ namespace ILCompiler /// protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) { - if (type.IsValueType) - { - return; - } DefType baseType = type.BaseType; - if (baseType == null || baseType.IsObject) - { - return; - } - + 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 diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index b59a605..3ca5d12 100644 --- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -2076,8 +2076,9 @@ namespace Internal.JitInterface } // ENCODE_FIELD_BASE_OFFSET - Debug.Assert(pResult->offset >= (uint)pMT.BaseType.InstanceByteCount.AsInt); - pResult->offset -= (uint)pMT.BaseType.InstanceByteCount.AsInt; + int fieldBaseOffset = pMT.FieldBaseOffset().AsInt; + Debug.Assert(pResult->offset >= (uint)fieldBaseOffset); + pResult->offset -= (uint)fieldBaseOffset; pResult->fieldAccessor = CORINFO_FIELD_ACCESSOR.CORINFO_FIELD_INSTANCE_WITH_BASE; pResult->fieldLookup = CreateConstLookupToSymbol(_compilation.SymbolNodeFactory.FieldBaseOffset(field.OwningType)); } diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/SectionBuilder.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/SectionBuilder.cs index dd82ab7..6ba8e55 100644 --- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/SectionBuilder.cs +++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/SectionBuilder.cs @@ -756,6 +756,7 @@ namespace ILCompiler.PEWriter // Emit the name pointer table; it should be alphabetically sorted. // Also, we can now fill in the export address table as we've detected its size // in the previous pass. + builder.Align(4); int namePointerTableRVA = sectionLocation.RelativeVirtualAddress + builder.Count; foreach (ExportSymbol symbol in _exportSymbols) { @@ -774,6 +775,7 @@ namespace ILCompiler.PEWriter } // Emit the address table + builder.Align(4); int addressTableRVA = sectionLocation.RelativeVirtualAddress + builder.Count; foreach (int addressTableEntry in addressTable) { @@ -781,6 +783,7 @@ namespace ILCompiler.PEWriter } // Emit the export directory table + builder.Align(4); int exportDirectoryTableRVA = sectionLocation.RelativeVirtualAddress + builder.Count; // +0x00: reserved builder.WriteInt32(0); diff --git a/src/coreclr/src/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ArchitectureSpecificFieldLayoutTests.cs b/src/coreclr/src/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ArchitectureSpecificFieldLayoutTests.cs index bdee2fb..23b18b8 100644 --- a/src/coreclr/src/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ArchitectureSpecificFieldLayoutTests.cs +++ b/src/coreclr/src/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ArchitectureSpecificFieldLayoutTests.cs @@ -55,11 +55,11 @@ namespace TypeSystemTests Assert.Equal(0x4, tX86.InstanceByteAlignment.AsInt); Assert.Equal(0x11, tX64.InstanceByteCountUnaligned.AsInt); - Assert.Equal(0x11, tARM.InstanceByteCountUnaligned.AsInt); + Assert.Equal(0xD, tARM.InstanceByteCountUnaligned.AsInt); Assert.Equal(0xD, tX86.InstanceByteCountUnaligned.AsInt); Assert.Equal(0x18, tX64.InstanceByteCount.AsInt); - Assert.Equal(0x18, tARM.InstanceByteCount.AsInt); + Assert.Equal(0x10, tARM.InstanceByteCount.AsInt); Assert.Equal(0x10, tX86.InstanceByteCount.AsInt); } @@ -75,7 +75,7 @@ namespace TypeSystemTests Assert.Equal(0x4, tX86.InstanceByteAlignment.AsInt); Assert.Equal(0x19, tX64.InstanceByteCountUnaligned.AsInt); - Assert.Equal(0x11, tARM.InstanceByteCountUnaligned.AsInt); + Assert.Equal(0x15, tARM.InstanceByteCountUnaligned.AsInt); Assert.Equal(0x15, tX86.InstanceByteCountUnaligned.AsInt); Assert.Equal(0x20, tX64.InstanceByteCount.AsInt); diff --git a/src/tests/readytorun/crossgen2/crossgen2smoke.csproj b/src/tests/readytorun/crossgen2/crossgen2smoke.csproj index bc5e965..eb44c22 100644 --- a/src/tests/readytorun/crossgen2/crossgen2smoke.csproj +++ b/src/tests/readytorun/crossgen2/crossgen2smoke.csproj @@ -7,9 +7,6 @@ true - - - true true -- 2.7.4