Repository navigation
Creating a Span from a struct causes stack to be zeroed twice #1007
Description
Using MemoryMarshal.CreateSpan to create a Span<T> from a single struct instance T causes RyuJIT to emit code to zero the stack twice.
Here is a simple example:
using System.Runtime.InteropServices;
public class C {
[StructLayout(LayoutKind.Sequential, Size = 20)]
public struct Buffer
{
public ulong Field1;
public ulong Field2;
public uint Field3;
}
public void M() {
var buf = new Buffer();
var span = MemoryMarshal.CreateSpan(ref buf, 1);
span[0].Field1 = 1;
}
}This code has the following codegen on Core CLR v4.700.19.51502:
C.M()
L0000: sub rsp, 0x18
L0004: xor eax, eax
L0006: mov [rsp], rax
L000a: mov [rsp+0x8], rax
L000f: mov [rsp+0x10], rax
L0014: xor eax, eax
L0016: mov [rsp], rax
L001a: mov [rsp+0x8], rax
L001f: mov [rsp+0x10], eax
L0023: lea rax, [rsp]
L0027: mov qword [rax], 0x1
L002e: add rsp, 0x18
L0032: retInterestingly, specifying a fixed struct size using the StructLayout attribute (in the previous example, 20) causes the JIT to emit "less optimal" code. An example without the fixed struct size:
using System.Runtime.InteropServices;
public class C {
[StructLayout(LayoutKind.Sequential)]
public struct Buffer
{
public ulong Field1;
public ulong Field2;
public uint Field3;
}
public void M() {
var buf = new Buffer();
var span = MemoryMarshal.CreateSpan(ref buf, 1);
span[0].Field1 = 1;
}
}C.M()
L0000: sub rsp, 0x18
L0004: vzeroupper
L0007: xor eax, eax
L0009: mov [rsp], rax
L000d: mov [rsp+0x8], rax
L0012: mov [rsp+0x10], rax
L0017: xor eax, eax
L0019: lea rdx, [rsp]
L001d: vxorps xmm0, xmm0, xmm0
L0021: vmovdqu [rdx], xmm0
L0025: mov [rdx+0x10], rax
L0029: lea rax, [rsp]
L002d: mov qword [rax], 0x1
L0034: add rsp, 0x18
L0038: retWithout the fixed struct size, RyuJIT attempts to zero the stack the second time around using vector instructions. In this particular example, it doesn't gain much in performance (due to the small struct size), however you could imagine a larger struct would have a bigger performance benefit using this code gen over the naive approach.
category:cq
theme:prolog-epilog
skill-level:expert
cost:medium
Activity
jduncanator commentedon Dec 18, 2019
It seems the stack zeroing code is non-optimal in a number of different circumstances, regardless of using Span's.
Here, the stack is zeroed twice for no apparent reason:
And here, simply using a struct with a fixed size that results in padding causes it to be zeroed twice (funnily enough, it also causes none of the code to be optimized away, but that seems like an unrelated issue):
Here, the JIT decides to init the second and third fields using two different registers both holding
0x0(🤔):If not broken, it seems the code-gen for zeroing the stack is at least extremely inconsistent, and causes less than optimal code-gen the vast majority of the time.
(thanks to @gdkchan for spotting some of these as well)
sandreenko commentedon Dec 19, 2019
From the first look the second init block is generated by struct init, for example:
and this is expected,
but the first init happens in prolog, because the parent V01 has
lvMustInitflag set from:runtime/src/coreclr/src/jit/codegencommon.cpp
Lines 4633 to 4637 in 2755362
and we come there because
varDsc->lvTrackedis not set, because it is promoted.Possibly we need to check
varDsc->lvPromotedand do not setlvMustInitfor such variables.GitHub1007.txt
sandreenko commentedon Dec 19, 2019
PTAL @erozenfeld @dotnet/jit-contrib
jduncanator commentedon Dec 20, 2019
Just another example.
Here the stack space for the
intis cleared in the instruction directly before the instruction that moves0x2into the same location:It's starting to look like this behavior is triggered by taking the address of a value on the stack, perhaps the JIT needs to ensure the entire stack space consumed by that value is cleared before passing the address to another consumer?
sandreenko commentedon Dec 20, 2019
Thanks @jduncanator for the issue and so good repro cases.
Yes, in general case we always need to zero-init such locals, but probably soon we will have an option to disable that, check https://github.com/dotnet/csharplang/blob/master/proposals/skip-localsinit.md
But the double init cases could be fixed now.
AndyAyersMS commentedon May 6, 2020
@erozenfeld going to mark this as 5.0 since you are planning to look into this area.
CarolEidt commentedon May 14, 2020
I'm not sure whether that is the right strategy. If we do that, we need to ensure that
lvMustInitis set, as needed, on its fields, which won't be tracked if the variable is markedlvDoNotEnregister. I think another approach would be to effectively "un-promote" a variable when it is markedlvDoNotEnregister, and allow its parent lclVar to once again become tracked. That would allow us to eliminate dead inits (and other dead stores).I believe there's a separate issue that occurs when the register allocator enregisters a promoted field that's being set to zero. If that field is then only used to copy to another field on the stack (or if it is spilled) we can wind up creating multiple zeros in registers.
There are a number of instances of these kinds of issues in the frameworks.
A couple to look at to see if they can be addressed by the same fix(es):
Microsoft.CodeAnalysis.VisualBasic.SyntaxFacts:BeginOfBlockStatementIfAny(Microsoft.CodeAnalysis.SyntaxNode):Microsoft.CodeAnalysis.SyntaxNodeIn Microsoft.CodeAnalysis.VisualBasic.dll. This has the "multiple copies of zero" problem (on x64, both Windows and Linux).Microsoft.Diagnostics.Tracing.Stacks.StackSourceInterner:FrameIntern(int,System.String):int:thisin Microsoft.Diagnostics.Tracing.TraceEvent.dll. This is a case where we demote a struct, and are then unable to eliminate the dead init because it is no longer tracked.erozenfeld commentedon May 14, 2020
I have a change that addresses all of these redundant zero intializations. I'll open a PR sometime next week. I'll also create a separate issue for multiple zeroes in registers.
erozenfeld commentedon May 27, 2020
@jduncanator Thank you for reporting these cases, they helped me with my work.
#36918 resolved most of the issues here. Here are the diffs for your examples:
Example 1:
G_M25775_IG01: 4883EC18 sub rsp, 24 - C5F877 vzeroupper C5D857E4 vxorps xmm4, xmm4 C5F97F2424 vmovdqa xmmword ptr [rsp], xmm4 33C0 xor rax, rax 4889442410 mov qword ptr [rsp+10H], rax G_M25775_IG02: - 33C0 xor eax, eax - C5F857C0 vxorps xmm0, xmm0 - C5FA7F0424 vmovdqu xmmword ptr [rsp], xmm0 - 89442410 mov dword ptr [rsp+10H], eax 488D0424 lea rax, bword ptr [rsp] 48C70001000000 mov qword ptr [rax], 1 G_M25775_IG03: 4883C418 add rsp, 24 C3 ret -; Total bytes of code 54, prolog size 23, PerfScore 14.97, (MethodHash=0c929b50) for method C:M() +; Total bytes of code 36, prolog size 20, PerfScore 9.38, (MethodHash=0c929b50) for method C:M()Example 2:
G_M25775_IG01: 4883EC18 sub rsp, 24 - C5F877 vzeroupper C5D857E4 vxorps xmm4, xmm4 C5F97F2424 vmovdqa xmmword ptr [rsp], xmm4 33C0 xor rax, rax 4889442410 mov qword ptr [rsp+10H], rax G_M25775_IG02: - 33C0 xor eax, eax - C5F857C0 vxorps xmm0, xmm0 - C5FA7F0424 vmovdqu xmmword ptr [rsp], xmm0 - 4889442410 mov qword ptr [rsp+10H], rax 488D0424 lea rax, bword ptr [rsp] 48C70001000000 mov qword ptr [rax], 1 G_M25775_IG03: 4883C418 add rsp, 24 C3 ret -; Total bytes of code 55, prolog size 23, PerfScore 15.07, (MethodHash=0c929b50) for method C:M() +; Total bytes of code 36, prolog size 20, PerfScore 9.38, (MethodHash=0c929b50) for method C:M()Example 3:
G_M25775_IG01: 4883EC38 sub rsp, 56 - C5F877 vzeroupper C5D857E4 vxorps xmm4, xmm4 C5F97F642420 vmovdqa xmmword ptr [rsp+20H], xmm4 33C0 xor rax, rax 4889442430 mov qword ptr [rsp+30H], rax G_M25775_IG02: - 33C9 xor ecx, ecx - C5F857C0 vxorps xmm0, xmm0 - C5FA7F442420 vmovdqu xmmword ptr [rsp+20H], xmm0 - 894C2430 mov dword ptr [rsp+30H], ecx 48C744242002000000 mov qword ptr [rsp+20H], 2 488D4C2420 lea rcx, bword ptr [rsp+20H] E86DA9FFFF call C:DoSomethingWithMyType(byref) 90 nop G_M25775_IG03: 4883C438 add rsp, 56 C3 ret -; Total bytes of code 65, prolog size 24, PerfScore 17.32, (MethodHash=0c929b50) for method C:M() +; Total bytes of code 46, prolog size 21, PerfScore 11.63, (MethodHash=0c929b50) for method C:M()Example 4:
G_M25775_IG01: 4883EC18 sub rsp, 24 - C5F877 vzeroupper C5D857E4 vxorps xmm4, xmm4 C5F97F2424 vmovdqa xmmword ptr [rsp], xmm4 33C0 xor rax, rax 4889442410 mov qword ptr [rsp+10H], rax ;; bbWeight=1 PerfScore 3.83 G_M25775_IG02: - 33C0 xor eax, eax - C5F857C0 vxorps xmm0, xmm0 - C5FA7F0424 vmovdqu xmmword ptr [rsp], xmm0 - 4889442410 mov qword ptr [rsp+10H], rax 48C7042402000000 mov qword ptr [rsp], 2 G_M25775_IG03: 4883C418 add rsp, 24 C3 ret -; Total bytes of code 52, prolog size 23, PerfScore 14.27, (MethodHash=0c929b50) for method C:M() +; Total bytes of code 33, prolog size 20, PerfScore 8.58, (MethodHash=0c929b50) for method C:M()Ideally the jit should eliminate all code here. Currently here is what is happening:
Example 5:
There are no diffs for this case. I will open a separate issue to follow up on the usage of two registers for 0.
Example 6:
G_M25775_IG01: 4883EC28 sub rsp, 40 - 33C0 xor rax, rax - 4889442420 mov qword ptr [rsp+20H], rax G_M25775_IG02: C744242002000000 mov dword ptr [rsp+20H], 2 488D4C2420 lea rcx, [rsp+20H] E88BA9FFFF call C:Hey(byref) 90 nop G_M25775_IG03: 4883C428 add rsp, 40 C3 ret -; Total bytes of code 35, prolog size 11, PerfScore 9.00, (MethodHash=0c929b50) for method C:M() +; Total bytes of code 28, prolog size 4, PerfScore 7.05, (MethodHash=0c929b50) for method C:M()erozenfeld commentedon May 27, 2020
#37079 is a follow-up issue for Example 5 (multiple zeroes in registers).
erozenfeld commentedon Jun 9, 2020
#36918 and #37280 addressed all issues in these examples except the multiple registers used to hold zero. The latter issue is tracked by #37079 so I'm closing this one.