From da95abdb5bf702ae42cbb597ca31dcfdcc544ce0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Fri, 22 Mar 2024 03:10:50 +0100 Subject: [PATCH] Convert small atomic fallbacks to managed (#99011) * Convert small atomic fallbacks to managed * Update Interlocked.cs * Fix Mono * Improve CompareExchange performance * Optimize for non-zero padding * Add static tests * Remove AsPointer Co-authored-by: Hamish Arblaster * Fix build errors --------- Co-authored-by: Hamish Arblaster --- .../System/Threading/Interlocked.CoreCLR.cs | 86 ---- src/coreclr/inc/winwrap.h | 9 - src/coreclr/nativeaot/Runtime/EHHelpers.cpp | 4 - .../nativeaot/Runtime/arm/Interlocked.S | 45 --- src/coreclr/nativeaot/Runtime/portable.cpp | 14 - .../src/System/Runtime/RuntimeImports.cs | 8 - .../src/System/Threading/Interlocked.cs | 54 --- .../src/System/Runtime/RuntimeImports.cs | 8 - .../src/System/Threading/Interlocked.cs | 20 - src/coreclr/pal/inc/pal.h | 34 -- src/coreclr/vm/comutilnative.cpp | 32 -- src/coreclr/vm/comutilnative.h | 4 - src/coreclr/vm/ecalllist.h | 6 +- .../src/System/Threading/Interlocked.cs | 169 +++++++- .../src/System/Threading/Interlocked.Mono.cs | 16 - src/mono/mono/metadata/icall-def.h | 4 - src/mono/mono/metadata/threads-types.h | 12 - src/mono/mono/metadata/threads.c | 32 -- src/mono/mono/utils/atomic.h | 91 +---- src/tests/JIT/Intrinsics/Interlocked.cs | 368 +++++++++++------- 20 files changed, 390 insertions(+), 626 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs index e2d0a033b5d0c..93df3bdfed9fb 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs @@ -42,48 +42,6 @@ public static long Decrement(ref long location) => #endregion #region Exchange - /// Sets a 8-bit unsigned integer to a specified value and returns the original value, as an atomic operation. - /// The variable to set to the specified value. - /// The value to which the parameter is set. - /// The original value of . - /// The address of location1 is a null pointer. - [Intrinsic] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static byte Exchange(ref byte location1, byte value) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return Exchange(ref location1, value); // Must expand intrinsic -#else - if (Unsafe.IsNullRef(ref location1)) - ThrowHelper.ThrowNullReferenceException(); - return Exchange8(ref location1, value); -#endif - } - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern byte Exchange8(ref byte location1, byte value); - - /// Sets a 16-bit signed integer to a specified value and returns the original value, as an atomic operation. - /// The variable to set to the specified value. - /// The value to which the parameter is set. - /// The original value of . - /// The address of location1 is a null pointer. - [Intrinsic] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static short Exchange(ref short location1, short value) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return Exchange(ref location1, value); // Must expand intrinsic -#else - if (Unsafe.IsNullRef(ref location1)) - ThrowHelper.ThrowNullReferenceException(); - return Exchange16(ref location1, value); -#endif - } - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern short Exchange16(ref short location1, short value); - /// Sets a 32-bit signed integer to a specified value and returns the original value, as an atomic operation. /// The variable to set to the specified value. /// The value to which the parameter is set. @@ -162,50 +120,6 @@ public static T Exchange([NotNullIfNotNull(nameof(value))] ref T location1, T #endregion #region CompareExchange - /// Compares two 8-bit unsigned integers for equality and, if they are equal, replaces the first value. - /// The destination, whose value is compared with and possibly replaced. - /// The value that replaces the destination value if the comparison results in equality. - /// The value that is compared to the value at . - /// The original value in . - /// The address of is a null pointer. - [Intrinsic] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static byte CompareExchange(ref byte location1, byte value, byte comparand) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return CompareExchange(ref location1, value, comparand); // Must expand intrinsic -#else - if (Unsafe.IsNullRef(ref location1)) - ThrowHelper.ThrowNullReferenceException(); - return CompareExchange8(ref location1, value, comparand); -#endif - } - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern byte CompareExchange8(ref byte location1, byte value, byte comparand); - - /// Compares two 16-bit signed integers for equality and, if they are equal, replaces the first value. - /// The destination, whose value is compared with and possibly replaced. - /// The value that replaces the destination value if the comparison results in equality. - /// The value that is compared to the value at . - /// The original value in . - /// The address of is a null pointer. - [Intrinsic] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static short CompareExchange(ref short location1, short value, short comparand) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return CompareExchange(ref location1, value, comparand); // Must expand intrinsic -#else - if (Unsafe.IsNullRef(ref location1)) - ThrowHelper.ThrowNullReferenceException(); - return CompareExchange16(ref location1, value, comparand); -#endif - } - - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern short CompareExchange16(ref short location1, short value, short comparand); - /// Compares two 32-bit signed integers for equality and, if they are equal, replaces the first value. /// The destination, whose value is compared with and possibly replaced. /// The value that replaces the destination value if the comparison results in equality. diff --git a/src/coreclr/inc/winwrap.h b/src/coreclr/inc/winwrap.h index 4cf0b9655ad1d..6235e4b5a1811 100644 --- a/src/coreclr/inc/winwrap.h +++ b/src/coreclr/inc/winwrap.h @@ -254,13 +254,4 @@ WszCreateProcess( LPPROCESS_INFORMATION lpProcessInformation ); -#ifdef HOST_WINDOWS - -// -// Workaround for https://github.com/microsoft/WindowsAppSDK/issues/4074 -// Windows SDK is missing InterlockedCompareExchange8 definition. -// -#define InterlockedCompareExchange8 _InterlockedCompareExchange8 - -#endif // HOST_WINDOWS #endif // __WIN_WRAP_H__ diff --git a/src/coreclr/nativeaot/Runtime/EHHelpers.cpp b/src/coreclr/nativeaot/Runtime/EHHelpers.cpp index c440aced22dd9..52357a187ddd4 100644 --- a/src/coreclr/nativeaot/Runtime/EHHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/EHHelpers.cpp @@ -329,8 +329,6 @@ EXTERN_C CODE_LOCATION RhpCheckedAssignRefEBPAVLocation; EXTERN_C CODE_LOCATION RhpCheckedLockCmpXchgAVLocation; EXTERN_C CODE_LOCATION RhpCheckedXchgAVLocation; #if !defined(HOST_AMD64) && !defined(HOST_ARM64) -EXTERN_C CODE_LOCATION RhpLockCmpXchg8AVLocation; -EXTERN_C CODE_LOCATION RhpLockCmpXchg16AVLocation; EXTERN_C CODE_LOCATION RhpLockCmpXchg32AVLocation; EXTERN_C CODE_LOCATION RhpLockCmpXchg64AVLocation; #endif @@ -372,8 +370,6 @@ static bool InWriteBarrierHelper(uintptr_t faultingIP) (uintptr_t)&RhpCheckedXchgAVLocation, #if !defined(HOST_AMD64) && !defined(HOST_ARM64) #if !defined(HOST_X86) - (uintptr_t)&RhpLockCmpXchg8AVLocation, - (uintptr_t)&RhpLockCmpXchg16AVLocation, (uintptr_t)&RhpLockCmpXchg32AVLocation, #endif (uintptr_t)&RhpLockCmpXchg64AVLocation, diff --git a/src/coreclr/nativeaot/Runtime/arm/Interlocked.S b/src/coreclr/nativeaot/Runtime/arm/Interlocked.S index 631731c7e3a32..47b8bbeff00e9 100644 --- a/src/coreclr/nativeaot/Runtime/arm/Interlocked.S +++ b/src/coreclr/nativeaot/Runtime/arm/Interlocked.S @@ -7,51 +7,6 @@ #include // generated by the build from AsmOffsets.cpp #include -// WARNING: Code in EHHelpers.cpp makes assumptions about this helper, in particular: -// - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpLockCmpXchg8AVLocation -// - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address -// r0 = destination address -// r1 = value -// r2 = comparand -LEAF_ENTRY RhpLockCmpXchg8, _TEXT - dmb -GLOBAL_LABEL RhpLockCmpXchg8AVLocation -LOCAL_LABEL(CmpXchg8Retry): - ldrexb r3, [r0] - cmp r2, r3 - bne LOCAL_LABEL(CmpXchg8Exit) - strexb r12, r1, [r0] - cmp r12, #0 - bne LOCAL_LABEL(CmpXchg8Retry) -LOCAL_LABEL(CmpXchg8Exit): - mov r0, r3 - dmb - bx lr -LEAF_END RhpLockCmpXchg8, _TEXT - -// WARNING: Code in EHHelpers.cpp makes assumptions about this helper, in particular: -// - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpLockCmpXchg16AVLocation -// - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address -// r0 = destination address -// r1 = value -// r2 = comparand -LEAF_ENTRY RhpLockCmpXchg16, _TEXT - uxth r2, r2 - dmb -GLOBAL_LABEL RhpLockCmpXchg16AVLocation -LOCAL_LABEL(CmpXchg16Retry): - ldrexh r3, [r0] - cmp r2, r3 - bne LOCAL_LABEL(CmpXchg16Exit) - strexh r12, r1, [r0] - cmp r12, #0 - bne LOCAL_LABEL(CmpXchg16Retry) -LOCAL_LABEL(CmpXchg16Exit): - sxth r0, r3 - dmb - bx lr -LEAF_END RhpLockCmpXchg16, _TEXT - // WARNING: Code in EHHelpers.cpp makes assumptions about this helper, in particular: // - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpLockCmpXchg32AVLocation // - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address diff --git a/src/coreclr/nativeaot/Runtime/portable.cpp b/src/coreclr/nativeaot/Runtime/portable.cpp index d797ad14687ba..11922ac274e64 100644 --- a/src/coreclr/nativeaot/Runtime/portable.cpp +++ b/src/coreclr/nativeaot/Runtime/portable.cpp @@ -390,20 +390,6 @@ FCIMPL2(Object *, RhpCheckedXchg, Object ** location, Object * value) } FCIMPLEND -FCIMPL3(uint8_t, RhpLockCmpXchg8, uint8_t * location, uint8_t value, uint8_t comparand) -{ - ASSERT_UNCONDITIONALLY("NYI"); - return 0; -} -FCIMPLEND - -FCIMPL3(int16_t, RhpLockCmpXchg16, int16_t * location, int16_t value, int16_t comparand) -{ - ASSERT_UNCONDITIONALLY("NYI"); - return 0; -} -FCIMPLEND - FCIMPL3(int32_t, RhpLockCmpXchg32, int32_t * location, int32_t value, int32_t comparand) { // @TODO: USE_PORTABLE_HELPERS - Null check 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 index 98604bc58561b..17a0bca07e13b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs @@ -645,14 +645,6 @@ internal static IntPtr RhGetModuleSection(TypeManagerHandle module, ReadyToRunSe // // Interlocked helpers // - [MethodImplAttribute(MethodImplOptions.InternalCall)] - [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg8")] - internal static extern byte InterlockedCompareExchange(ref byte location1, byte value, byte comparand); - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg16")] - internal static extern short InterlockedCompareExchange(ref short location1, short value, short comparand); - [MethodImplAttribute(MethodImplOptions.InternalCall)] [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg32")] internal static extern int InterlockedCompareExchange(ref int location1, int value, int 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 index b4fd68cf7c703..6b351011a00b2 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs @@ -11,26 +11,6 @@ public static partial class Interlocked { #region CompareExchange - [Intrinsic] - public static byte CompareExchange(ref byte location1, byte value, byte comparand) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return CompareExchange(ref location1, value, comparand); -#else - return RuntimeImports.InterlockedCompareExchange(ref location1, value, comparand); -#endif - } - - [Intrinsic] - public static short CompareExchange(ref short location1, short value, short comparand) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return CompareExchange(ref location1, value, comparand); -#else - return RuntimeImports.InterlockedCompareExchange(ref location1, value, comparand); -#endif - } - [Intrinsic] public static int CompareExchange(ref int location1, int value, int comparand) { @@ -70,40 +50,6 @@ public static T CompareExchange(ref T location1, T value, T comparand) where #region Exchange - [Intrinsic] - public static byte Exchange(ref byte location1, byte value) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return Exchange(ref location1, value); -#else - byte oldValue; - - do - { - oldValue = location1; - } while (CompareExchange(ref location1, value, oldValue) != oldValue); - - return oldValue; -#endif - } - - [Intrinsic] - public static short Exchange(ref short location1, short value) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return Exchange(ref location1, value); -#else - short oldValue; - - do - { - oldValue = location1; - } while (CompareExchange(ref location1, value, oldValue) != oldValue); - - return oldValue; -#endif - } - [Intrinsic] public static int Exchange(ref int location1, int value) { diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs index d755bae45859a..4751e40da3b24 100644 --- a/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs +++ b/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs @@ -90,14 +90,6 @@ internal static IntPtr RhGetModuleSection(TypeManagerHandle module, ReadyToRunSe // // Interlocked helpers // - [MethodImplAttribute(MethodImplOptions.InternalCall)] - [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg8")] - internal static extern byte InterlockedCompareExchange(ref byte location1, byte value, byte comparand); - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg16")] - internal static extern short InterlockedCompareExchange(ref short location1, short value, short comparand); - [MethodImplAttribute(MethodImplOptions.InternalCall)] [RuntimeImport(RuntimeLibrary, "RhpLockCmpXchg32")] internal static extern int InterlockedCompareExchange(ref int location1, int value, int comparand); diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/Threading/Interlocked.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/Threading/Interlocked.cs index 12b3bb500d3a3..9ac6aa5110ad8 100644 --- a/src/coreclr/nativeaot/Test.CoreLib/src/System/Threading/Interlocked.cs +++ b/src/coreclr/nativeaot/Test.CoreLib/src/System/Threading/Interlocked.cs @@ -18,26 +18,6 @@ public static IntPtr CompareExchange(ref IntPtr location1, IntPtr value, IntPtr #endif } - [Intrinsic] - public static byte CompareExchange(ref byte location1, byte value, byte comparand) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return CompareExchange(ref location1, value, comparand); -#else - return RuntimeImports.InterlockedCompareExchange(ref location1, value, comparand); -#endif - } - - [Intrinsic] - public static short CompareExchange(ref short location1, short value, short comparand) - { -#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 - return CompareExchange(ref location1, value, comparand); -#else - return RuntimeImports.InterlockedCompareExchange(ref location1, value, comparand); -#endif - } - [Intrinsic] public static int CompareExchange(ref int location1, int value, int comparand) { diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index 4a0a341b2272e..cb60519e727e8 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -3636,20 +3636,6 @@ Return Values The function returns the initial value pointed to by Target. --*/ -Define_InterlockMethod( - CHAR, - InterlockedExchange8(IN OUT CHAR volatile *Target, CHAR Value), - InterlockedExchange8(Target, Value), - __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) -) - -Define_InterlockMethod( - SHORT, - InterlockedExchange16(IN OUT SHORT volatile *Target, SHORT Value), - InterlockedExchange16(Target, Value), - __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) -) - Define_InterlockMethod( LONG, InterlockedExchange(IN OUT LONG volatile *Target, LONG Value), @@ -3708,26 +3694,6 @@ Return Values The return value is the initial value of the destination. --*/ -Define_InterlockMethod( - CHAR, - InterlockedCompareExchange8(IN OUT CHAR volatile *Destination, IN CHAR Exchange, IN CHAR Comperand), - InterlockedCompareExchange8(Destination, Exchange, Comperand), - __sync_val_compare_and_swap( - Destination, /* The pointer to a variable whose value is to be compared with. */ - Comperand, /* The value to be compared */ - Exchange /* The value to be stored */) -) - -Define_InterlockMethod( - SHORT, - InterlockedCompareExchange16(IN OUT SHORT volatile *Destination, IN SHORT Exchange, IN SHORT Comperand), - InterlockedCompareExchange16(Destination, Exchange, Comperand), - __sync_val_compare_and_swap( - Destination, /* The pointer to a variable whose value is to be compared with. */ - Comperand, /* The value to be compared */ - Exchange /* The value to be stored */) -) - Define_InterlockMethod( LONG, InterlockedCompareExchange(IN OUT LONG volatile *Destination, IN LONG Exchange, IN LONG Comperand), diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 6c7e2468d2744..a3c9d0a848cdf 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1463,22 +1463,6 @@ NOINLINE void GCInterface::GarbageCollectModeAny(int generation) #include -FCIMPL2(FC_UINT8_RET,COMInterlocked::Exchange8, UINT8 *location, UINT8 value) -{ - FCALL_CONTRACT; - - return (UINT8)InterlockedExchange8((CHAR *) location, (CHAR)value); -} -FCIMPLEND - -FCIMPL2(FC_INT16_RET,COMInterlocked::Exchange16, INT16 *location, INT16 value) -{ - FCALL_CONTRACT; - - return InterlockedExchange16((SHORT *) location, value); -} -FCIMPLEND - FCIMPL2(INT32,COMInterlocked::Exchange32, INT32 *location, INT32 value) { FCALL_CONTRACT; @@ -1495,22 +1479,6 @@ FCIMPL2_IV(INT64,COMInterlocked::Exchange64, INT64 *location, INT64 value) } FCIMPLEND -FCIMPL3(FC_UINT8_RET, COMInterlocked::CompareExchange8, UINT8* location, UINT8 value, UINT8 comparand) -{ - FCALL_CONTRACT; - - return (UINT8)InterlockedCompareExchange8((CHAR*)location, (CHAR)value, (CHAR)comparand); -} -FCIMPLEND - -FCIMPL3(FC_INT16_RET, COMInterlocked::CompareExchange16, INT16* location, INT16 value, INT16 comparand) -{ - FCALL_CONTRACT; - - return InterlockedCompareExchange16((SHORT*)location, value, comparand); -} -FCIMPLEND - FCIMPL3(INT32, COMInterlocked::CompareExchange32, INT32* location, INT32 value, INT32 comparand) { FCALL_CONTRACT; diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index 0f305e0af9007..3e64207564c84 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -229,12 +229,8 @@ extern "C" uint64_t QCALLTYPE GCInterface_GetGenerationBudget(int generation); class COMInterlocked { public: - static FCDECL2(FC_UINT8_RET, Exchange8, UINT8 *location, UINT8 value); - static FCDECL2(FC_INT16_RET, Exchange16, INT16 *location, INT16 value); static FCDECL2(INT32, Exchange32, INT32 *location, INT32 value); static FCDECL2_IV(INT64, Exchange64, INT64 *location, INT64 value); - static FCDECL3(FC_UINT8_RET, CompareExchange8, UINT8* location, UINT8 value, UINT8 comparand); - static FCDECL3(FC_INT16_RET, CompareExchange16, INT16* location, INT16 value, INT16 comparand); static FCDECL3(INT32, CompareExchange32, INT32* location, INT32 value, INT32 comparand); static FCDECL3_IVV(INT64, CompareExchange64, INT64* location, INT64 value, INT64 comparand); static FCDECL2(LPVOID, ExchangeObject, LPVOID* location, LPVOID value); diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index fac53467d4c60..9d7e8f7125c39 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -234,7 +234,7 @@ FCFuncStart(gCOMFieldHandleNewFuncs) FCFuncElement("GetLoaderAllocator", RuntimeFieldHandle::GetLoaderAllocator) FCFuncElement("IsFastPathSupported", RuntimeFieldHandle::IsFastPathSupported) FCFuncElement("GetInstanceFieldOffset", RuntimeFieldHandle::GetInstanceFieldOffset) - FCFuncElement("GetStaticFieldAddress", RuntimeFieldHandle::GetStaticFieldAddress) + FCFuncElement("GetStaticFieldAddress", RuntimeFieldHandle::GetStaticFieldAddress) FCFuncEnd() FCFuncStart(gCOMModuleHandleFuncs) @@ -428,13 +428,9 @@ FCFuncStart(gInteropMarshalFuncs) FCFuncEnd() FCFuncStart(gInterlockedFuncs) - FCFuncElement("Exchange8", COMInterlocked::Exchange8) - FCFuncElement("Exchange16", COMInterlocked::Exchange16) FCFuncElement("Exchange32", COMInterlocked::Exchange32) FCFuncElement("Exchange64", COMInterlocked::Exchange64) FCFuncElement("ExchangeObject", COMInterlocked::ExchangeObject) - FCFuncElement("CompareExchange8", COMInterlocked::CompareExchange8) - FCFuncElement("CompareExchange16", COMInterlocked::CompareExchange16) FCFuncElement("CompareExchange32", COMInterlocked::CompareExchange32) FCFuncElement("CompareExchange64", COMInterlocked::CompareExchange64) FCFuncElement("CompareExchangeObject", COMInterlocked::CompareExchangeObject) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs index 31e10ce2b11e7..45ebce76f7b0e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs +++ b/src/libraries/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.Runtime.CompilerServices; namespace System.Threading @@ -67,9 +68,85 @@ public static sbyte Exchange(ref sbyte location1, sbyte value) => /// The address of location1 is a null pointer. [Intrinsic] [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static short Exchange(ref short location1, short value) => + (short)Exchange(ref Unsafe.As(ref location1), (ushort)value); + + /// Sets a 8-bit unsigned integer to a specified value and returns the original value, as an atomic operation. + /// The variable to set to the specified value. + /// The value to which the parameter is set. + /// The original value of . + /// The address of location1 is a null pointer. + [Intrinsic] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe byte Exchange(ref byte location1, byte value) + { +#if !MONO && (TARGET_X86 || TARGET_AMD64 || TARGET_ARM64) + return Exchange(ref location1, value); // Must expand intrinsic +#else + // this relies on GC keeping 4B alignment for refs and on subtracting to such alignment being in the same object + nuint offset = Unsafe.OpportunisticMisalignment(ref location1, sizeof(uint)); + ref uint alignedRef = ref Unsafe.As(ref Unsafe.SubtractByteOffset(ref location1, offset)); + int bitOffset = + (int)((BitConverter.IsLittleEndian ? offset : sizeof(uint) - offset - sizeof(byte)) * 8); // to bit offset + Debug.Assert(bitOffset is 0 or 8 or 16 or 24); + uint mask = ~((uint)byte.MaxValue << bitOffset); + uint shiftedValue = (uint)value << bitOffset; + + // this doesn't need to be volatile since CompareExchange will update stale values + uint originalValue = alignedRef; + uint newValue; + do + { + // make sure the ref is still aligned + Debug.Assert(Unsafe.IsOpportunisticallyAligned(ref alignedRef, sizeof(uint))); + newValue = originalValue & mask | shiftedValue; + } while (originalValue != + (originalValue = CompareExchange(ref alignedRef, newValue, originalValue))); + + // verify the GC hasn't broken the ref + Debug.Assert((nuint)Unsafe.ByteOffset(ref Unsafe.As(ref alignedRef), ref location1) == offset); + return (byte)(originalValue >> bitOffset); +#endif + } + + /// Sets a 16-bit signed integer to a specified value and returns the original value, as an atomic operation. + /// The variable to set to the specified value. + /// The value to which the parameter is set. + /// The original value of . + /// The address of location1 is a null pointer. + [Intrinsic] + [MethodImpl(MethodImplOptions.AggressiveInlining)] [CLSCompliant(false)] - public static ushort Exchange(ref ushort location1, ushort value) => - (ushort)Exchange(ref Unsafe.As(ref location1), (short)value); + public static unsafe ushort Exchange(ref ushort location1, ushort value) + { +#if !MONO && (TARGET_X86 || TARGET_AMD64 || TARGET_ARM64) + return Exchange(ref location1, value); // Must expand intrinsic +#else + // this relies on GC keeping 4B alignment for refs and on subtracting to such alignment being in the same object + nuint offset = Unsafe.OpportunisticMisalignment(ref location1, sizeof(uint)); + ref uint alignedRef = ref Unsafe.As(ref Unsafe.SubtractByteOffset(ref location1, offset)); + int bitOffset = + (int)((BitConverter.IsLittleEndian ? offset : sizeof(uint) - offset - sizeof(byte)) * 8); // to bit offset + Debug.Assert(bitOffset is 0 or 16); + uint mask = ~((uint)ushort.MaxValue << bitOffset); + uint shiftedValue = (uint)value << bitOffset; + + // this doesn't need to be volatile since CompareExchange will update stale values + uint originalValue = alignedRef; + uint newValue; + do + { + // make sure the ref is still aligned + Debug.Assert(Unsafe.IsOpportunisticallyAligned(ref alignedRef, sizeof(uint))); + newValue = originalValue & mask | shiftedValue; + } while (originalValue != + (originalValue = CompareExchange(ref alignedRef, newValue, originalValue))); + + // verify the GC hasn't broken the ref + Debug.Assert((nuint)Unsafe.ByteOffset(ref Unsafe.As(ref alignedRef), ref location1) == offset); + return (ushort)(originalValue >> bitOffset); +#endif + } /// Sets a 32-bit unsigned integer to a specified value and returns the original value, as an atomic operation. /// The variable to set to the specified value. @@ -168,9 +245,93 @@ public static sbyte CompareExchange(ref sbyte location1, sbyte value, sbyte comp /// The address of is a null pointer. [Intrinsic] [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static short CompareExchange(ref short location1, short value, short comparand) => + (short)CompareExchange(ref Unsafe.As(ref location1), (ushort)value, (ushort)comparand); + + /// Compares two 8-bit unsigned integers for equality and, if they are equal, replaces the first value. + /// The destination, whose value is compared with and possibly replaced. + /// The value that replaces the destination value if the comparison results in equality. + /// The value that is compared to the value at . + /// The original value in . + /// The address of is a null pointer. + [Intrinsic] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe byte CompareExchange(ref byte location1, byte value, byte comparand) + { +#if !MONO && (TARGET_X86 || TARGET_AMD64 || TARGET_ARM64) + return CompareExchange(ref location1, value, comparand); // Must expand intrinsic +#else + // this relies on GC keeping 4B alignment for refs and on subtracting to such alignment being in the same object + nuint offset = Unsafe.OpportunisticMisalignment(ref location1, sizeof(uint)); + ref uint alignedRef = ref Unsafe.As(ref Unsafe.SubtractByteOffset(ref location1, offset)); + int bitOffset = + (int)((BitConverter.IsLittleEndian ? offset : sizeof(uint) - offset - sizeof(byte)) * 8); // to bit offset + Debug.Assert(bitOffset is 0 or 8 or 16 or 24); + uint mask = ~((uint)byte.MaxValue << bitOffset); + uint shiftedValue = (uint)value << bitOffset; + uint shiftedComparand = (uint)comparand << bitOffset; + + // this doesn't need to be volatile since CompareExchange will update stale values + uint originalValue = alignedRef; + uint fullComparand, newValue; + do + { + // make sure the ref is still aligned + Debug.Assert(Unsafe.IsOpportunisticallyAligned(ref alignedRef, sizeof(uint))); + uint otherMemory = originalValue & mask; + fullComparand = otherMemory | shiftedComparand; + newValue = otherMemory | shiftedValue; + } while (originalValue != + (originalValue = CompareExchange(ref alignedRef, newValue, fullComparand))); + + // verify the GC hasn't broken the ref + Debug.Assert((nuint)Unsafe.ByteOffset(ref Unsafe.As(ref alignedRef), ref location1) == offset); + return (byte)(originalValue >> bitOffset); +#endif + } + + /// Compares two 16-bit signed integers for equality and, if they are equal, replaces the first value. + /// The destination, whose value is compared with and possibly replaced. + /// The value that replaces the destination value if the comparison results in equality. + /// The value that is compared to the value at . + /// The original value in . + /// The address of is a null pointer. + [Intrinsic] + [MethodImpl(MethodImplOptions.AggressiveInlining)] [CLSCompliant(false)] - public static ushort CompareExchange(ref ushort location1, ushort value, ushort comparand) => - (ushort)CompareExchange(ref Unsafe.As(ref location1), (short)value, (short)comparand); + public static unsafe ushort CompareExchange(ref ushort location1, ushort value, ushort comparand) + { +#if !MONO && (TARGET_X86 || TARGET_AMD64 || TARGET_ARM64) + return CompareExchange(ref location1, value, comparand); // Must expand intrinsic +#else + // this relies on GC keeping 4B alignment for refs and on subtracting to such alignment being in the same object + nuint offset = Unsafe.OpportunisticMisalignment(ref location1, sizeof(uint)); + ref uint alignedRef = ref Unsafe.As(ref Unsafe.SubtractByteOffset(ref location1, offset)); + int bitOffset = + (int)((BitConverter.IsLittleEndian ? offset : sizeof(uint) - offset - sizeof(byte)) * 8); // to bit offset + Debug.Assert(bitOffset is 0 or 16); + uint mask = ~((uint)ushort.MaxValue << bitOffset); + uint shiftedValue = (uint)value << bitOffset; + uint shiftedComparand = (uint)comparand << bitOffset; + + // this doesn't need to be volatile since CompareExchange will update stale values + uint originalValue = alignedRef; + uint fullComparand, newValue; + do + { + // make sure the ref is still aligned + Debug.Assert(Unsafe.IsOpportunisticallyAligned(ref alignedRef, sizeof(uint))); + uint otherMemory = originalValue & mask; + fullComparand = otherMemory | shiftedComparand; + newValue = otherMemory | shiftedValue; + } while (originalValue != + (originalValue = CompareExchange(ref alignedRef, newValue, fullComparand))); + + // verify the GC hasn't broken the ref + Debug.Assert((nuint)Unsafe.ByteOffset(ref Unsafe.As(ref alignedRef), ref location1) == offset); + return (ushort)(originalValue >> bitOffset); +#endif + } /// Compares two 32-bit unsigned integers for equality and, if they are equal, replaces the first value. /// The destination, whose value is compared with and possibly replaced. diff --git a/src/mono/System.Private.CoreLib/src/System/Threading/Interlocked.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Threading/Interlocked.Mono.cs index 36adaa901e8b0..79e193e70e09c 100644 --- a/src/mono/System.Private.CoreLib/src/System/Threading/Interlocked.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Threading/Interlocked.Mono.cs @@ -8,14 +8,6 @@ namespace System.Threading { public static partial class Interlocked { - [Intrinsic] - [MethodImplAttribute(MethodImplOptions.InternalCall)] - public static extern byte CompareExchange(ref byte location1, byte value, byte comparand); - - [Intrinsic] - [MethodImplAttribute(MethodImplOptions.InternalCall)] - public static extern short CompareExchange(ref short location1, short value, short comparand); - [Intrinsic] [MethodImplAttribute(MethodImplOptions.InternalCall)] public static extern int CompareExchange(ref int location1, int value, int comparand); @@ -61,14 +53,6 @@ public static partial class Interlocked [MethodImplAttribute(MethodImplOptions.InternalCall)] public static extern long Increment(ref long location); - [Intrinsic] - [MethodImplAttribute(MethodImplOptions.InternalCall)] - public static extern byte Exchange(ref byte location1, byte value); - - [Intrinsic] - [MethodImplAttribute(MethodImplOptions.InternalCall)] - public static extern short Exchange(ref short location1, short value); - [Intrinsic] [MethodImplAttribute(MethodImplOptions.InternalCall)] public static extern int Exchange(ref int location1, int value); diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index b8f552dc72dcf..ea62127f66b75 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -556,17 +556,13 @@ HANDLES(STRING_11, "InternalIsInterned", ves_icall_System_String_InternalIsInter ICALL_TYPE(ILOCK, "System.Threading.Interlocked", ILOCK_1) NOHANDLES(ICALL(ILOCK_1, "Add(int&,int)", ves_icall_System_Threading_Interlocked_Add_Int)) NOHANDLES(ICALL(ILOCK_2, "Add(long&,long)", ves_icall_System_Threading_Interlocked_Add_Long)) -NOHANDLES(ICALL(ILOCK_24, "CompareExchange(byte&,byte,byte)", ves_icall_System_Threading_Interlocked_CompareExchange_Byte)) NOHANDLES(ICALL(ILOCK_5, "CompareExchange(int&,int,int)", ves_icall_System_Threading_Interlocked_CompareExchange_Int)) NOHANDLES(ICALL(ILOCK_6, "CompareExchange(int&,int,int,bool&)", ves_icall_System_Threading_Interlocked_CompareExchange_Int_Success)) -NOHANDLES(ICALL(ILOCK_25, "CompareExchange(int16&,int16,int16)", ves_icall_System_Threading_Interlocked_CompareExchange_Short)) NOHANDLES(ICALL(ILOCK_8, "CompareExchange(long&,long,long)", ves_icall_System_Threading_Interlocked_CompareExchange_Long)) NOHANDLES(ICALL(ILOCK_9, "CompareExchange(object&,object&,object&,object&)", ves_icall_System_Threading_Interlocked_CompareExchange_Object)) NOHANDLES(ICALL(ILOCK_11, "Decrement(int&)", ves_icall_System_Threading_Interlocked_Decrement_Int)) NOHANDLES(ICALL(ILOCK_12, "Decrement(long&)", ves_icall_System_Threading_Interlocked_Decrement_Long)) -NOHANDLES(ICALL(ILOCK_26, "Exchange(byte&,byte)", ves_icall_System_Threading_Interlocked_Exchange_Byte)) NOHANDLES(ICALL(ILOCK_15, "Exchange(int&,int)", ves_icall_System_Threading_Interlocked_Exchange_Int)) -NOHANDLES(ICALL(ILOCK_27, "Exchange(int16&,int16)", ves_icall_System_Threading_Interlocked_Exchange_Short)) NOHANDLES(ICALL(ILOCK_17, "Exchange(long&,long)", ves_icall_System_Threading_Interlocked_Exchange_Long)) NOHANDLES(ICALL(ILOCK_18, "Exchange(object&,object&,object&)", ves_icall_System_Threading_Interlocked_Exchange_Object)) NOHANDLES(ICALL(ILOCK_20, "Increment(int&)", ves_icall_System_Threading_Interlocked_Increment_Int)) diff --git a/src/mono/mono/metadata/threads-types.h b/src/mono/mono/metadata/threads-types.h index 576cdcb25ca99..c92e61497dba6 100644 --- a/src/mono/mono/metadata/threads-types.h +++ b/src/mono/mono/metadata/threads-types.h @@ -136,12 +136,6 @@ gint32 ves_icall_System_Threading_Interlocked_Decrement_Int(gint32 *location); ICALL_EXPORT gint64 ves_icall_System_Threading_Interlocked_Decrement_Long(gint64 * location); -ICALL_EXPORT -guint8 ves_icall_System_Threading_Interlocked_Exchange_Byte(guint8 *location, guint8 value); - -ICALL_EXPORT -gint16 ves_icall_System_Threading_Interlocked_Exchange_Short(gint16 *location, gint16 value); - ICALL_EXPORT gint32 ves_icall_System_Threading_Interlocked_Exchange_Int(gint32 *location, gint32 value); @@ -151,12 +145,6 @@ gint64 ves_icall_System_Threading_Interlocked_Exchange_Long(gint64 *location, gi ICALL_EXPORT void ves_icall_System_Threading_Interlocked_Exchange_Object (MonoObject *volatile*location, MonoObject *volatile*value, MonoObject *volatile*res); -ICALL_EXPORT -guint8 ves_icall_System_Threading_Interlocked_CompareExchange_Byte(guint8 *location, guint8 value, guint8 comparand); - -ICALL_EXPORT -gint16 ves_icall_System_Threading_Interlocked_CompareExchange_Short(gint16 *location, gint16 value, gint16 comparand); - ICALL_EXPORT gint32 ves_icall_System_Threading_Interlocked_CompareExchange_Int(gint32 *location, gint32 value, gint32 comparand); diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c index 88c4628284b83..22a7760450e84 100644 --- a/src/mono/mono/metadata/threads.c +++ b/src/mono/mono/metadata/threads.c @@ -2143,22 +2143,6 @@ gint64 ves_icall_System_Threading_Interlocked_Decrement_Long (gint64 * location) return mono_atomic_dec_i64 (location); } -guint8 ves_icall_System_Threading_Interlocked_Exchange_Byte (guint8 *location, guint8 value) -{ - if (G_UNLIKELY (!location)) - return (guint8)set_pending_null_reference_exception (); - - return mono_atomic_xchg_u8(location, value); -} - -gint16 ves_icall_System_Threading_Interlocked_Exchange_Short (gint16 *location, gint16 value) -{ - if (G_UNLIKELY (!location)) - return (gint16)set_pending_null_reference_exception (); - - return mono_atomic_xchg_i16(location, value); -} - gint32 ves_icall_System_Threading_Interlocked_Exchange_Int (gint32 *location, gint32 value) { if (G_UNLIKELY (!location)) @@ -2207,22 +2191,6 @@ ves_icall_System_Threading_Interlocked_Exchange_Long (gint64 *location, gint64 v return mono_atomic_xchg_i64 (location, value); } -guint8 ves_icall_System_Threading_Interlocked_CompareExchange_Byte(guint8 *location, guint8 value, guint8 comparand) -{ - if (G_UNLIKELY (!location)) - return (guint8)set_pending_null_reference_exception (); - - return mono_atomic_cas_u8(location, value, comparand); -} - -gint16 ves_icall_System_Threading_Interlocked_CompareExchange_Short(gint16 *location, gint16 value, gint16 comparand) -{ - if (G_UNLIKELY (!location)) - return (gint16)set_pending_null_reference_exception (); - - return mono_atomic_cas_i16(location, value, comparand); -} - gint32 ves_icall_System_Threading_Interlocked_CompareExchange_Int(gint32 *location, gint32 value, gint32 comparand) { if (G_UNLIKELY (!location)) diff --git a/src/mono/mono/utils/atomic.h b/src/mono/mono/utils/atomic.h index 83a835da1895e..7c7c684ab94eb 100644 --- a/src/mono/mono/utils/atomic.h +++ b/src/mono/mono/utils/atomic.h @@ -95,22 +95,6 @@ Apple targets have historically being problematic, xcode 4.6 would miscompile th #include -static inline guint8 -mono_atomic_cas_u8 (volatile guint8 *dest, guint8 exch, guint8 comp) -{ - g_static_assert (sizeof (atomic_uchar) == sizeof (*dest) && ATOMIC_CHAR_LOCK_FREE == 2); - (void)atomic_compare_exchange_strong ((volatile atomic_uchar *)dest, &comp, exch); - return comp; -} - -static inline gint16 -mono_atomic_cas_i16 (volatile gint16 *dest, gint16 exch, gint16 comp) -{ - g_static_assert (sizeof (atomic_short) == sizeof (*dest) && ATOMIC_SHORT_LOCK_FREE == 2); - (void)atomic_compare_exchange_strong ((volatile atomic_short *)dest, &comp, exch); - return comp; -} - static inline gint32 mono_atomic_cas_i32 (volatile gint32 *dest, gint32 exch, gint32 comp) { @@ -187,20 +171,6 @@ mono_atomic_dec_i64 (volatile gint64 *dest) return mono_atomic_add_i64 (dest, -1); } -static inline guint8 -mono_atomic_xchg_u8 (volatile guint8 *dest, guint8 exch) -{ - g_static_assert (sizeof (atomic_uchar) == sizeof (*dest) && ATOMIC_CHAR_LOCK_FREE == 2); - return atomic_exchange ((volatile atomic_uchar *)dest, exch); -} - -static inline gint16 -mono_atomic_xchg_i16 (volatile gint16 *dest, gint16 exch) -{ - g_static_assert (sizeof (atomic_short) == sizeof (*dest) && ATOMIC_SHORT_LOCK_FREE == 2); - return atomic_exchange ((volatile atomic_short *)dest, exch); -} - static inline gint32 mono_atomic_xchg_i32 (volatile gint32 *dest, gint32 exch) { @@ -341,18 +311,6 @@ mono_atomic_store_ptr (volatile gpointer *dst, gpointer val) #include #include -static inline guint8 -mono_atomic_cas_u8 (volatile guint8 *dest, guint8 exch, guint8 comp) -{ - return _InterlockedCompareExchange8 ((CHAR volatile *)dest, (CHAR)exch, (CHAR)comp); -} - -static inline gint16 -mono_atomic_cas_i16 (volatile gint16 *dest, gint16 exch, gint16 comp) -{ - return _InterlockedCompareExchange16 ((SHORT volatile *)dest, (SHORT)exch, (SHORT)comp); -} - static inline gint32 mono_atomic_cas_i32 (volatile gint32 *dest, gint32 exch, gint32 comp) { @@ -407,18 +365,6 @@ mono_atomic_dec_i64 (volatile gint64 *dest) return InterlockedDecrement64 ((LONG64 volatile *)dest); } -static inline guint8 -mono_atomic_xchg_u8 (volatile guint8 *dest, guint8 exch) -{ - return _InterlockedExchange8 ((CHAR volatile *)dest, (CHAR)exch); -} - -static inline gint16 -mono_atomic_xchg_i16 (volatile gint16 *dest, gint16 exch) -{ - return _InterlockedExchange16 ((SHORT volatile *)dest, (SHORT)exch); -} - static inline gint32 mono_atomic_xchg_i32 (volatile gint32 *dest, gint32 exch) { @@ -562,18 +508,6 @@ mono_atomic_store_ptr (volatile gpointer *dst, gpointer val) #define gcc_sync_fetch_and_add(a, b) __sync_fetch_and_add (a, b) #endif -static inline guint8 mono_atomic_cas_u8(volatile guint8 *dest, - guint8 exch, guint8 comp) -{ - return gcc_sync_val_compare_and_swap (dest, comp, exch); -} - -static inline gint16 mono_atomic_cas_i16(volatile gint16 *dest, - gint16 exch, gint16 comp) -{ - return gcc_sync_val_compare_and_swap (dest, comp, exch); -} - static inline gint32 mono_atomic_cas_i32(volatile gint32 *dest, gint32 exch, gint32 comp) { @@ -600,24 +534,6 @@ static inline gint32 mono_atomic_dec_i32(volatile gint32 *val) return gcc_sync_sub_and_fetch (val, 1); } -static inline guint8 mono_atomic_xchg_u8(volatile guint8 *val, guint8 new_val) -{ - guint8 old_val; - do { - old_val = *val; - } while (gcc_sync_val_compare_and_swap (val, old_val, new_val) != old_val); - return old_val; -} - -static inline gint16 mono_atomic_xchg_i16(volatile gint16 *val, gint16 new_val) -{ - gint16 old_val; - do { - old_val = *val; - } while (gcc_sync_val_compare_and_swap (val, old_val, new_val) != old_val); - return old_val; -} - static inline gint32 mono_atomic_xchg_i32(volatile gint32 *val, gint32 new_val) { gint32 old_val; @@ -809,12 +725,7 @@ static inline void mono_atomic_store_i64(volatile gint64 *dst, gint64 val) #define WAPI_NO_ATOMIC_ASM -/* Fallbacks seem to not be used anymore, they should be removed - * or small type ones should be added in case we find a platform that still needs them. - * extern guint8 mono_atomic_cas_u8(volatile guint8 *dest, guint8 exch, guint8 comp); - * extern gint16 mono_atomic_cas_i16(volatile gint16 *dest, gint16 exch, gint16 comp); - * extern guint8 mono_atomic_xchg_u8(volatile guint8 *dest, guint8 exch); - * extern gint16 mono_atomic_xchg_i16(volatile gint16 *dest, gint16 exch); */ +/* Fallbacks seem to not be used anymore, they should be removed. */ extern gint32 mono_atomic_cas_i32(volatile gint32 *dest, gint32 exch, gint32 comp); extern gint64 mono_atomic_cas_i64(volatile gint64 *dest, gint64 exch, gint64 comp); extern gpointer mono_atomic_cas_ptr(volatile gpointer *dest, gpointer exch, gpointer comp); diff --git a/src/tests/JIT/Intrinsics/Interlocked.cs b/src/tests/JIT/Intrinsics/Interlocked.cs index 5ac15785448e6..09e37223dd966 100644 --- a/src/tests/JIT/Intrinsics/Interlocked.cs +++ b/src/tests/JIT/Intrinsics/Interlocked.cs @@ -3,16 +3,61 @@ // using System; -using System.Threading; +using System.Diagnostics; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Threading; using Xunit; namespace InterlockedTest { public unsafe class Program { - private static int _errors = 0; + [StructLayout(LayoutKind.Explicit)] + private sealed class Box + { + [FieldOffset(0)] + private long memory; + [FieldOffset(8)] + private long val; + [FieldOffset(16)] + public nuint offset; + + public long Memory => memory; + + [MethodImpl(MethodImplOptions.NoInlining)] + public ref T GetRef() where T : unmanaged + { + return ref Unsafe.Add(ref Unsafe.As(ref memory), offset); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public long GetValue(T value, [CallerLineNumber] int line = 0) where T : unmanaged + { + long l = val; + if (l is not (0L or -1L)) + { + Console.WriteLine($"Line {line}: found write out of bounds at offset {offset}"); + _errors++; + } + Unsafe.Add(ref Unsafe.As(ref l), offset) = value; + return l; + } + + public void Set(long value, [CallerLineNumber] int line = 0) + { + if (value != ~val) + { + Console.WriteLine($"Line {line}: found corrupt check value at offset {offset}"); + _errors++; + } + memory = val = value; + } + } + + private static int _errors; + private static Box _box; + private static uint _staticMemory; [Fact] public static int TestEntryPoint() @@ -35,144 +80,135 @@ public static int TestEntryPoint() [MethodImpl(MethodImplOptions.NoInlining)] static delegate* CompareExchangeUShort() => &Interlocked.CompareExchange; - long mem = -1; - [MethodImpl(MethodImplOptions.NoInlining)] - long GetValue(T val) where T : unmanaged + _box = new(); + for (; _box.offset < sizeof(long) / sizeof(ushort); _box.offset++) { - Unsafe.As(ref mem) = val; - return mem; - } + _box.Set(-1); + Equals(255, Interlocked.Exchange(ref _box.GetRef(), 254)); + Equals(_box.GetValue(254), _box.Memory); + Equals(254, ExchangeByte()(ref _box.GetRef(), 253)); + Equals(_box.GetValue(253), _box.Memory); + + _box.Set(0); + Equals(0, Interlocked.Exchange(ref _box.GetRef(), -4)); + Equals(_box.GetValue(-4), _box.Memory); + Equals(-4, ExchangeSByte()(ref _box.GetRef(), -5)); + Equals(_box.GetValue(-5), _box.Memory); + + _box.Set(-1); + Equals(255, Interlocked.CompareExchange(ref _box.GetRef(), 254, 255)); + Equals(_box.GetValue(254), _box.Memory); + Equals(254, CompareExchangeByte()(ref _box.GetRef(), 253, 254)); + Equals(_box.GetValue(253), _box.Memory); + + _box.Set(0); + Equals(0, Interlocked.CompareExchange(ref _box.GetRef(), -4, 0)); + Equals(_box.GetValue(-4), _box.Memory); + Equals(-4, CompareExchangeSByte()(ref _box.GetRef(), -5, -4)); + Equals(_box.GetValue(-5), _box.Memory); - long l = -1; - Equals(255, Interlocked.Exchange(ref Unsafe.As(ref l), 254)); - Equals(GetValue(254), l); - Equals(254, ExchangeByte()(ref Unsafe.As(ref l), 253)); - Equals(GetValue(253), l); - - mem = 0; - l = 0; - Equals(0, Interlocked.Exchange(ref Unsafe.As(ref l), -4)); - Equals(GetValue(-4), l); - Equals(-4, ExchangeSByte()(ref Unsafe.As(ref l), -5)); - Equals(GetValue(-5), l); - - mem = -1; - l = -1; - Equals(255, Interlocked.CompareExchange(ref Unsafe.As(ref l), 254, 255)); - Equals(GetValue(254), l); - Equals(254, CompareExchangeByte()(ref Unsafe.As(ref l), 253, 254)); - Equals(GetValue(253), l); - - mem = 0; - l = 0; - Equals(0, Interlocked.CompareExchange(ref Unsafe.As(ref l), -4, 0)); - Equals(GetValue(-4), l); - Equals(-4, CompareExchangeSByte()(ref Unsafe.As(ref l), -5, -4)); - Equals(GetValue(-5), l); - - Equals(251, Interlocked.CompareExchange(ref Unsafe.As(ref l), 2, 10)); - Equals(GetValue(251), l); - Equals(251, CompareExchangeByte()(ref Unsafe.As(ref l), 2, 10)); - Equals(GetValue(251), l); - Equals(-5, Interlocked.CompareExchange(ref Unsafe.As(ref l), 2, 10)); - Equals(GetValue(-5), l); - Equals(-5, CompareExchangeSByte()(ref Unsafe.As(ref l), 2, 10)); - Equals(GetValue(-5), l); - - mem = 0; - l = 0; - Equals(0, Interlocked.Exchange(ref Unsafe.As(ref l), -2)); - Equals(GetValue(-2), l); - Equals(-2, ExchangeShort()(ref Unsafe.As(ref l), -3)); - Equals(GetValue(-3), l); - - mem = -1; - l = -1; - Equals(65535, Interlocked.Exchange(ref Unsafe.As(ref l), 65532)); - Equals(GetValue(65532), l); - Equals(65532, ExchangeUShort()(ref Unsafe.As(ref l), 65531)); - Equals(GetValue(65531), l); - - mem = 0; - l = 0; - Equals(0, Interlocked.CompareExchange(ref Unsafe.As(ref l), -2, 0)); - Equals(GetValue(-2), l); - Equals(-2, CompareExchangeShort()(ref Unsafe.As(ref l), -3, -2)); - Equals(GetValue(-3), l); - - mem = -1; - l = -1; - Equals(65535, Interlocked.CompareExchange(ref Unsafe.As(ref l), 65532, 65535)); - Equals(GetValue(65532), l); - Equals(65532, CompareExchangeUShort()(ref Unsafe.As(ref l), 65531, 65532)); - Equals(GetValue(65531), l); - - Equals(-5, Interlocked.CompareExchange(ref Unsafe.As(ref l), 1444, 1555)); - Equals(GetValue(-5), l); - Equals(-5, CompareExchangeShort()(ref Unsafe.As(ref l), 1444, 1555)); - Equals(GetValue(-5), l); - Equals(65531, Interlocked.CompareExchange(ref Unsafe.As(ref l), 1444, 1555)); - Equals(GetValue(65531), l); - Equals(65531, CompareExchangeUShort()(ref Unsafe.As(ref l), 1444, 1555)); - Equals(GetValue(65531), l); - - mem = -1; - l = -1; - Interlocked.Exchange(ref Unsafe.As(ref l), 123); - Equals(GetValue(123), l); - ExchangeByte()(ref Unsafe.As(ref l), 124); - Equals(GetValue(124), l); - Interlocked.Exchange(ref Unsafe.As(ref l), 125); - Equals(GetValue(125), l); - ExchangeSByte()(ref Unsafe.As(ref l), 126); - Equals(GetValue(126), l); - - Interlocked.CompareExchange(ref Unsafe.As(ref l), 55, 126); - Equals(GetValue(55), l); - CompareExchangeByte()(ref Unsafe.As(ref l), 56, 55); - Equals(GetValue(56), l); - Interlocked.CompareExchange(ref Unsafe.As(ref l), 57, 56); - Equals(GetValue(57), l); - CompareExchangeSByte()(ref Unsafe.As(ref l), 58, 57); - Equals(GetValue(58), l); - - Interlocked.CompareExchange(ref Unsafe.As(ref l), 10, 2); - Equals(GetValue(58), l); - CompareExchangeByte()(ref Unsafe.As(ref l), 10, 2); - Equals(GetValue(58), l); - Interlocked.CompareExchange(ref Unsafe.As(ref l), 10, 2); - Equals(GetValue(58), l); - CompareExchangeSByte()(ref Unsafe.As(ref l), 10, 2); - Equals(GetValue(58), l); - - mem = -1; - l = -1; - Interlocked.Exchange(ref Unsafe.As(ref l), 12345); - Equals(GetValue(12345), l); - ExchangeShort()(ref Unsafe.As(ref l), 12346); - Equals(GetValue(12346), l); - Interlocked.Exchange(ref Unsafe.As(ref l), 12347); - Equals(GetValue(12347), l); - ExchangeUShort()(ref Unsafe.As(ref l), 12348); - Equals(GetValue(12348), l); - - Interlocked.CompareExchange(ref Unsafe.As(ref l), 1234, 12348); - Equals(GetValue(1234), l); - CompareExchangeShort()(ref Unsafe.As(ref l), 1235, 1234); - Equals(GetValue(1235), l); - Interlocked.CompareExchange(ref Unsafe.As(ref l), 1236, 1235); - Equals(GetValue(1236), l); - CompareExchangeUShort()(ref Unsafe.As(ref l), 1237, 1236); - Equals(GetValue(1237), l); - - Interlocked.CompareExchange(ref Unsafe.As(ref l), 1555, 1444); - Equals(GetValue(1237), l); - CompareExchangeShort()(ref Unsafe.As(ref l), 1555, 1444); - Equals(GetValue(1237), l); - Interlocked.CompareExchange(ref Unsafe.As(ref l), 1555, 1444); - Equals(GetValue(1237), l); - CompareExchangeUShort()(ref Unsafe.As(ref l), 1555, 1444); - Equals(GetValue(1237), l); + Equals(251, Interlocked.CompareExchange(ref _box.GetRef(), 2, 10)); + Equals(_box.GetValue(251), _box.Memory); + Equals(251, CompareExchangeByte()(ref _box.GetRef(), 2, 10)); + Equals(_box.GetValue(251), _box.Memory); + Equals(-5, Interlocked.CompareExchange(ref _box.GetRef(), 2, 10)); + Equals(_box.GetValue(-5), _box.Memory); + Equals(-5, CompareExchangeSByte()(ref _box.GetRef(), 2, 10)); + Equals(_box.GetValue(-5), _box.Memory); + + _box.Set(-1); + _box.Set(0); + Equals(0, Interlocked.Exchange(ref _box.GetRef(), -2)); + Equals(_box.GetValue(-2), _box.Memory); + Equals(-2, ExchangeShort()(ref _box.GetRef(), -3)); + Equals(_box.GetValue(-3), _box.Memory); + + _box.Set(-1); + Equals(65535, Interlocked.Exchange(ref _box.GetRef(), 65532)); + Equals(_box.GetValue(65532), _box.Memory); + Equals(65532, ExchangeUShort()(ref _box.GetRef(), 65531)); + Equals(_box.GetValue(65531), _box.Memory); + + _box.Set(0); + Equals(0, Interlocked.CompareExchange(ref _box.GetRef(), -2, 0)); + Equals(_box.GetValue(-2), _box.Memory); + Equals(-2, CompareExchangeShort()(ref _box.GetRef(), -3, -2)); + Equals(_box.GetValue(-3), _box.Memory); + + _box.Set(-1); + Equals(65535, Interlocked.CompareExchange(ref _box.GetRef(), 65532, 65535)); + Equals(_box.GetValue(65532), _box.Memory); + Equals(65532, CompareExchangeUShort()(ref _box.GetRef(), 65531, 65532)); + Equals(_box.GetValue(65531), _box.Memory); + + Equals(-5, Interlocked.CompareExchange(ref _box.GetRef(), 1444, 1555)); + Equals(_box.GetValue(-5), _box.Memory); + Equals(-5, CompareExchangeShort()(ref _box.GetRef(), 1444, 1555)); + Equals(_box.GetValue(-5), _box.Memory); + Equals(65531, Interlocked.CompareExchange(ref _box.GetRef(), 1444, 1555)); + Equals(_box.GetValue(65531), _box.Memory); + Equals(65531, CompareExchangeUShort()(ref _box.GetRef(), 1444, 1555)); + Equals(_box.GetValue(65531), _box.Memory); + + _box.Set(0); + _box.Set(-1); + Interlocked.Exchange(ref _box.GetRef(), 123); + Equals(_box.GetValue(123), _box.Memory); + ExchangeByte()(ref _box.GetRef(), 124); + Equals(_box.GetValue(124), _box.Memory); + Interlocked.Exchange(ref _box.GetRef(), 125); + Equals(_box.GetValue(125), _box.Memory); + ExchangeSByte()(ref _box.GetRef(), 126); + Equals(_box.GetValue(126), _box.Memory); + + Interlocked.CompareExchange(ref _box.GetRef(), 55, 126); + Equals(_box.GetValue(55), _box.Memory); + CompareExchangeByte()(ref _box.GetRef(), 56, 55); + Equals(_box.GetValue(56), _box.Memory); + Interlocked.CompareExchange(ref _box.GetRef(), 57, 56); + Equals(_box.GetValue(57), _box.Memory); + CompareExchangeSByte()(ref _box.GetRef(), 58, 57); + Equals(_box.GetValue(58), _box.Memory); + + Interlocked.CompareExchange(ref _box.GetRef(), 10, 2); + Equals(_box.GetValue(58), _box.Memory); + CompareExchangeByte()(ref _box.GetRef(), 10, 2); + Equals(_box.GetValue(58), _box.Memory); + Interlocked.CompareExchange(ref _box.GetRef(), 10, 2); + Equals(_box.GetValue(58), _box.Memory); + CompareExchangeSByte()(ref _box.GetRef(), 10, 2); + Equals(_box.GetValue(58), _box.Memory); + + _box.Set(0); + _box.Set(-1); + Interlocked.Exchange(ref _box.GetRef(), 12345); + Equals(_box.GetValue(12345), _box.Memory); + ExchangeShort()(ref _box.GetRef(), 12346); + Equals(_box.GetValue(12346), _box.Memory); + Interlocked.Exchange(ref _box.GetRef(), 12347); + Equals(_box.GetValue(12347), _box.Memory); + ExchangeUShort()(ref _box.GetRef(), 12348); + Equals(_box.GetValue(12348), _box.Memory); + + Interlocked.CompareExchange(ref _box.GetRef(), 1234, 12348); + Equals(_box.GetValue(1234), _box.Memory); + CompareExchangeShort()(ref _box.GetRef(), 1235, 1234); + Equals(_box.GetValue(1235), _box.Memory); + Interlocked.CompareExchange(ref _box.GetRef(), 1236, 1235); + Equals(_box.GetValue(1236), _box.Memory); + CompareExchangeUShort()(ref _box.GetRef(), 1237, 1236); + Equals(_box.GetValue(1237), _box.Memory); + + Interlocked.CompareExchange(ref _box.GetRef(), 1555, 1444); + Equals(_box.GetValue(1237), _box.Memory); + CompareExchangeShort()(ref _box.GetRef(), 1555, 1444); + Equals(_box.GetValue(1237), _box.Memory); + Interlocked.CompareExchange(ref _box.GetRef(), 1555, 1444); + Equals(_box.GetValue(1237), _box.Memory); + CompareExchangeUShort()(ref _box.GetRef(), 1555, 1444); + Equals(_box.GetValue(1237), _box.Memory); + _box.Set(0); + } ThrowsNRE(() => { Interlocked.Exchange(ref Unsafe.NullRef(), 0); }); ThrowsNRE(() => { Interlocked.Exchange(ref Unsafe.NullRef(), 0); }); @@ -191,21 +227,63 @@ long GetValue(T val) where T : unmanaged ThrowsNRE(() => { CompareExchangeShort()(ref Unsafe.NullRef(), 0, 0); }); ThrowsNRE(() => { CompareExchangeUShort()(ref Unsafe.NullRef(), 0, 0); }); + // test for asserts with statics since their addresses are constant which caused issues earlier + // test with 4B alignment provided by the uint field + _staticMemory = 0; + Equals(0, Interlocked.Exchange(ref Unsafe.As(ref _staticMemory), 255)); + Equals(255, Unsafe.As(ref _staticMemory)); + + _staticMemory = 0; + Equals(0, Interlocked.Exchange(ref Unsafe.As(ref _staticMemory), 65535)); + Equals(65535, Unsafe.As(ref _staticMemory)); + + _staticMemory = 0; + Equals(0, Interlocked.CompareExchange(ref Unsafe.As(ref _staticMemory), 255, 0)); + Equals(255, Unsafe.As(ref _staticMemory)); + Equals(255, Interlocked.CompareExchange(ref Unsafe.As(ref _staticMemory), 1, 0)); + Equals(255, Unsafe.As(ref _staticMemory)); + + _staticMemory = 0; + Equals(0, Interlocked.CompareExchange(ref Unsafe.As(ref _staticMemory), 65535, 0)); + Equals(65535, Unsafe.As(ref _staticMemory)); + Equals(65535, Interlocked.CompareExchange(ref Unsafe.As(ref _staticMemory), 1, 0)); + Equals(65535, Unsafe.As(ref _staticMemory)); + + // offset the address by 1 to avoid 4B alignment + _staticMemory = 0; + Equals(0, Interlocked.Exchange(ref Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1), 255)); + Equals(255, Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1)); + + _staticMemory = 0; + Equals(0, Interlocked.Exchange(ref Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1), 65535)); + Equals(65535, Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1)); + + _staticMemory = 0; + Equals(0, Interlocked.CompareExchange(ref Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1), 255, 0)); + Equals(255, Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1)); + Equals(255, Interlocked.CompareExchange(ref Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1), 1, 0)); + Equals(255, Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1)); + + _staticMemory = 0; + Equals(0, Interlocked.CompareExchange(ref Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1), 65535, 0)); + Equals(65535, Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1)); + Equals(65535, Interlocked.CompareExchange(ref Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1), 1, 0)); + Equals(65535, Unsafe.Add(ref Unsafe.As(ref _staticMemory), 1)); + return 100 + _errors; } [MethodImpl(MethodImplOptions.NoInlining)] - static void Equals(long left, long right, [CallerLineNumber] int line = 0, [CallerFilePath] string file = "") + private static void Equals(long left, long right, [CallerLineNumber] int line = 0, [CallerFilePath] string file = "") { - if (left != right) - { - Console.WriteLine($"{file}:L{line} test failed (expected: equal, actual: {left}-{right})."); - _errors++; - } + if (left == right) + return; + Console.WriteLine($"{file}:L{line} test failed (not equal, expected: {left}, actual: {right}) at offset {_box.offset}."); + _errors++; } [MethodImpl(MethodImplOptions.NoInlining)] - static void ThrowsNRE(Action action, [CallerLineNumber] int line = 0, [CallerFilePath] string file = "") + private static void ThrowsNRE(Action action, [CallerLineNumber] int line = 0, [CallerFilePath] string file = "") { try {