Skip to content

Commit 3e2bc17

Browse files
jkotasjkotas
authored
Remove uses of FCThrow in interlocked helpers (#96916)
FCThrow is implemented usign HELPER_METHOD_FRAME. All uses of FCThrow need to be removed to fix #95695.
1 parent eb5f4e6 commit 3e2bc17

File tree

4 files changed

+115
-57
lines changed

4 files changed

+115
-57
lines changed

‎src/coreclr/System.Private.CoreLib/src/System/Threading/Interlocked.CoreCLR.cs

Lines changed: 101 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,27 +48,60 @@ public static long Decrement(ref long location) =>
4848
/// <returns>The original value of <paramref name="location1"/>.</returns>
4949
/// <exception cref="NullReferenceException">The address of location1 is a null pointer.</exception>
5050
[Intrinsic]
51+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
52+
public static int Exchange(ref int location1, int value)
53+
{
54+
#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64
55+
return Exchange(ref location1, value); // Must expand intrinsic
56+
#else
57+
if (Unsafe.IsNullRef(ref location1))
58+
ThrowHelper.ThrowNullReferenceException();
59+
return Exchange32(ref location1, value);
60+
#endif
61+
}
62+
5163
[MethodImpl(MethodImplOptions.InternalCall)]
52-
public static extern int Exchange(ref int location1, int value);
64+
private static extern int Exchange32(ref int location1, int value);
5365

5466
/// <summary>Sets a 64-bit signed integer to a specified value and returns the original value, as an atomic operation.</summary>
5567
/// <param name="location1">The variable to set to the specified value.</param>
5668
/// <param name="value">The value to which the <paramref name="location1"/> parameter is set.</param>
5769
/// <returns>The original value of <paramref name="location1"/>.</returns>
5870
/// <exception cref="NullReferenceException">The address of location1 is a null pointer.</exception>
5971
[Intrinsic]
72+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
73+
public static long Exchange(ref long location1, long value)
74+
{
75+
#if TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64
76+
return Exchange(ref location1, value); // Must expand intrinsic
77+
#else
78+
if (Unsafe.IsNullRef(ref location1))
79+
ThrowHelper.ThrowNullReferenceException();
80+
return Exchange64(ref location1, value);
81+
#endif
82+
}
83+
6084
[MethodImpl(MethodImplOptions.InternalCall)]
61-
public static extern long Exchange(ref long location1, long value);
85+
private static extern long Exchange64(ref long location1, long value);
6286

6387
/// <summary>Sets an object to the specified value and returns a reference to the original object, as an atomic operation.</summary>
6488
/// <param name="location1">The variable to set to the specified value.</param>
6589
/// <param name="value">The value to which the <paramref name="location1"/> parameter is set.</param>
6690
/// <returns>The original value of <paramref name="location1"/>.</returns>
6791
/// <exception cref="NullReferenceException">The address of location1 is a null pointer.</exception>
6892
[Intrinsic]
69-
[MethodImpl(MethodImplOptions.InternalCall)]
93+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
7094
[return: NotNullIfNotNull(nameof(location1))]
71-
public static extern object? Exchange([NotNullIfNotNull(nameof(value))] ref object? location1, object? value);
95+
public static object? Exchange([NotNullIfNotNull(nameof(value))] ref object? location1, object? value)
96+
{
97+
if (Unsafe.IsNullRef(ref location1))
98+
ThrowHelper.ThrowNullReferenceException();
99+
return ExchangeObject(ref location1, value);
100+
}
101+
102+
[return: NotNullIfNotNull(nameof(location1))]
103+
[MethodImpl(MethodImplOptions.InternalCall)]
104+
private static extern object? ExchangeObject([NotNullIfNotNull(nameof(value))] ref object? location1, object? value);
72105

73106
// The below whole method reduces to a single call to Exchange(ref object, object) but
74107
// the JIT thinks that it will generate more native code than it actually does.
@@ -84,7 +117,7 @@ public static long Decrement(ref long location) =>
84117
[MethodImpl(MethodImplOptions.AggressiveInlining)]
85118
public static T Exchange<T>([NotNullIfNotNull(nameof(value))] ref T location1, T value) where T : class? =>
86119
Unsafe.As<T>(Exchange(ref Unsafe.As<T, object?>(ref location1), value));
87-
#endregion
120+
#endregion
88121

89122
#region CompareExchange
90123
/// <summary>Compares two 32-bit signed integers for equality and, if they are equal, replaces the first value.</summary>
@@ -94,8 +127,20 @@ public static T Exchange<T>([NotNullIfNotNull(nameof(value))] ref T location1, T
94127
/// <returns>The original value in <paramref name="location1"/>.</returns>
95128
/// <exception cref="NullReferenceException">The address of <paramref name="location1"/> is a null pointer.</exception>
96129
[Intrinsic]
130+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
131+
public static int CompareExchange(ref int location1, int value, int comparand)
132+
{
133+
#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64
134+
return CompareExchange(ref location1, value, comparand); // Must expand intrinsic
135+
#else
136+
if (Unsafe.IsNullRef(ref location1))
137+
ThrowHelper.ThrowNullReferenceException();
138+
return CompareExchange32(ref location1, value, comparand);
139+
#endif
140+
}
141+
97142
[MethodImpl(MethodImplOptions.InternalCall)]
98-
public static extern int CompareExchange(ref int location1, int value, int comparand);
143+
private static extern int CompareExchange32(ref int location1, int value, int comparand);
99144

100145
/// <summary>Compares two 64-bit signed integers for equality and, if they are equal, replaces the first value.</summary>
101146
/// <param name="location1">The destination, whose value is compared with <paramref name="comparand"/> and possibly replaced.</param>
@@ -104,8 +149,20 @@ public static T Exchange<T>([NotNullIfNotNull(nameof(value))] ref T location1, T
104149
/// <returns>The original value in <paramref name="location1"/>.</returns>
105150
/// <exception cref="NullReferenceException">The address of <paramref name="location1"/> is a null pointer.</exception>
106151
[Intrinsic]
152+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
153+
public static long CompareExchange(ref long location1, long value, long comparand)
154+
{
155+
#if TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64
156+
return CompareExchange(ref location1, value, comparand); // Must expand intrinsic
157+
#else
158+
if (Unsafe.IsNullRef(ref location1))
159+
ThrowHelper.ThrowNullReferenceException();
160+
return CompareExchange64(ref location1, value, comparand);
161+
#endif
162+
}
163+
107164
[MethodImpl(MethodImplOptions.InternalCall)]
108-
public static extern long CompareExchange(ref long location1, long value, long comparand);
165+
private static extern long CompareExchange64(ref long location1, long value, long comparand);
109166

110167
/// <summary>Compares two objects for reference equality and, if they are equal, replaces the first object.</summary>
111168
/// <param name="location1">The destination object that is compared by reference with <paramref name="comparand"/> and possibly replaced.</param>
@@ -114,9 +171,18 @@ public static T Exchange<T>([NotNullIfNotNull(nameof(value))] ref T location1, T
114171
/// <returns>The original value in <paramref name="location1"/>.</returns>
115172
/// <exception cref="NullReferenceException">The address of <paramref name="location1"/> is a null pointer.</exception>
116173
[Intrinsic]
174+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
175+
[return: NotNullIfNotNull(nameof(location1))]
176+
public static object? CompareExchange(ref object? location1, object? value, object? comparand)
177+
{
178+
if (Unsafe.IsNullRef(ref location1))
179+
ThrowHelper.ThrowNullReferenceException();
180+
return CompareExchangeObject(ref location1, value, comparand);
181+
}
182+
117183
[MethodImpl(MethodImplOptions.InternalCall)]
118184
[return: NotNullIfNotNull(nameof(location1))]
119-
public static extern object? CompareExchange(ref object? location1, object? value, object? comparand);
185+
private static extern object? CompareExchangeObject(ref object? location1, object? value, object? comparand);
120186

121187
// Note that getILIntrinsicImplementationForInterlocked() in vm\jitinterface.cpp replaces
122188
// the body of the following method with the following IL:
@@ -136,8 +202,8 @@ public static T Exchange<T>([NotNullIfNotNull(nameof(value))] ref T location1, T
136202
/// <exception cref="NullReferenceException">The address of <paramref name="location1"/> is a null pointer.</exception>
137203
/// <typeparam name="T">The type to be used for <paramref name="location1"/>, <paramref name="value"/>, and <paramref name="comparand"/>. This type must be a reference type.</typeparam>
138204
[Intrinsic]
139-
[return: NotNullIfNotNull(nameof(location1))]
140205
[MethodImpl(MethodImplOptions.AggressiveInlining)]
206+
[return: NotNullIfNotNull(nameof(location1))]
141207
public static T CompareExchange<T>(ref T location1, T value, T comparand) where T : class? =>
142208
Unsafe.As<T>(CompareExchange(ref Unsafe.As<T, object?>(ref location1), value, comparand));
143209
#endregion
@@ -160,12 +226,36 @@ public static long Add(ref long location1, long value) =>
160226
ExchangeAdd(ref location1, value) + value;
161227

162228
[Intrinsic]
229+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
230+
private static int ExchangeAdd(ref int location1, int value)
231+
{
232+
#if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64
233+
return ExchangeAdd(ref location1, value); // Must expand intrinsic
234+
#else
235+
if (Unsafe.IsNullRef(ref location1))
236+
ThrowHelper.ThrowNullReferenceException();
237+
return ExchangeAdd32(ref location1, value);
238+
#endif
239+
}
240+
163241
[MethodImpl(MethodImplOptions.InternalCall)]
164-
private static extern int ExchangeAdd(ref int location1, int value);
242+
private static extern int ExchangeAdd32(ref int location1, int value);
165243

166244
[Intrinsic]
245+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
246+
private static long ExchangeAdd(ref long location1, long value)
247+
{
248+
#if TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64
249+
return ExchangeAdd(ref location1, value); // Must expand intrinsic
250+
#else
251+
if (Unsafe.IsNullRef(ref location1))
252+
ThrowHelper.ThrowNullReferenceException();
253+
return ExchangeAdd64(ref location1, value);
254+
#endif
255+
}
256+
167257
[MethodImpl(MethodImplOptions.InternalCall)]
168-
private static extern long ExchangeAdd(ref long location1, long value);
258+
private static extern long ExchangeAdd64(ref long location1, long value);
169259
#endregion
170260

171261
#region Read

‎src/coreclr/vm/comutilnative.cpp

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,14 +1490,10 @@ NOINLINE void GCInterface::GarbageCollectModeAny(int generation)
14901490

14911491
#include <optsmallperfcritical.h>
14921492

1493-
FCIMPL2(INT32,COMInterlocked::Exchange, INT32 *location, INT32 value)
1493+
FCIMPL2(INT32,COMInterlocked::Exchange32, INT32 *location, INT32 value)
14941494
{
14951495
FCALL_CONTRACT;
14961496

1497-
if( NULL == location) {
1498-
FCThrow(kNullReferenceException);
1499-
}
1500-
15011497
return InterlockedExchange((LONG *) location, value);
15021498
}
15031499
FCIMPLEND
@@ -1506,22 +1502,14 @@ FCIMPL2_IV(INT64,COMInterlocked::Exchange64, INT64 *location, INT64 value)
15061502
{
15071503
FCALL_CONTRACT;
15081504

1509-
if( NULL == location) {
1510-
FCThrow(kNullReferenceException);
1511-
}
1512-
15131505
return InterlockedExchange64((INT64 *) location, value);
15141506
}
15151507
FCIMPLEND
15161508

1517-
FCIMPL3(INT32, COMInterlocked::CompareExchange, INT32* location, INT32 value, INT32 comparand)
1509+
FCIMPL3(INT32, COMInterlocked::CompareExchange32, INT32* location, INT32 value, INT32 comparand)
15181510
{
15191511
FCALL_CONTRACT;
15201512

1521-
if( NULL == location) {
1522-
FCThrow(kNullReferenceException);
1523-
}
1524-
15251513
return InterlockedCompareExchange((LONG*)location, value, comparand);
15261514
}
15271515
FCIMPLEND
@@ -1530,10 +1518,6 @@ FCIMPL3_IVV(INT64, COMInterlocked::CompareExchange64, INT64* location, INT64 val
15301518
{
15311519
FCALL_CONTRACT;
15321520

1533-
if( NULL == location) {
1534-
FCThrow(kNullReferenceException);
1535-
}
1536-
15371521
return InterlockedCompareExchange64((INT64*)location, value, comparand);
15381522
}
15391523
FCIMPLEND
@@ -1542,10 +1526,6 @@ FCIMPL2(LPVOID,COMInterlocked::ExchangeObject, LPVOID*location, LPVOID value)
15421526
{
15431527
FCALL_CONTRACT;
15441528

1545-
if( NULL == location) {
1546-
FCThrow(kNullReferenceException);
1547-
}
1548-
15491529
LPVOID ret = InterlockedExchangeT(location, value);
15501530
#ifdef _DEBUG
15511531
Thread::ObjectRefAssign((OBJECTREF *)location);
@@ -1559,10 +1539,6 @@ FCIMPL3(LPVOID,COMInterlocked::CompareExchangeObject, LPVOID *location, LPVOID v
15591539
{
15601540
FCALL_CONTRACT;
15611541

1562-
if( NULL == location) {
1563-
FCThrow(kNullReferenceException);
1564-
}
1565-
15661542
// <TODO>@todo: only set ref if is updated</TODO>
15671543
LPVOID ret = InterlockedCompareExchangeT(location, value, comparand);
15681544
if (ret == comparand) {
@@ -1579,10 +1555,6 @@ FCIMPL2(INT32,COMInterlocked::ExchangeAdd32, INT32 *location, INT32 value)
15791555
{
15801556
FCALL_CONTRACT;
15811557

1582-
if( NULL == location) {
1583-
FCThrow(kNullReferenceException);
1584-
}
1585-
15861558
return InterlockedExchangeAdd((LONG *) location, value);
15871559
}
15881560
FCIMPLEND
@@ -1591,10 +1563,6 @@ FCIMPL2_IV(INT64,COMInterlocked::ExchangeAdd64, INT64 *location, INT64 value)
15911563
{
15921564
FCALL_CONTRACT;
15931565

1594-
if( NULL == location) {
1595-
FCThrow(kNullReferenceException);
1596-
}
1597-
15981566
return InterlockedExchangeAdd64((INT64 *) location, value);
15991567
}
16001568
FCIMPLEND

‎src/coreclr/vm/comutilnative.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,10 @@ extern "C" uint64_t QCALLTYPE GCInterface_GetGenerationBudget(int generation);
230230
class COMInterlocked
231231
{
232232
public:
233-
static FCDECL2(INT32, Exchange, INT32 *location, INT32 value);
234-
static FCDECL2_IV(INT64, Exchange64, INT64 *location, INT64 value);
235-
static FCDECL3(INT32, CompareExchange, INT32* location, INT32 value, INT32 comparand);
236-
static FCDECL3_IVV(INT64, CompareExchange64, INT64* location, INT64 value, INT64 comparand);
233+
static FCDECL2(INT32, Exchange32, INT32 *location, INT32 value);
234+
static FCDECL2_IV(INT64, Exchange64, INT64 *location, INT64 value);
235+
static FCDECL3(INT32, CompareExchange32, INT32* location, INT32 value, INT32 comparand);
236+
static FCDECL3_IVV(INT64, CompareExchange64, INT64* location, INT64 value, INT64 comparand);
237237
static FCDECL2(LPVOID, ExchangeObject, LPVOID* location, LPVOID value);
238238
static FCDECL3(LPVOID, CompareExchangeObject, LPVOID* location, LPVOID value, LPVOID comparand);
239239
static FCDECL2(INT32, ExchangeAdd32, INT32 *location, INT32 value);

‎src/coreclr/vm/ecalllist.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -457,14 +457,14 @@ FCFuncStart(gInteropMarshalFuncs)
457457
FCFuncEnd()
458458

459459
FCFuncStart(gInterlockedFuncs)
460-
FCFuncElementSig("Exchange", &gsig_SM_RefInt_Int_RetInt, COMInterlocked::Exchange)
461-
FCFuncElementSig("Exchange", &gsig_SM_RefLong_Long_RetLong, COMInterlocked::Exchange64)
462-
FCFuncElementSig("Exchange", &gsig_SM_RefObj_Obj_RetObj, COMInterlocked::ExchangeObject)
463-
FCFuncElementSig("CompareExchange", &gsig_SM_RefInt_Int_Int_RetInt, COMInterlocked::CompareExchange)
464-
FCFuncElementSig("CompareExchange", &gsig_SM_RefLong_Long_Long_RetLong, COMInterlocked::CompareExchange64)
465-
FCFuncElementSig("CompareExchange", &gsig_SM_RefObj_Obj_Obj_RetObj, COMInterlocked::CompareExchangeObject)
466-
FCFuncElementSig("ExchangeAdd", &gsig_SM_RefInt_Int_RetInt, COMInterlocked::ExchangeAdd32)
467-
FCFuncElementSig("ExchangeAdd", &gsig_SM_RefLong_Long_RetLong, COMInterlocked::ExchangeAdd64)
460+
FCFuncElement("Exchange32", COMInterlocked::Exchange32)
461+
FCFuncElement("Exchange64", COMInterlocked::Exchange64)
462+
FCFuncElement("ExchangeObject", COMInterlocked::ExchangeObject)
463+
FCFuncElement("CompareExchange32", COMInterlocked::CompareExchange32)
464+
FCFuncElement("CompareExchange64", COMInterlocked::CompareExchange64)
465+
FCFuncElement("CompareExchangeObject", COMInterlocked::CompareExchangeObject)
466+
FCFuncElement("ExchangeAdd32", COMInterlocked::ExchangeAdd32)
467+
FCFuncElement("ExchangeAdd64", COMInterlocked::ExchangeAdd64)
468468
FCFuncEnd()
469469

470470
FCFuncStart(gJitInfoFuncs)

0 commit comments

Comments
 (0)