Skip to content

Commit

Permalink
Move Interlocked.CompareExchange/Exchange to shared (#47368)
Browse files Browse the repository at this point in the history
* Optimize Interlocked.Exchange and Interlocked.CompareExchange for IntPtr

* Address feedback
  • Loading branch information
EgorBo committed Jan 23, 2021
1 parent 626bbc8 commit 01116d4
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 56 deletions.
Expand Up @@ -85,14 +85,6 @@ public static partial class Interlocked
[return: NotNullIfNotNull("location1")]
public static extern object? Exchange([NotNullIfNotNull("value")] ref object? location1, object? value);

/// <summary>Sets a platform-specific handle or pointer to a specified value and returns the original value, as an atomic operation.</summary>
/// <param name="location1">The variable to set to the specified value.</param>
/// <param name="value">The value to which the <paramref name="location1"/> parameter is set.</param>
/// <returns>The original value of <paramref name="location1"/>.</returns>
/// <exception cref="NullReferenceException">The address of location1 is a null pointer.</exception>
[MethodImpl(MethodImplOptions.InternalCall)]
public static extern IntPtr Exchange(ref IntPtr location1, IntPtr value);

// The below whole method reduces to a single call to Exchange(ref object, object) but
// the JIT thinks that it will generate more native code than it actually does.

Expand Down Expand Up @@ -156,15 +148,6 @@ public static partial class Interlocked
[return: NotNullIfNotNull("location1")]
public static extern object? CompareExchange(ref object? location1, object? value, object? comparand);

/// <summary>Compares two platform-specific handles or pointers for equality and, if they are equal, replaces the first one.</summary>
/// <param name="location1">The destination <see cref="IntPtr"/>, whose value is compared with the value of <paramref name="comparand"/> and possibly replaced by <paramref name="value"/>.</param>
/// <param name="value">The <see cref="IntPtr"/> that replaces the destination value if the comparison results in equality.</param>
/// <param name="comparand">The <see cref="IntPtr"/> that is compared to the value at <paramref name="location1"/>.</param>
/// <returns>The original value in <paramref name="location1"/>.</returns>
/// <exception cref="NullReferenceException">The address of <paramref name="location1"/> is a null pointer.</exception>
[MethodImpl(MethodImplOptions.InternalCall)]
public static extern IntPtr CompareExchange(ref IntPtr location1, IntPtr value, IntPtr comparand);

// Note that getILIntrinsicImplementationForInterlocked() in vm\jitinterface.cpp replaces
// the body of the following method with the the following IL:
// ldarg.0
Expand Down
26 changes: 0 additions & 26 deletions src/coreclr/vm/comutilnative.cpp
Expand Up @@ -1537,19 +1537,6 @@ FCIMPL2_IV(INT64,COMInterlocked::Exchange64, INT64 *location, INT64 value)
}
FCIMPLEND

FCIMPL2(LPVOID,COMInterlocked::ExchangePointer, LPVOID *location, LPVOID value)
{
FCALL_CONTRACT;

if( NULL == location) {
FCThrow(kNullReferenceException);
}

FCUnique(0x15);
return FastInterlockExchangePointer(location, value);
}
FCIMPLEND

FCIMPL3(INT32, COMInterlocked::CompareExchange, INT32* location, INT32 value, INT32 comparand)
{
FCALL_CONTRACT;
Expand All @@ -1574,19 +1561,6 @@ FCIMPL3_IVV(INT64, COMInterlocked::CompareExchange64, INT64* location, INT64 val
}
FCIMPLEND

FCIMPL3(LPVOID,COMInterlocked::CompareExchangePointer, LPVOID *location, LPVOID value, LPVOID comparand)
{
FCALL_CONTRACT;

if( NULL == location) {
FCThrow(kNullReferenceException);
}

FCUnique(0x59);
return FastInterlockCompareExchangePointer(location, value, comparand);
}
FCIMPLEND

FCIMPL2_IV(float,COMInterlocked::ExchangeFloat, float *location, float value)
{
FCALL_CONTRACT;
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/vm/comutilnative.h
Expand Up @@ -215,10 +215,8 @@ class COMInterlocked
public:
static FCDECL2(INT32, Exchange, INT32 *location, INT32 value);
static FCDECL2_IV(INT64, Exchange64, INT64 *location, INT64 value);
static FCDECL2(LPVOID, ExchangePointer, LPVOID* location, LPVOID value);
static FCDECL3(INT32, CompareExchange, INT32* location, INT32 value, INT32 comparand);
static FCDECL3_IVV(INT64, CompareExchange64, INT64* location, INT64 value, INT64 comparand);
static FCDECL3(LPVOID, CompareExchangePointer, LPVOID* location, LPVOID value, LPVOID comparand);
static FCDECL2_IV(float, ExchangeFloat, float *location, float value);
static FCDECL2_IV(double, ExchangeDouble, double *location, double value);
static FCDECL3_IVV(float, CompareExchangeFloat, float *location, float value, float comparand);
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/vm/ecalllist.h
Expand Up @@ -813,13 +813,11 @@ FCFuncStart(gInterlockedFuncs)
FCFuncElementSig("Exchange", &gsig_SM_RefDbl_Dbl_RetDbl, COMInterlocked::ExchangeDouble)
FCFuncElementSig("Exchange", &gsig_SM_RefFlt_Flt_RetFlt, COMInterlocked::ExchangeFloat)
FCFuncElementSig("Exchange", &gsig_SM_RefObj_Obj_RetObj, COMInterlocked::ExchangeObject)
FCFuncElementSig("Exchange", &gsig_SM_RefIntPtr_IntPtr_RetIntPtr, COMInterlocked::ExchangePointer)
FCIntrinsicSig("CompareExchange", &gsig_SM_RefInt_Int_Int_RetInt, COMInterlocked::CompareExchange, CORINFO_INTRINSIC_InterlockedCmpXchg32)
FCIntrinsicSig("CompareExchange", &gsig_SM_RefLong_Long_Long_RetLong, COMInterlocked::CompareExchange64, CORINFO_INTRINSIC_InterlockedCmpXchg64)
FCFuncElementSig("CompareExchange", &gsig_SM_RefDbl_Dbl_Dbl_RetDbl, COMInterlocked::CompareExchangeDouble)
FCFuncElementSig("CompareExchange", &gsig_SM_RefFlt_Flt_Flt_RetFlt, COMInterlocked::CompareExchangeFloat)
FCFuncElementSig("CompareExchange", &gsig_SM_RefObj_Obj_Obj_RetObj, COMInterlocked::CompareExchangeObject)
FCFuncElementSig("CompareExchange", &gsig_SM_RefIntPtr_IntPtr_IntPtr_RetIntPtr, COMInterlocked::CompareExchangePointer)
FCIntrinsicSig("ExchangeAdd", &gsig_SM_RefInt_Int_RetInt, COMInterlocked::ExchangeAdd32, CORINFO_INTRINSIC_InterlockedXAdd32)
FCIntrinsicSig("ExchangeAdd", &gsig_SM_RefLong_Long_RetLong, COMInterlocked::ExchangeAdd64, CORINFO_INTRINSIC_InterlockedXAdd64)

Expand Down
Expand Up @@ -69,6 +69,21 @@ public static partial class Interlocked
[CLSCompliant(false)]
public static ulong Exchange(ref ulong location1, ulong value) =>
(ulong)Exchange(ref Unsafe.As<ulong, long>(ref location1), (long)value);

/// <summary>Sets a platform-specific handle or pointer to a specified value and returns the original value, as an atomic operation.</summary>
/// <param name="location1">The variable to set to the specified value.</param>
/// <param name="value">The value to which the <paramref name="location1"/> parameter is set.</param>
/// <returns>The original value of <paramref name="location1"/>.</returns>
/// <exception cref="NullReferenceException">The address of location1 is a null pointer.</exception>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static IntPtr Exchange(ref IntPtr location1, IntPtr value)
{
#if TARGET_64BIT
return (IntPtr)Interlocked.Exchange(ref Unsafe.As<IntPtr, long>(ref location1), (long)value);
#else
return (IntPtr)Interlocked.Exchange(ref Unsafe.As<IntPtr, int>(ref location1), (int)value);
#endif
}
#endregion

#region CompareExchange
Expand All @@ -93,6 +108,22 @@ public static partial class Interlocked
[CLSCompliant(false)]
public static ulong CompareExchange(ref ulong location1, ulong value, ulong comparand) =>
(ulong)CompareExchange(ref Unsafe.As<ulong, long>(ref location1), (long)value, (long)comparand);

/// <summary>Compares two platform-specific handles or pointers for equality and, if they are equal, replaces the first one.</summary>
/// <param name="location1">The destination <see cref="IntPtr"/>, whose value is compared with the value of <paramref name="comparand"/> and possibly replaced by <paramref name="value"/>.</param>
/// <param name="value">The <see cref="IntPtr"/> that replaces the destination value if the comparison results in equality.</param>
/// <param name="comparand">The <see cref="IntPtr"/> that is compared to the value at <paramref name="location1"/>.</param>
/// <returns>The original value in <paramref name="location1"/>.</returns>
/// <exception cref="NullReferenceException">The address of <paramref name="location1"/> is a null pointer.</exception>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static IntPtr CompareExchange(ref IntPtr location1, IntPtr value, IntPtr comparand)
{
#if TARGET_64BIT
return (IntPtr)Interlocked.CompareExchange(ref Unsafe.As<IntPtr, long>(ref location1), (long)value, (long)comparand);
#else
return (IntPtr)Interlocked.CompareExchange(ref Unsafe.As<IntPtr, int>(ref location1), (int)value, (int)comparand);
#endif
}
#endregion

#region Add
Expand Down
2 changes: 0 additions & 2 deletions src/mono/mono/metadata/icall-def-netcore.h
Expand Up @@ -460,15 +460,13 @@ NOHANDLES(ICALL(ILOCK_2, "Add(long&,long)", ves_icall_System_Threading_Interlock
NOHANDLES(ICALL(ILOCK_4, "CompareExchange(double&,double,double)", ves_icall_System_Threading_Interlocked_CompareExchange_Double))
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_7, "CompareExchange(intptr&,intptr,intptr)", ves_icall_System_Threading_Interlocked_CompareExchange_IntPtr))
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_10, "CompareExchange(single&,single,single)", ves_icall_System_Threading_Interlocked_CompareExchange_Single))
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_14, "Exchange(double&,double)", ves_icall_System_Threading_Interlocked_Exchange_Double))
NOHANDLES(ICALL(ILOCK_15, "Exchange(int&,int)", ves_icall_System_Threading_Interlocked_Exchange_Int))
NOHANDLES(ICALL(ILOCK_16, "Exchange(intptr&,intptr)", ves_icall_System_Threading_Interlocked_Exchange_IntPtr))
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_19, "Exchange(single&,single)", ves_icall_System_Threading_Interlocked_Exchange_Single))
Expand Down
4 changes: 4 additions & 0 deletions src/mono/mono/metadata/threads-types.h
Expand Up @@ -161,8 +161,10 @@ 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);

#ifndef ENABLE_NETCORE
ICALL_EXPORT
gpointer ves_icall_System_Threading_Interlocked_Exchange_IntPtr(gpointer *location, gpointer value);
#endif

ICALL_EXPORT
gfloat ves_icall_System_Threading_Interlocked_Exchange_Single(gfloat *location, gfloat value);
Expand All @@ -182,8 +184,10 @@ gint64 ves_icall_System_Threading_Interlocked_CompareExchange_Long(gint64 *locat
ICALL_EXPORT
void ves_icall_System_Threading_Interlocked_CompareExchange_Object (MonoObject *volatile*location, MonoObject *volatile*value, MonoObject *volatile*comparand, MonoObject *volatile*res);

#ifndef ENABLE_NETCORE
ICALL_EXPORT
gpointer ves_icall_System_Threading_Interlocked_CompareExchange_IntPtr(gpointer *location, gpointer value, gpointer comparand);
#endif

ICALL_EXPORT
gfloat ves_icall_System_Threading_Interlocked_CompareExchange_Single(gfloat *location, gfloat value, gfloat comparand);
Expand Down
4 changes: 4 additions & 0 deletions src/mono/mono/metadata/threads.c
Expand Up @@ -2617,10 +2617,12 @@ ves_icall_System_Threading_Interlocked_Exchange_Object (MonoObject *volatile*loc
mono_gc_wbarrier_generic_nostore_internal ((gpointer)location); // FIXME volatile
}

#ifndef ENABLE_NETCORE
gpointer ves_icall_System_Threading_Interlocked_Exchange_IntPtr (gpointer *location, gpointer value)
{
return mono_atomic_xchg_ptr(location, value);
}
#endif

gfloat ves_icall_System_Threading_Interlocked_Exchange_Single (gfloat *location, gfloat value)
{
Expand Down Expand Up @@ -2686,10 +2688,12 @@ ves_icall_System_Threading_Interlocked_CompareExchange_Object (MonoObject *volat
mono_gc_wbarrier_generic_nostore_internal ((gpointer)location); // FIXME volatile
}

#ifndef ENABLE_NETCORE
gpointer ves_icall_System_Threading_Interlocked_CompareExchange_IntPtr(gpointer *location, gpointer value, gpointer comparand)
{
return mono_atomic_cas_ptr(location, value, comparand);
}
#endif

gfloat ves_icall_System_Threading_Interlocked_CompareExchange_Single (gfloat *location, gfloat value, gfloat comparand)
{
Expand Down
Expand Up @@ -80,9 +80,6 @@ public static partial class Interlocked
[MethodImplAttribute(MethodImplOptions.InternalCall)]
public static extern long CompareExchange(ref long location1, long value, long comparand);

[MethodImplAttribute(MethodImplOptions.InternalCall)]
public static extern IntPtr CompareExchange(ref IntPtr location1, IntPtr value, IntPtr comparand);

[MethodImplAttribute(MethodImplOptions.InternalCall)]
public static extern double CompareExchange(ref double location1, double value, double comparand);

Expand Down Expand Up @@ -114,10 +111,6 @@ public static partial class Interlocked
[MethodImplAttribute(MethodImplOptions.InternalCall)]
public static extern long Exchange(ref long location1, long value);

[Intrinsic]
[MethodImplAttribute(MethodImplOptions.InternalCall)]
public static extern IntPtr Exchange(ref IntPtr location1, IntPtr value);

[Intrinsic]
[MethodImplAttribute(MethodImplOptions.InternalCall)]
public static extern double Exchange(ref double location1, double value);
Expand Down

0 comments on commit 01116d4

Please sign in to comment.