Skip to content

Commit a3b2d40

Browse files
EgorBojkotasjakobbotschCopilot
authored
Inline CORINFO_HELP_ARRADDR_ST helper call, remove WriteBarrier FCall (#117583)
Inline write barriers with covariance checks (in case if inliner decides that it's profitable) - this allows to remove redundant write barriers (previously we could only do that in early phases) and range checks (previously they were inside the helper call and now JIT can fold them). Related: #9159 ## Benchmark ```cs using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Running; BenchmarkSwitcher.FromAssembly(typeof(Benchmarks).Assembly).Run(args); public class Benchmarks { static object[] _strings = new string[4]; static object _x = ""; [Benchmark] public void AssignString() { var arr = _strings; if (arr.Length >= 4) { // We now can remove write barriers and redundant range checks if StelemRef gets inlined arr[0] = ""; arr[1] = ""; arr[2] = ""; arr[3] = ""; } } [Benchmark] public void SwapElements() { var arr = _strings; (arr[1], arr[0]) = (arr[0], arr[1]); } [Benchmark] public void SingleAssignmentCns() { _strings[0] = ""; } [Benchmark] public void SingleAssignmentVar() { _strings[0] = _x; } } ``` **Linux-AMD (Genoa):** | Method | Toolchain | Mean | Error | Ratio | |----------------- |------------------------ |----------:|----------:|------:| | AssignString | Main | 7.5633 ns | 0.0022 ns | 1.00 | | AssignString | PR | 1.3160 ns | 0.0033 ns | 0.17 | | | | | | | | SwapElements | Main | 2.9979 ns | 0.0008 ns | 1.00 | | SwapElements | PR | 1.0471 ns | 0.0012 ns | 0.35 | | | | | | | | SingleAssignmentCns | Main | 1.9075 ns | 0.0005 ns | 1.00 | | SingleAssignmentCns | PR | 0.2723 ns | 0.0002 ns | 0.14 | | | | | | | | SingleAssignmentVar | Main | 1.9080 ns | 0.0003 ns | 1.00 | | SingleAssignmentVar | PR | 1.6356 ns | 0.0005 ns | 0.86 | **Linux-ARM64 (Cobalt100):** | Method | Toolchain | Mean | Error | Ratio | |----------------- |------------------------ |-----------:|----------:|------:| | AssignString | Main | 10.3296 ns | 0.0032 ns | 1.00 | | AssignString | PR | 1.8738 ns | 0.0006 ns | 0.18 | | | | | | | | SwapElements | Main | 2.6604 ns | 0.0012 ns | 1.00 | | SwapElements | PR | 1.1768 ns | 0.0024 ns | 0.44 | | | | | | | | SingleAssignmentCns | Main | 1.9655 ns | 0.0023 ns | 1.00 | | SingleAssignmentCns | PR | 0.3821 ns | 0.0003 ns | 0.19 | | | | | | | | SingleAssignmentVar | Main | 1.8030 ns | 0.0013 ns | 1.00 | | SingleAssignmentVar | PR | 0.7219 ns | 0.0006 ns | 0.40 | --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com> Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com> Co-authored-by: EgorBo <egorbo@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7891017 commit a3b2d40

File tree

25 files changed

+151
-139
lines changed

25 files changed

+151
-139
lines changed

‎src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,6 @@ private static object ChkCastAny_NoCacheLookup(void* toTypeHnd, object obj)
5959
return obj;
6060
}
6161

62-
[MethodImpl(MethodImplOptions.InternalCall)]
63-
private static extern void WriteBarrier(ref object? dst, object? obj);
64-
6562
// IsInstanceOf test used for unusual cases (naked type parameters, variant generic types)
6663
// Unlike the IsInstanceOfInterface and IsInstanceOfClass functions,
6764
// this test must deal with all kinds of type tests
@@ -471,7 +468,7 @@ private static void StelemRef(object?[] array, nint index, object? obj)
471468
goto notExactMatch;
472469

473470
doWrite:
474-
WriteBarrier(ref element, obj);
471+
RuntimeHelpers.WriteBarrier(ref element, obj);
475472
return;
476473

477474
assigningNull:
@@ -493,7 +490,7 @@ private static void StelemRef_Helper(ref object? element, void* elementType, obj
493490
CastResult result = CastCache.TryGet(s_table!, (nuint)RuntimeHelpers.GetMethodTable(obj), (nuint)elementType);
494491
if (result == CastResult.CanCast)
495492
{
496-
WriteBarrier(ref element, obj);
493+
RuntimeHelpers.WriteBarrier(ref element, obj);
497494
return;
498495
}
499496

@@ -512,7 +509,7 @@ private static void StelemRef_Helper_NoCacheLookup(ref object? element, void* el
512509
ThrowArrayMismatchException();
513510
}
514511

515-
WriteBarrier(ref element, obj2);
512+
RuntimeHelpers.WriteBarrier(ref element, obj2);
516513
}
517514

518515
[DebuggerHidden]

‎src/coreclr/jit/compiler.h‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5342,6 +5342,30 @@ class Compiler
53425342
GenTree* dereferencedAddress,
53435343
InlArgInfo* inlArgInfo);
53445344

5345+
typedef JitHashTable<CORINFO_METHOD_HANDLE, JitPtrKeyFuncs<struct CORINFO_METHOD_STRUCT_>, CORINFO_METHOD_HANDLE> HelperToManagedMap;
5346+
HelperToManagedMap* m_helperToManagedMap = nullptr;
5347+
5348+
public:
5349+
HelperToManagedMap* GetHelperToManagedMap()
5350+
{
5351+
if (m_helperToManagedMap == nullptr)
5352+
{
5353+
m_helperToManagedMap = new (getAllocator()) HelperToManagedMap(getAllocator());
5354+
}
5355+
return m_helperToManagedMap;
5356+
}
5357+
bool HelperToManagedMapLookup(CORINFO_METHOD_HANDLE helperCallHnd, CORINFO_METHOD_HANDLE* userCallHnd)
5358+
{
5359+
if (m_helperToManagedMap == nullptr)
5360+
{
5361+
return false;
5362+
}
5363+
bool found = m_helperToManagedMap->Lookup(helperCallHnd, userCallHnd);
5364+
return found;
5365+
}
5366+
private:
5367+
5368+
void impConvertToUserCallAndMarkForInlining(GenTreeCall* call);
53455369
void impMarkInlineCandidate(GenTree* call,
53465370
CORINFO_CONTEXT_HANDLE exactContextHnd,
53475371
bool exactContextNeedsRuntimeLookup,

‎src/coreclr/jit/gentree.cpp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2398,6 +2398,34 @@ bool GenTreeCall::IsHelperCall(Compiler* compiler, unsigned helper) const
23982398
return IsHelperCall(compiler->eeFindHelper(helper));
23992399
}
24002400

2401+
//-------------------------------------------------------------------------
2402+
// IsHelperCallOrUserEquivalent: Determine if this GT_CALL node is a specific helper call
2403+
// or its CT_USER equivalent.
2404+
//
2405+
// Arguments:
2406+
// compiler - the compiler instance so that we can call eeFindHelper
2407+
//
2408+
// Return Value:
2409+
// Returns true if this GT_CALL node is a call to the specified helper.
2410+
//
2411+
bool GenTreeCall::IsHelperCallOrUserEquivalent(Compiler* compiler, unsigned helper) const
2412+
{
2413+
CORINFO_METHOD_HANDLE helperCallHnd = Compiler::eeFindHelper(helper);
2414+
if (IsHelperCall())
2415+
{
2416+
return helperCallHnd == gtCallMethHnd;
2417+
}
2418+
2419+
if (gtCallType == CT_USER_FUNC)
2420+
{
2421+
CORINFO_METHOD_HANDLE userCallHnd = NO_METHOD_HANDLE;
2422+
return compiler->impInlineRoot()->HelperToManagedMapLookup(helperCallHnd, &userCallHnd) &&
2423+
(userCallHnd == gtCallMethHnd);
2424+
}
2425+
2426+
return false;
2427+
}
2428+
24012429
//-------------------------------------------------------------------------
24022430
// IsRuntimeLookupHelperCall: Determine if this GT_CALL node represents a runtime lookup helper call.
24032431
//
@@ -13011,6 +13039,9 @@ void Compiler::gtDispTree(GenTree* tree,
1301113039
case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:
1301213040
printf(" isKnownConst");
1301313041
break;
13042+
case NI_System_Runtime_CompilerServices_RuntimeHelpers_WriteBarrier:
13043+
printf(" WriteBarrier");
13044+
break;
1301413045
#if defined(FEATURE_SIMD)
1301513046
case NI_SIMD_UpperRestore:
1301613047
printf(" simdUpperRestore");

‎src/coreclr/jit/gentree.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5877,6 +5877,8 @@ struct GenTreeCall final : public GenTree
58775877

58785878
bool IsHelperCall(Compiler* compiler, unsigned helper) const;
58795879

5880+
bool IsHelperCallOrUserEquivalent(Compiler* compiler, unsigned helper) const;
5881+
58805882
bool IsRuntimeLookupHelperCall(Compiler* compiler) const;
58815883

58825884
bool IsSpecialIntrinsic(Compiler* compiler, NamedIntrinsic ni) const;

‎src/coreclr/jit/importer.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7268,7 +7268,12 @@ void Compiler::impImportBlockCode(BasicBlock* block)
72687268
// The array helper takes a native int for array length.
72697269
// So if we have an int, explicitly extend it to be a native int.
72707270
index = impImplicitIorI4Cast(index, TYP_I_IMPL);
7271-
op1 = gtNewHelperCallNode(CORINFO_HELP_ARRADDR_ST, TYP_VOID, array, index, value);
7271+
7272+
GenTreeCall* call = gtNewHelperCallNode(CORINFO_HELP_ARRADDR_ST, TYP_VOID, array, index, value);
7273+
INDEBUG(call->gtRawILOffset = opcodeOffs);
7274+
impConvertToUserCallAndMarkForInlining(call);
7275+
op1 = call;
7276+
72727277
goto SPILL_APPEND;
72737278
}
72747279

‎src/coreclr/jit/importercalls.cpp‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3411,6 +3411,8 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
34113411
// This one is just `return true/false`
34123412
case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:
34133413

3414+
case NI_System_Runtime_CompilerServices_RuntimeHelpers_WriteBarrier:
3415+
34143416
// Not expanding this can lead to noticeable allocations in T0
34153417
case NI_System_Runtime_CompilerServices_RuntimeHelpers_CreateSpan:
34163418

@@ -3640,6 +3642,14 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
36403642
break;
36413643
}
36423644

3645+
case NI_System_Runtime_CompilerServices_RuntimeHelpers_WriteBarrier:
3646+
{
3647+
GenTree* val = impPopStack().val;
3648+
GenTree* dst = impPopStack().val;
3649+
retNode = gtNewStoreIndNode(TYP_REF, dst, val, GTF_IND_TGT_HEAP);
3650+
break;
3651+
}
3652+
36433653
case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:
36443654
{
36453655
GenTree* op1 = impPopStack().val;
@@ -7978,6 +7988,53 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call,
79787988
call->AddGDVCandidateInfo(this, pInfo);
79797989
}
79807990

7991+
//------------------------------------------------------------------------
7992+
// impConvertToUserCallAndMarkForInlining: convert a helper call to a user call
7993+
// and mark it for inlining. This is used for helper calls that are
7994+
// known to be backed by a user method that can be inlined.
7995+
//
7996+
// Arguments:
7997+
// call - the helper call to convert
7998+
//
7999+
void Compiler::impConvertToUserCallAndMarkForInlining(GenTreeCall* call)
8000+
{
8001+
assert(call->IsHelperCall());
8002+
8003+
if (!opts.OptEnabled(CLFLG_INLINING))
8004+
{
8005+
return;
8006+
}
8007+
8008+
CORINFO_METHOD_HANDLE helperCallHnd = call->gtCallMethHnd;
8009+
CORINFO_METHOD_HANDLE managedCallHnd = NO_METHOD_HANDLE;
8010+
CORINFO_CONST_LOOKUP pNativeEntrypoint = {};
8011+
info.compCompHnd->getHelperFtn(eeGetHelperNum(helperCallHnd), &pNativeEntrypoint, &managedCallHnd);
8012+
8013+
if (managedCallHnd != NO_METHOD_HANDLE)
8014+
{
8015+
call->gtCallMethHnd = managedCallHnd;
8016+
call->gtCallType = CT_USER_FUNC;
8017+
8018+
CORINFO_CALL_INFO hCallInfo = {};
8019+
hCallInfo.hMethod = managedCallHnd;
8020+
hCallInfo.methodFlags = info.compCompHnd->getMethodAttribs(hCallInfo.hMethod);
8021+
impMarkInlineCandidate(call, nullptr, false, &hCallInfo, compInlineContext);
8022+
8023+
#if DEBUG
8024+
CORINFO_METHOD_HANDLE existingValue = NO_METHOD_HANDLE;
8025+
if (impInlineRoot()->HelperToManagedMapLookup(helperCallHnd, &existingValue))
8026+
{
8027+
// Let's make sure HelperToManagedMap::Overwrite behavior always overwrites the same value.
8028+
assert(existingValue == managedCallHnd);
8029+
}
8030+
#endif
8031+
8032+
impInlineRoot()->GetHelperToManagedMap()->Set(helperCallHnd, managedCallHnd, HelperToManagedMap::Overwrite);
8033+
JITDUMP("Converting helperCall [%06u] to user call [%s] and marking for inlining\n", dspTreeID(call),
8034+
eeGetMethodFullName(managedCallHnd));
8035+
}
8036+
}
8037+
79818038
//------------------------------------------------------------------------
79828039
// impMarkInlineCandidate: determine if this call can be subsequently inlined
79838040
//
@@ -10931,6 +10988,10 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
1093110988
{
1093210989
result = NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant;
1093310990
}
10991+
else if (strcmp(methodName, "WriteBarrier") == 0)
10992+
{
10993+
result = NI_System_Runtime_CompilerServices_RuntimeHelpers_WriteBarrier;
10994+
}
1093410995
else if (strcmp(methodName, "IsReferenceOrContainsReferences") == 0)
1093510996
{
1093610997
result =

‎src/coreclr/jit/morph.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6459,14 +6459,23 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
64596459

64606460
// Morph stelem.ref helper call to store a null value, into a store into an array without the helper.
64616461
// This needs to be done after the arguments are morphed to ensure constant propagation has already taken place.
6462-
if (opts.OptimizationEnabled() && call->IsHelperCall(this, CORINFO_HELP_ARRADDR_ST))
6462+
if (opts.OptimizationEnabled() && call->IsHelperCallOrUserEquivalent(this, CORINFO_HELP_ARRADDR_ST))
64636463
{
64646464
assert(call->gtArgs.CountUserArgs() == 3);
64656465

64666466
GenTree* arr = call->gtArgs.GetUserArgByIndex(0)->GetNode();
64676467
GenTree* index = call->gtArgs.GetUserArgByIndex(1)->GetNode();
64686468
GenTree* value = call->gtArgs.GetUserArgByIndex(2)->GetNode();
64696469

6470+
if (!call->IsHelperCall())
6471+
{
6472+
// Convert back to helper call if it wasn't inlined.
6473+
// Currently, only helper calls are eligible to be direct calls if the target has reached
6474+
// its final tier. TODO: remove this workaround and convert this user call to direct as well.
6475+
call->gtCallMethHnd = eeFindHelper(CORINFO_HELP_ARRADDR_ST);
6476+
call->gtCallType = CT_HELPER;
6477+
}
6478+
64706479
if (gtCanSkipCovariantStoreCheck(value, arr))
64716480
{
64726481
// Either or both of the array and index arguments may have been spilled to temps by `fgMorphArgs`. Copy

‎src/coreclr/jit/namedintrinsiclist.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ enum NamedIntrinsic : unsigned short
123123
NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant,
124124
NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences,
125125
NI_System_Runtime_CompilerServices_RuntimeHelpers_GetMethodTable,
126+
NI_System_Runtime_CompilerServices_RuntimeHelpers_WriteBarrier,
126127
NI_System_Runtime_CompilerServices_RuntimeHelpers_SetNextCallGenericContext,
127128
NI_System_Runtime_CompilerServices_RuntimeHelpers_SetNextCallAsyncContinuation,
128129

‎src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/CompilerServices/RuntimeHelpers.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,8 @@ public static int OffsetToStringData
1818

1919
[Intrinsic]
2020
public static extern void InitializeArray(Array array, RuntimeFieldHandle fldHandle);
21+
22+
[Intrinsic]
23+
internal static void WriteBarrier(ref object? dst, object? obj) => dst = obj;
2124
}
2225
}

‎src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,6 @@ internal static class InternalCalls
121121
internal static extern unsafe object RhpNewFastMisalign(MethodTable * pEEType);
122122
#endif // FEATURE_64BIT_ALIGNMENT
123123

124-
[RuntimeImport(RuntimeLibrary, "RhpAssignRef")]
125-
[MethodImpl(MethodImplOptions.InternalCall)]
126-
internal static extern unsafe void RhpAssignRef(ref object? address, object? obj);
127-
128124
[MethodImplAttribute(MethodImplOptions.InternalCall)]
129125
[RuntimeImport(RuntimeLibrary, "RhpGcSafeZeroMemory")]
130126
internal static extern unsafe ref byte RhpGcSafeZeroMemory(ref byte dmem, nuint size);

0 commit comments

Comments
 (0)