From d19b38b6da8b0416a37e5c386292c0a094a4baea Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Sun, 4 Aug 2024 18:35:48 +0200 Subject: [PATCH 1/9] Emit Align8 flag for thread statics --- .../Compiler/DependencyAnalysis/GCStaticEETypeNode.cs | 11 ++++++++++- .../Compiler/DependencyAnalysis/GCStaticsNode.cs | 3 ++- .../DependencyAnalysis/NativeLayoutVertexNode.cs | 6 ++++-- .../Compiler/DependencyAnalysis/NodeFactory.cs | 10 +++++----- .../Compiler/DependencyAnalysis/ThreadStaticsNode.cs | 3 ++- .../aot/ILCompiler.Compiler/Compiler/ILScanner.cs | 5 +++-- 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticEETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticEETypeNode.cs index 62df975327641..9e2c0d74220f6 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticEETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticEETypeNode.cs @@ -20,11 +20,13 @@ public class GCStaticEETypeNode : DehydratableObjectNode, ISymbolDefinitionNode { private GCPointerMap _gcMap; private TargetDetails _target; + private bool _requiresAlign8; - public GCStaticEETypeNode(TargetDetails target, GCPointerMap gcMap) + public GCStaticEETypeNode(TargetDetails target, GCPointerMap gcMap, bool requiresAlign8) { _gcMap = gcMap; _target = target; + _requiresAlign8 = requiresAlign8; } protected override string GetName(NodeFactory factory) => this.GetMangledName(factory.NameMangler); @@ -83,6 +85,13 @@ protected override ObjectData GetDehydratableData(NodeFactory factory, bool relo if (containsPointers) flags |= (uint)EETypeFlags.HasPointersFlag; + if (_requiresAlign8) + { + // Mark the method table as non-value type that requires 8-byte alignment + flags |= (uint)EETypeFlagsEx.RequiresAlign8Flag; + flags |= (uint)EETypeElementType.Class << (byte)EETypeFlags.ElementTypeShift; + } + dataBuilder.EmitUInt(flags); totalSize = Math.Max(totalSize, _target.PointerSize * 3); // minimum GC MethodTable size is 3 pointers diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs index 5b07a130fa3a3..f5efcbbfc70cd 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs @@ -47,7 +47,8 @@ public static string GetMangledName(TypeDesc type, NameMangler nameMangler) private ISymbolNode GetGCStaticEETypeNode(NodeFactory factory) { GCPointerMap map = GCPointerMap.FromStaticLayout(_type); - return factory.GCStaticEEType(map); + bool requiresAlign8 = _type.ThreadGcStaticFieldAlignment.AsInt > factory.Target.PointerSize; + return factory.GCStaticEEType(map, requiresAlign8); } protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs index eca454ffeb712..375d27acabe14 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs @@ -993,7 +993,8 @@ private static TypeDesc GetActualTemplateTypeForType(NodeFactory factory, TypeDe private ISymbolNode GetStaticsNode(NodeFactory context, out BagElementKind staticsBagKind) { MetadataType closestCanonDefType = (MetadataType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); - ISymbolNode symbol = context.GCStaticEEType(GCPointerMap.FromStaticLayout(closestCanonDefType)); + bool requiresAlign8 = closestCanonDefType.ThreadGcStaticFieldAlignment.AsInt > context.Target.PointerSize; + ISymbolNode symbol = context.GCStaticEEType(GCPointerMap.FromStaticLayout(closestCanonDefType), requiresAlign8); staticsBagKind = BagElementKind.GcStaticDesc; return symbol; @@ -1002,7 +1003,8 @@ private ISymbolNode GetStaticsNode(NodeFactory context, out BagElementKind stati private ISymbolNode GetThreadStaticsNode(NodeFactory context, out BagElementKind staticsBagKind) { MetadataType closestCanonDefType = (MetadataType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); - ISymbolNode symbol = context.GCStaticEEType(GCPointerMap.FromThreadStaticLayout(closestCanonDefType)); + bool requiresAlign8 = closestCanonDefType.ThreadGcStaticFieldAlignment.AsInt > context.Target.PointerSize; + ISymbolNode symbol = context.GCStaticEEType(GCPointerMap.FromThreadStaticLayout(closestCanonDefType), requiresAlign8); staticsBagKind = BagElementKind.ThreadStaticDesc; return symbol; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs index 01de1d162cd21..d4895516755b5 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -241,9 +241,9 @@ private void CreateNodeCaches() return new TypeThreadStaticIndexNode(type, null); }); - _GCStaticEETypes = new NodeCache((GCPointerMap gcMap) => + _GCStaticEETypes = new NodeCache<(GCPointerMap, bool), GCStaticEETypeNode>(((GCPointerMap gcMap, bool requiresAlign8) key) => { - return new GCStaticEETypeNode(Target, gcMap); + return new GCStaticEETypeNode(Target, key.gcMap, key.requiresAlign8); }); _readOnlyDataBlobs = new NodeCache(key => @@ -810,11 +810,11 @@ public EmbeddedTrimmingDescriptorNode EmbeddedTrimmingDescriptor(EcmaModule modu return _embeddedTrimmingDescriptors.GetOrAdd(module); } - private NodeCache _GCStaticEETypes; + private NodeCache<(GCPointerMap, bool), GCStaticEETypeNode> _GCStaticEETypes; - public ISymbolNode GCStaticEEType(GCPointerMap gcMap) + public ISymbolNode GCStaticEEType(GCPointerMap gcMap, bool requiredAlign8) { - return _GCStaticEETypes.GetOrAdd(gcMap); + return _GCStaticEETypes.GetOrAdd((gcMap, requiredAlign8)); } private NodeCache _readOnlyDataBlobs; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs index 8935ce8d3d056..9a1a96f045bc2 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs @@ -61,8 +61,9 @@ private ISymbolNode GetGCStaticEETypeNode(NodeFactory factory) _inlined.GetOffsets(), _inlined.GetSize(), factory.Target.PointerSize); + bool requiresAlign8 = _type.ThreadGcStaticFieldAlignment.AsInt > factory.Target.PointerSize; - return factory.GCStaticEEType(map); + return factory.GCStaticEEType(map, requiresAlign8); } public override IEnumerable GetStaticDependencies(NodeFactory factory) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs index 5bf78fb125efc..f2c31155d5f45 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs @@ -850,8 +850,9 @@ public ScannedInlinedThreadStatics(NodeFactory factory, ImmutableArray PointerSize alignments. - // GCStaticEEType does not currently set RequiresAlign8Flag + // N.B. for ARM32, we would need to deal with > PointerSize alignments. We + // currently don't support inlined thread statics on ARM32, regular GCStaticEEType + // handles this with RequiresAlign8Flag Debug.Assert(t.ThreadGcStaticFieldAlignment.AsInt <= factory.Target.PointerSize); nextDataOffset = nextDataOffset.AlignUp(t.ThreadGcStaticFieldAlignment.AsInt); From e105e05afc38240bef430019c474746065169f4a Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Sat, 3 Aug 2024 07:49:00 +0200 Subject: [PATCH 2/9] Port WASM test Co-authored-by: yowl --- .../SmokeTests/UnitTests/BasicThreading.cs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs b/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs index 0b62338d53628..d8c9a9eb62a50 100644 --- a/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs +++ b/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -18,6 +19,9 @@ internal static int Run() ThreadStaticsTestWithTasks.Run(); + if (ThreadStaticAlignmentTest.Run() != Pass) + return Fail; + if (ThreadTest.Run() != Pass) return Fail; @@ -187,6 +191,59 @@ public static void Run() } } +class ThreadStaticAlignmentTest +{ + public static int Run() + { + // Check for 8-byte alignment requirement + if (RuntimeInformation.ProcessArchitecture is Architecture.Arm or Architecture.Wasm) + { + // Assume that these are allocated sequentially, use a padding object of size 12 (mod 8 is not 0) + // to move the alignment of the second AddressOfReturnArea in case the first is coincidentally aligned 8. + var ts1Addr = ThreadStaticAlignCheck1.returnArea.AddressOfReturnArea(); + var p = new Padder(); + var ts2Addr = ThreadStaticAlignCheck2.returnArea.AddressOfReturnArea(); + + if (((nint)ts1Addr) % 8 != 0) + return BasicThreading.Fail; + if (((nint)ts2Addr) % 8 != 0) + return BasicThreading.Fail; + } + + return BasicThreading.Pass; + } + + [InlineArray(3)] + private struct ReturnArea + { + private ulong buffer; + + internal unsafe nint AddressOfReturnArea() + { + return (nint)Unsafe.AsPointer(ref buffer); + } + } + + private class ThreadStaticAlignCheck1 + { + [ThreadStatic] + [FixedAddressValueType] + internal static ReturnArea returnArea = default; + } + + private class Padder + { + private object o1; + } + + private class ThreadStaticAlignCheck2 + { + [ThreadStatic] + [FixedAddressValueType] + internal static ReturnArea returnArea = default; + } +} + class ThreadTest { private static readonly List s_startedThreads = new List(); From 242726263e066324bb0c0bfc9f5c8243e618db4c Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Mon, 5 Aug 2024 10:22:01 +0200 Subject: [PATCH 3/9] Fix NRE for inlined thread statics --- .../Compiler/DependencyAnalysis/ThreadStaticsNode.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs index 9a1a96f045bc2..f024dd24e1345 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs @@ -61,7 +61,7 @@ private ISymbolNode GetGCStaticEETypeNode(NodeFactory factory) _inlined.GetOffsets(), _inlined.GetSize(), factory.Target.PointerSize); - bool requiresAlign8 = _type.ThreadGcStaticFieldAlignment.AsInt > factory.Target.PointerSize; + bool requiresAlign8 = _type is not null && _type.ThreadGcStaticFieldAlignment.AsInt > factory.Target.PointerSize; return factory.GCStaticEEType(map, requiresAlign8); } From 3e0c8d06da3b3b8a8a5b39708440356b58fc8cb5 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Wed, 7 Aug 2024 09:44:53 +0200 Subject: [PATCH 4/9] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michal Strehovský --- .../Compiler/DependencyAnalysis/GCStaticsNode.cs | 2 +- .../Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs index f5efcbbfc70cd..1e1a5bda25ce7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs @@ -47,7 +47,7 @@ public static string GetMangledName(TypeDesc type, NameMangler nameMangler) private ISymbolNode GetGCStaticEETypeNode(NodeFactory factory) { GCPointerMap map = GCPointerMap.FromStaticLayout(_type); - bool requiresAlign8 = _type.ThreadGcStaticFieldAlignment.AsInt > factory.Target.PointerSize; + bool requiresAlign8 = _type.GcStaticFieldAlignment.AsInt > factory.Target.PointerSize; return factory.GCStaticEEType(map, requiresAlign8); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs index 375d27acabe14..d0996cca6ef8e 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs @@ -993,7 +993,7 @@ private static TypeDesc GetActualTemplateTypeForType(NodeFactory factory, TypeDe private ISymbolNode GetStaticsNode(NodeFactory context, out BagElementKind staticsBagKind) { MetadataType closestCanonDefType = (MetadataType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); - bool requiresAlign8 = closestCanonDefType.ThreadGcStaticFieldAlignment.AsInt > context.Target.PointerSize; + bool requiresAlign8 = closestCanonDefType.GcStaticFieldAlignment.AsInt > context.Target.PointerSize; ISymbolNode symbol = context.GCStaticEEType(GCPointerMap.FromStaticLayout(closestCanonDefType), requiresAlign8); staticsBagKind = BagElementKind.GcStaticDesc; From af387a57e61a319377000eb6fe9d21af6db1e82e Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Wed, 7 Aug 2024 10:16:30 +0200 Subject: [PATCH 5/9] Fix build --- .../Compiler/DependencyAnalysis/GCStaticsNode.cs | 2 +- .../Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs index 1e1a5bda25ce7..bf645650170c3 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs @@ -47,7 +47,7 @@ public static string GetMangledName(TypeDesc type, NameMangler nameMangler) private ISymbolNode GetGCStaticEETypeNode(NodeFactory factory) { GCPointerMap map = GCPointerMap.FromStaticLayout(_type); - bool requiresAlign8 = _type.GcStaticFieldAlignment.AsInt > factory.Target.PointerSize; + bool requiresAlign8 = _type.GCStaticFieldAlignment.AsInt > factory.Target.PointerSize; return factory.GCStaticEEType(map, requiresAlign8); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs index d0996cca6ef8e..cec7762015272 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs @@ -993,7 +993,7 @@ private static TypeDesc GetActualTemplateTypeForType(NodeFactory factory, TypeDe private ISymbolNode GetStaticsNode(NodeFactory context, out BagElementKind staticsBagKind) { MetadataType closestCanonDefType = (MetadataType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); - bool requiresAlign8 = closestCanonDefType.GcStaticFieldAlignment.AsInt > context.Target.PointerSize; + bool requiresAlign8 = closestCanonDefType.GCStaticFieldAlignment.AsInt > context.Target.PointerSize; ISymbolNode symbol = context.GCStaticEEType(GCPointerMap.FromStaticLayout(closestCanonDefType), requiresAlign8); staticsBagKind = BagElementKind.GcStaticDesc; From d5a1e71fa6e6b94fb1650e8673e5f7869e081b10 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Wed, 7 Aug 2024 10:22:13 +0200 Subject: [PATCH 6/9] Clear the requiredAlign8 flag on non-ARM/non-WASM --- .../Compiler/DependencyAnalysis/NodeFactory.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs index d4895516755b5..28c67d9266814 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -814,6 +814,11 @@ public EmbeddedTrimmingDescriptorNode EmbeddedTrimmingDescriptor(EcmaModule modu public ISymbolNode GCStaticEEType(GCPointerMap gcMap, bool requiredAlign8) { + // Ignore requiredAlign8 flag for anything but ARM32 and WASM + if (Target.Architecture is not TargetArchitecture.ARM and not TargetArchitecture.Wasm32) + { + requiredAlign8 = false; + } return _GCStaticEETypes.GetOrAdd((gcMap, requiredAlign8)); } From 9a44da301ccf7ecd86ed1aaa94ab3eeacd76275a Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 8 Aug 2024 08:57:27 +0200 Subject: [PATCH 7/9] Add SupportsAlign8 helper property on TargetDetails --- .../tools/Common/TypeSystem/Common/TargetDetails.cs | 12 ++++++++++++ .../Common/TypeSystem/Common/TypeSystemHelpers.cs | 2 +- .../Compiler/DependencyAnalysis/NodeFactory.cs | 6 +----- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs b/src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs index 42ccc8994e64a..bb7929b88affe 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs @@ -348,5 +348,17 @@ public int MaxHomogeneousAggregateElementCount /// CodeDelta - encapsulate the fact that ARM requires a thumb bit /// public int CodeDelta { get => (Architecture == TargetArchitecture.ARM) ? 1 : 0; } + + /// + /// Encapsulates the fact that some architectures require 8-byte (larger than pointer + /// size) alignment on some value types and arrays. + /// + public bool SupportsAlign8 + { + get + { + return Architecture is TargetArchitecture.ARM or TargetArchitecture.Wasm32; + } + } } } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs index 749136b698b20..27a0ce99541da 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs @@ -415,7 +415,7 @@ public static bool RequiresSlotUnification(this MethodDesc method) /// public static bool RequiresAlign8(this TypeDesc type) { - if (type.Context.Target.Architecture != TargetArchitecture.ARM && type.Context.Target.Architecture != TargetArchitecture.Wasm32) + if (!type.Context.Target.SupportsAlign8) { return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs index 28c67d9266814..aa73c735ae70a 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -814,11 +814,7 @@ public EmbeddedTrimmingDescriptorNode EmbeddedTrimmingDescriptor(EcmaModule modu public ISymbolNode GCStaticEEType(GCPointerMap gcMap, bool requiredAlign8) { - // Ignore requiredAlign8 flag for anything but ARM32 and WASM - if (Target.Architecture is not TargetArchitecture.ARM and not TargetArchitecture.Wasm32) - { - requiredAlign8 = false; - } + requiredAlign8 &= Target.SupportsAlign8; return _GCStaticEETypes.GetOrAdd((gcMap, requiredAlign8)); } From 789e2a21962aa6ea8305ae10527e8d0c628c63d8 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 8 Aug 2024 19:49:13 +0200 Subject: [PATCH 8/9] Add test with generic method --- .../SmokeTests/UnitTests/BasicThreading.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs b/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs index d8c9a9eb62a50..926e5d4221eae 100644 --- a/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs +++ b/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs @@ -208,11 +208,30 @@ public static int Run() return BasicThreading.Fail; if (((nint)ts2Addr) % 8 != 0) return BasicThreading.Fail; + + typeof(ThreadStaticAlignmentTest).GetMethod("RunGeneric").MakeGenericMethod(GetAtom()).Invoke(null, []); } return BasicThreading.Pass; } + public static void RunGeneric() + { + // Assume that these are allocated sequentially, use a padding object of size 12 (mod 8 is not 0) + // to move the alignment of the second AddressOfReturnArea in case the first is coincidentally aligned 8. + var ts1Addr = ThreadStaticAlignCheck1.returnArea.AddressOfReturnArea(); + var p = new Padder(); + var ts2Addr = ThreadStaticAlignCheck2.returnArea.AddressOfReturnArea(); + + if (((nint)ts1Addr) % 8 != 0) + throw new Exception(); + if (((nint)ts2Addr) % 8 != 0) + throw new Exception(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Type GetAtom() => typeof(Atom); + [InlineArray(3)] private struct ReturnArea { @@ -242,6 +261,22 @@ private class ThreadStaticAlignCheck2 [FixedAddressValueType] internal static ReturnArea returnArea = default; } + + private class ThreadStaticAlignCheck1 + { + [ThreadStatic] + [FixedAddressValueType] + internal static ReturnArea returnArea = default; + } + + private class ThreadStaticAlignCheck2 + { + [ThreadStatic] + [FixedAddressValueType] + internal static ReturnArea returnArea = default; + } + + private class Atom { } } class ThreadTest From 47d809182d1782232a0196d7105d6bf67c25ba1c Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 8 Aug 2024 19:51:27 +0200 Subject: [PATCH 9/9] Cleanup --- .../SmokeTests/UnitTests/BasicThreading.cs | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs b/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs index 926e5d4221eae..2c826e37e1291 100644 --- a/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs +++ b/src/tests/nativeaot/SmokeTests/UnitTests/BasicThreading.cs @@ -209,24 +209,26 @@ public static int Run() if (((nint)ts2Addr) % 8 != 0) return BasicThreading.Fail; - typeof(ThreadStaticAlignmentTest).GetMethod("RunGeneric").MakeGenericMethod(GetAtom()).Invoke(null, []); + return (int)typeof(ThreadStaticAlignmentTest).GetMethod("RunGeneric").MakeGenericMethod(GetAtom()).Invoke(null, []); } return BasicThreading.Pass; } - public static void RunGeneric() + public static int RunGeneric() { - // Assume that these are allocated sequentially, use a padding object of size 12 (mod 8 is not 0) - // to move the alignment of the second AddressOfReturnArea in case the first is coincidentally aligned 8. - var ts1Addr = ThreadStaticAlignCheck1.returnArea.AddressOfReturnArea(); - var p = new Padder(); - var ts2Addr = ThreadStaticAlignCheck2.returnArea.AddressOfReturnArea(); + // Assume that these are allocated sequentially, use a padding object of size 12 (mod 8 is not 0) + // to move the alignment of the second AddressOfReturnArea in case the first is coincidentally aligned 8. + var ts1Addr = ThreadStaticAlignCheck1.returnArea.AddressOfReturnArea(); + var p = new Padder(); + var ts2Addr = ThreadStaticAlignCheck2.returnArea.AddressOfReturnArea(); - if (((nint)ts1Addr) % 8 != 0) - throw new Exception(); - if (((nint)ts2Addr) % 8 != 0) - throw new Exception(); + if (((nint)ts1Addr) % 8 != 0) + return BasicThreading.Fail; + if (((nint)ts2Addr) % 8 != 0) + return BasicThreading.Fail; + + return BasicThreading.Pass; } [MethodImpl(MethodImplOptions.NoInlining)]