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

diff --git a/src/coreclr/src/tools/Common/Compiler/TypeExtensions.cs b/src/coreclr/src/tools/Common/Compiler/TypeExtensions.cs
index beacedf..efaee58 100644 (file)
--- 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;
         }
 
-        /// <summary>
-        /// Determines whether an object of type '<paramref name="type"/>' requires 8-byte alignment on 
-        /// 32bit ARM or 32bit Wasm architectures.
-        /// </summary>
-        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 (file)
--- 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 (file)
--- 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;
         }
+
+        /// <summary>
+        /// Determines whether an object of type '<paramref name="type"/>' requires 8-byte alignment on 
+        /// 32bit ARM or 32bit Wasm architectures.
+        /// </summary>
+        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 (file)
--- 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<Relocation>(), alignment: 1, definedSymbols: new ISymbolDefinitionNode[] { this });
+            return new ObjectData(_ehInfoBuilder.ToArray(), Array.Empty<Relocation>(), 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 (file)
--- 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 (file)
--- 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 (file)
--- 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
     {
         /// <summary>
@@ -811,16 +825,8 @@ namespace ILCompiler
         /// </summary>
         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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- a/src/tests/readytorun/crossgen2/crossgen2smoke.csproj
+++ b/src/tests/readytorun/crossgen2/crossgen2smoke.csproj
@@ -7,9 +7,6 @@
 
     <!-- ilasm round-trip testing doesn't make sense for this test -->
     <IlasmRoundTripIncompatible>true</IlasmRoundTripIncompatible>
-    
-    <!-- Crossgen2 currently targets x64 and ARM64 only -->
-    <CLRTestTargetUnsupported Condition="'$(TargetArchitecture)' != 'x64' and '$(TargetArchitecture)' != 'arm64' and '$(TargetArchitecture)' != 'x86'">true</CLRTestTargetUnsupported>
 
     <!-- This is an explicit crossgen test -->
     <AlwaysUseCrossGen2>true</AlwaysUseCrossGen2>