Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NativeAOT] Introduce pointer-based CompareExchange intrinsic and use operating with syncblock bits. #106703

Merged
merged 3 commits into from
Aug 20, 2024
Merged

[NativeAOT] Introduce pointer-based CompareExchange intrinsic and use operating with syncblock bits. #106703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28c71ba
introduce CompareExchange(int* location1, int value, int comparand)
VSadov Aug 20, 2024
dfd2c1d
Apply suggestions from code review
VSadov Aug 20, 2024
a8f1559
fix arm32 build
VSadov Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 19 additions & 0 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wonder why some of CompareExchange overloads are AggressiveInlining and some are not? Is that intentional or just does not matter?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is needed for the ones that contain non-trivial amount of code. For the trivial ones, it is probably just copy&paste.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I will keep the new one without AgressiveInlining and existing ones unchanged.
It seems like in perf-interesting scenarios this will not matter either way.

{
#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)
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}

//
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down