From 3b378626656da1a26a47b575420101a2a12dcf4a Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 6 Aug 2024 11:56:14 +0200 Subject: [PATCH 1/5] Move RequiresAlign8 flag from RareFlags into ExtendedFlags --- .../Common/src/Internal/Runtime/MethodTable.cs | 2 +- .../Runtime.Base/src/System/Runtime/RuntimeExports.cs | 6 ++++-- src/coreclr/nativeaot/Runtime/inc/MethodTable.h | 3 +++ .../Common/Internal/Runtime/EETypeBuilderHelpers.cs | 5 +++++ .../Common/Internal/Runtime/MethodTable.Constants.cs | 10 ++++++---- .../Compiler/DependencyAnalysis/EETypeNode.cs | 5 ----- 6 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs index a701d39ead25a..58a189620e67a 100644 --- a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs +++ b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs @@ -732,7 +732,7 @@ internal bool RequiresAlign8 { get { - return (RareFlags & EETypeRareFlags.RequiresAlign8Flag) != 0; + return (Flags & ((uint)EETypeFlags.HasComponentSizeFlag | (uint)EETypeFlagsEx.RequiresAlign8Flag)) == (uint)EETypeFlagsEx.RequiresAlign8Flag; } } diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/RuntimeExports.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/RuntimeExports.cs index a1e7ab60f30da..b7a1863d3727d 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/RuntimeExports.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/RuntimeExports.cs @@ -61,7 +61,8 @@ public static unsafe object RhNewArray(MethodTable* pEEType, int length) Debug.Assert(pEEType->IsArray || pEEType->IsString); #if FEATURE_64BIT_ALIGNMENT - if (pEEType->RequiresAlign8) + MethodTable* pEEElementType = pEEType->RelatedParameterType; + if (pEEElementType->IsValueType && pEEElementType->RequiresAlign8) { return InternalCalls.RhpNewArrayAlign8(pEEType, length); } @@ -381,7 +382,8 @@ internal static unsafe IntPtr RhGetRuntimeHelperForType(MethodTable* pEEType, Ru case RuntimeHelperKind.AllocateArray: #if FEATURE_64BIT_ALIGNMENT - if (pEEType->RequiresAlign8) + MethodTable* pEEElementType = pEEType->RelatedParameterType; + if (pEEElementType->IsValueType && pEEElementType->RequiresAlign8) return (IntPtr)(delegate*)&InternalCalls.RhpNewArrayAlign8; #endif // FEATURE_64BIT_ALIGNMENT diff --git a/src/coreclr/nativeaot/Runtime/inc/MethodTable.h b/src/coreclr/nativeaot/Runtime/inc/MethodTable.h index f33a5d066dc3e..a23b17c406438 100644 --- a/src/coreclr/nativeaot/Runtime/inc/MethodTable.h +++ b/src/coreclr/nativeaot/Runtime/inc/MethodTable.h @@ -161,6 +161,9 @@ class MethodTable // GC depends on this bit, this type has a critical finalizer HasCriticalFinalizerFlag = 0x0002, IsTrackedReferenceWithFinalizerFlag = 0x0004, + + // This type requires 8-byte alignment for its fields on certain platforms (ARM32, WASM) + RequiresAlign8 = 0x1000 }; public: diff --git a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs index 2f6d2c7f04d6f..2e008b9a1debe 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/EETypeBuilderHelpers.cs @@ -128,6 +128,11 @@ mdType.Name is "WeakReference" or "WeakReference`1" && flagsEx |= (ushort)EETypeFlagsEx.IDynamicInterfaceCastableFlag; } + if (type.RequiresAlign8()) + { + flagsEx |= (ushort)EETypeFlagsEx.RequiresAlign8Flag; + } + return flagsEx; } diff --git a/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs b/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs index 1d2834a4e5bee..daf7d35a0f103 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs @@ -84,6 +84,11 @@ internal enum EETypeFlagsEx : ushort /// This type implements IDynamicInterfaceCastable to allow dynamic resolution of interface casts. /// IDynamicInterfaceCastableFlag = 0x0008, + + /// + /// This type requires 8-byte alignment for its fields on certain platforms (ARM32, WASM) + /// + RequiresAlign8 = 0x1000 } internal enum EETypeKind : uint @@ -116,10 +121,7 @@ internal enum EETypeKind : uint [Flags] internal enum EETypeRareFlags : int { - /// - /// This type requires 8-byte alignment for its fields on certain platforms (only ARM currently). - /// - RequiresAlign8Flag = 0x00000001, + // UNUSED = 0x00000001, // UNUSED1 = 0x00000002, diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs index 7c637812029c7..934a69e505c50 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -1352,11 +1352,6 @@ private void ComputeRareFlags() { uint flags = 0; - if (_type.RequiresAlign8()) - { - flags |= (uint)EETypeRareFlags.RequiresAlign8Flag; - } - if (_type.IsByRefLike) { flags |= (uint)EETypeRareFlags.IsByRefLikeFlag; From 1c65433eea35bce586d334133b66a0511facc204 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 6 Aug 2024 12:26:23 +0200 Subject: [PATCH 2/5] Fix cut & paste error when cleaning up the comments --- src/coreclr/nativeaot/Runtime/inc/MethodTable.h | 2 +- .../tools/Common/Internal/Runtime/MethodTable.Constants.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/inc/MethodTable.h b/src/coreclr/nativeaot/Runtime/inc/MethodTable.h index a23b17c406438..ca7f09feb1d81 100644 --- a/src/coreclr/nativeaot/Runtime/inc/MethodTable.h +++ b/src/coreclr/nativeaot/Runtime/inc/MethodTable.h @@ -163,7 +163,7 @@ class MethodTable IsTrackedReferenceWithFinalizerFlag = 0x0004, // This type requires 8-byte alignment for its fields on certain platforms (ARM32, WASM) - RequiresAlign8 = 0x1000 + RequiresAlign8Flag = 0x1000 }; public: diff --git a/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs b/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs index daf7d35a0f103..bfa142befb24b 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/MethodTable.Constants.cs @@ -88,7 +88,7 @@ internal enum EETypeFlagsEx : ushort /// /// This type requires 8-byte alignment for its fields on certain platforms (ARM32, WASM) /// - RequiresAlign8 = 0x1000 + RequiresAlign8Flag = 0x1000 } internal enum EETypeKind : uint From 3cc9ece2f8f5c6dc130fe640c626700ee384f917 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 6 Aug 2024 14:48:48 +0200 Subject: [PATCH 3/5] Optimize the RequiresAlign8 check --- .../nativeaot/Common/src/Internal/Runtime/MethodTable.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs index 58a189620e67a..2a6825c8f8dd9 100644 --- a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs +++ b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs @@ -732,7 +732,14 @@ internal bool RequiresAlign8 { get { - return (Flags & ((uint)EETypeFlags.HasComponentSizeFlag | (uint)EETypeFlagsEx.RequiresAlign8Flag)) == (uint)EETypeFlagsEx.RequiresAlign8Flag; + // Use the same trick as HasComponentSize. HasComponentSizeFlag is sign bit, so if + // it's set then _uFlags are negative. Once we mask the value we get negative value + // for HasComponentSize == true, 0 for HasComponentSize == false && RequiresAlign8 == false + // and positive value for HasComponentSize == false && RequiresAlign8 == true. + // + // return (Flags & ((uint)EETypeFlags.HasComponentSizeFlag | (uint)EETypeFlagsEx.RequiresAlign8Flag)) == (uint)EETypeFlagsEx.RequiresAlign8Flag; + + return (int)(_uFlags & ((uint)EETypeFlags.HasComponentSizeFlag | (uint)EETypeFlagsEx.RequiresAlign8Flag)) > 0; } } From 0cf9dd3c7957319627def73e1e2ff3a77f71bd01 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 6 Aug 2024 20:11:55 +0200 Subject: [PATCH 4/5] Simplify RequiresAlign8 even further, add debug assert --- .../Common/src/Internal/Runtime/MethodTable.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs index 2a6825c8f8dd9..56b9bbc8fcbab 100644 --- a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs +++ b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs @@ -732,14 +732,14 @@ internal bool RequiresAlign8 { get { - // Use the same trick as HasComponentSize. HasComponentSizeFlag is sign bit, so if - // it's set then _uFlags are negative. Once we mask the value we get negative value - // for HasComponentSize == true, 0 for HasComponentSize == false && RequiresAlign8 == false - // and positive value for HasComponentSize == false && RequiresAlign8 == true. - // - // return (Flags & ((uint)EETypeFlags.HasComponentSizeFlag | (uint)EETypeFlagsEx.RequiresAlign8Flag)) == (uint)EETypeFlagsEx.RequiresAlign8Flag; - - return (int)(_uFlags & ((uint)EETypeFlags.HasComponentSizeFlag | (uint)EETypeFlagsEx.RequiresAlign8Flag)) > 0; + // NOTE: Does not work for types with HasComponentSize, ie. arrays and strings. + // Since this is called early through RhNewObject we cannot use regular Debug.Assert + // here to enforce the assumption. +#if DEBUG + if (!HasComponentSize) + Debug.Fail("RequiresAlign8 called for array or string"); +#endif + return (_uFlags & (uint)EETypeFlagsEx.RequiresAlign8Flag) != 0; } } From 57ad0747815f153bd7b920351ad4d4086c0d3d18 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Wed, 7 Aug 2024 04:26:24 +0200 Subject: [PATCH 5/5] Fix flipped condition in assertion --- .../nativeaot/Common/src/Internal/Runtime/MethodTable.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs index 56b9bbc8fcbab..d3f95bb05377a 100644 --- a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs +++ b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs @@ -736,7 +736,7 @@ internal bool RequiresAlign8 // Since this is called early through RhNewObject we cannot use regular Debug.Assert // here to enforce the assumption. #if DEBUG - if (!HasComponentSize) + if (HasComponentSize) Debug.Fail("RequiresAlign8 called for array or string"); #endif return (_uFlags & (uint)EETypeFlagsEx.RequiresAlign8Flag) != 0;