[NativeAOT] Introduce pointer-based CompareExchange intrinsic and use…

… operating with syncblock bits. (dotnet#106703) * introduce CompareExchange(int* location1, int value, int comparand) * Apply suggestions from code review Co-authored-by: Jan Kotas <[email protected]> * fix arm32 build --------- Co-authored-by: Jan Kotas <[email protected]>
lewing · Aug 20, 2024 · ed68a64 · ed68a64
1 parent de3c892
commit ed68a64
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 6 deletions.
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs
@@ -649,6 +649,10 @@ internal static IntPtr RhGetModuleSection(TypeManagerHandle module, ReadyToRunSe
         [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg32")]
         internal static extern int InterlockedCompareExchange(ref int location1, int value, int comparand);
 
+        [MethodImplAttribute(MethodImplOptions.InternalCall)]
+        [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg32")]
+        internal static extern unsafe int InterlockedCompareExchange(int* location1, int value, int comparand);
+
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg64")]
         internal static extern long InterlockedCompareExchange(ref long location1, long value, long comparand);

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Runtime;
 using System.Runtime.CompilerServices;
@@ -23,6 +24,24 @@ public static int CompareExchange(ref int location1, int value, int comparand)
 #endif
         }
 
+        // This is used internally by NativeAOT runtime in cases where having a managed
+        // ref to the location is unsafe (Ex: it is the syncblock of a pinned object).
+        // The intrinsic expansion for this overload is exactly the same as for the `ref int`
+        // variant and will go on the same path since expansion is triggered by the name and
+        // return type of the method.
+        // The important part is avoiding `ref *location` in the unexpanded scenario, like
+        // in a case when compiling the Debug flavor of the app.
+        [Intrinsic]
+        internal static unsafe int CompareExchange(int* location1, int value, int comparand)
+        {
+#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64
+            return CompareExchange(location1, value, comparand); // Must expand intrinsic
+#else
+            Debug.Assert(location1 != null);
+            return RuntimeImports.InterlockedCompareExchange(location1, value, comparand);
+#endif
+        }
+
         [Intrinsic]
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static long CompareExchange(ref long location1, long value, long comparand)

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs
@@ -152,7 +152,7 @@ private static unsafe int AssignHashCode(object o, int* pHeader)
                 // there is nothing - try set hashcode inline
                 Debug.Assert((oldBits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) == 0);
                 int newBits = BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX | BIT_SBLK_IS_HASHCODE | oldBits | newHash;
-                if (Interlocked.CompareExchange(ref *pHeader, newBits, oldBits) == oldBits)
+                if (Interlocked.CompareExchange(pHeader, newBits, oldBits) == oldBits)
                 {
                     return newHash;
                 }
@@ -247,7 +247,7 @@ public static unsafe void SetSyncEntryIndex(int* pHeader, int syncIndex)
                 newBits = oldBits & ~(BIT_SBLK_IS_HASHCODE | MASK_HASHCODE_INDEX);
                 newBits |= syncIndex | BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX;
             }
-            while (Interlocked.CompareExchange(ref *pHeader, newBits, oldBits) != oldBits);
+            while (Interlocked.CompareExchange(pHeader, newBits, oldBits) != oldBits);
         }
 
         //
@@ -312,7 +312,7 @@ public static unsafe int TryAcquire(object obj, int currentThreadID, bool oneSho
                     // N.B. hashcode, thread ID and sync index are never 0, and hashcode is largest of all
                     if ((oldBits & MASK_HASHCODE_INDEX) == 0)
                     {
-                        if (Interlocked.CompareExchange(ref *pHeader, oldBits | currentThreadID, oldBits) == oldBits)
+                        if (Interlocked.CompareExchange(pHeader, oldBits | currentThreadID, oldBits) == oldBits)
                         {
                             return -1;
                         }
@@ -369,7 +369,7 @@ private static unsafe int TryAcquireUncommon(object obj, int currentThreadID, bo
                         if ((oldBits & MASK_HASHCODE_INDEX) == 0)
                         {
                             int newBits = oldBits | currentThreadID;
-                            if (Interlocked.CompareExchange(ref *pHeader, newBits, oldBits) == oldBits)
+                            if (Interlocked.CompareExchange(pHeader, newBits, oldBits) == oldBits)
                             {
                                 return -1;
                             }
@@ -398,7 +398,7 @@ private static unsafe int TryAcquireUncommon(object obj, int currentThreadID, bo
                             int newBits = oldBits + SBLK_LOCK_RECLEVEL_INC;
                             if ((newBits & SBLK_MASK_LOCK_RECLEVEL) != 0)
                             {
-                                if (Interlocked.CompareExchange(ref *pHeader, newBits, oldBits) == oldBits)
+                                if (Interlocked.CompareExchange(pHeader, newBits, oldBits) == oldBits)
                                 {
                                     return -1;
                                 }
@@ -458,7 +458,7 @@ public static unsafe void Release(object obj)
                             oldBits - SBLK_LOCK_RECLEVEL_INC :
                             oldBits & ~SBLK_MASK_LOCK_THREADID;
 
-                        if (Interlocked.CompareExchange(ref *pHeader, newBits, oldBits) == oldBits)
+                        if (Interlocked.CompareExchange(pHeader, newBits, oldBits) == oldBits)
                         {
                             return;
                         }

diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs
@@ -94,6 +94,10 @@ internal static IntPtr RhGetModuleSection(TypeManagerHandle module, ReadyToRunSe
         [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg32")]
         internal static extern int InterlockedCompareExchange(ref int location1, int value, int comparand);
 
+        [MethodImplAttribute(MethodImplOptions.InternalCall)]
+        [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg32")]
+        internal static extern unsafe int InterlockedCompareExchange(int* location1, int value, int comparand);
+
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg64")]
         internal static extern long InterlockedCompareExchange(ref long location1, long value, long comparand);