Inline CORINFO_HELP_ARRADDR_ST helper call, remove WriteBarrier FCall#117583
Inline CORINFO_HELP_ARRADDR_ST helper call, remove WriteBarrier FCall#117583EgorBo wants to merge 23 commits intodotnet:mainfrom
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Right, you would have to fix the JIT to be able to prove this in order to make this change without introducing regressions.
The barrier needs to be made checked when executed against stack allocated copy. It would be tough tradeoff to de-optimize all write barriers to enable more stack allocation. |
I believe I've already done that in this PR. The previous logic was a bit conservative - it tried to handle only |
|
Although, this one can't be made unchecked by the JIT due to NoInlining: 
And if I inline it, then StelemRef becomes too big and I was planning to inline it in the JIT (although, not sure, might be inlined in fact) |
|
Ugh. another problem is that JIT never does tail-calling for helper-calls 😢 
|
|
@MihuBot -arm |
|
@EgorBot -amd -arm -windows_intel using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
BenchmarkSwitcher.FromAssembly(typeof(Benchmarks).Assembly).Run(args);
public class Benchmarks
{
static object[] _strings = new string[4];
[Benchmark]
public void AssignString()
{
var arr = _strings;
if (arr.Length >= 4)
{
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 SingleAssignment()
{
_strings[0] = "";
}
} |
src/libraries/Common/tests/System/Runtime/CompilerServices/RuntimeHelpers.cs
Outdated
Show resolved
Hide resolved
…timeHelpers.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>
What's the code for this method with this change? There are two array store invariance checks in this method. One of them is optimized before this change. Is this change enabling the second one to be optimized (the improvement tracked by #9159)? I do not see what makes it possible. |
The codegen (win-x64) for this method is: G_M39739_IG01: ;; offset=0x0000
push rdi
push rsi
push rbp
push rbx
sub rsp, 40
mov rbx, rcx
mov esi, r8d
;; size=14 bbWeight=1 PerfScore 4.75
G_M39739_IG02: ;; offset=0x000E
mov edi, dword ptr [rbx+0x08]
cmp edx, edi
jae SHORT G_M39739_IG09
mov ecx, edx
mov rbp, gword ptr [rbx+8*rcx+0x10]
cmp esi, edi
jae SHORT G_M39739_IG09
mov edx, esi
mov rdx, gword ptr [rbx+8*rdx+0x10]
lea rcx, bword ptr [rbx |
|
I guess it's not entirely eliminated and the |
|
In order to eliminate the second invariant array store check, the JIT would have to figure out what the value came from the same array. Helper inlining won't help you with that. I do not think that this PR is fixing #9159 that's specifically about eliminating invariant array store checks in code that is swapping elements of an array. Invariant array store checks can be very expensive depending on the type of the array and type of the item being stored into the array. It is why it is interesting to eliminate them. |
|
(The idea behind this change sounds reasonable to me otherwise.) |
You're right I just misremembered that it did fix it when I was working on this. Changed the wording. |
Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
# Conflicts: # src/coreclr/jit/namedintrinsiclist.h # src/coreclr/vm/ecalllist.h # src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs
The WriteBarrier_Helper define and all callers were removed in this PR, so this PORTABILITY_ASSERT stub is now dead code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| case NI_System_Runtime_CompilerServices_RuntimeHelpers_WriteBarrier: | ||
| { | ||
| GenTree* val = impPopStack().val; | ||
| GenTree* dst = impPopStack().val; | ||
| retNode = gtNewStoreIndNode(TYP_REF, dst, val, GTF_IND_TGT_HEAP); | ||
| break; |
| } | ||
|
|
||
| InternalCalls.RhpAssignRef(ref element, obj); | ||
| RuntimeHelpers.WriteBarrier(ref element, obj); |
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
Linux-AMD (Genoa):
Linux-ARM64 (Cobalt100):