Creating a Span from a struct causes stack to be zeroed twice

[jduncanator]

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: ret

Interestingly, 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: ret

Without 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

[jduncanator]

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:

using System.Runtime.CompilerServices;
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();
        buf.Field1 = 2;
        
        DoSomethingWithMyType(ref buf);
    }
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    public void DoSomethingWithMyType(ref Buffer buf)
    {
        // Do Something.. to prevent the JIT from optimizing it all awayyyy
    }
}

C.M()
    L0000: sub rsp, 0x38
    L0004: xor eax, eax
    L0006: mov [rsp+0x20], rax
    L000b: mov [rsp+0x28], rax
    L0010: mov [rsp+0x30], rax
    L0015: xor edx, edx
    L0017: mov [rsp+0x20], rdx
    L001c: mov [rsp+0x28], rdx
    L0021: mov [rsp+0x30], edx
    L0025: mov qword [rsp+0x20], 0x2
    L002e: lea rdx, [rsp+0x20]
    L0033: call C.DoSomethingWithMyType(Buffer ByRef)
    L0038: nop
    L0039: add rsp, 0x38
    L003d: ret

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):

using System.Runtime.InteropServices;

public class C {
    [StructLayout(LayoutKind.Sequential, Size = 24)]
    public struct Buffer
    {
        public ulong Field1;
        public ulong Field2;
        public uint Field3;
    }
    
    public void M() {
        var buf = new Buffer();
        buf.Field1 = 2;
    }
}

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: mov qword [rsp], 0x2
    L0031: add rsp, 0x18
    L0035: ret

Here, the JIT decides to init the second and third fields using two different registers both holding 0x0 (🤔):

using System.Runtime.CompilerServices;
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();
        buf.Field1 = 2;
        
        DoSomethingWithMyType(buf);
    }
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    public void DoSomethingWithMyType(Buffer buf)
    {
        // Do Something.. to prevent the JIT from optimizing it all awayyyy
    }
}

C.M()
    L0000: sub rsp, 0x38
    L0004: xor edx, edx
    L0006: xor eax, eax
    L0008: mov r8d, 0x2
    L000e: lea r9, [rsp+0x20]
    L0013: mov [r9], r8
    L0016: mov [r9+0x8], rdx
    L001a: mov [r9+0x10], eax
    L001e: lea rdx, [rsp+0x20]
    L0023: call C.DoSomethingWithMyType(Buffer)
    L0028: nop
    L0029: add rsp, 0x38
    L002d: ret

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]

From the first look the second init block is generated by struct init, for example:

fgMorphTree BB01, STMT00000 (before)
               [000003] IA----------              *  ASG       struct (init)
               [000000] D------N----              +--*  LCL_VAR   struct<Buffer, 20>(AX)(P) V01 loc0         
                                                  +--*    long   V01.Field1 (offs=0x00) -> V03 tmp1         
                                                  +--*    long   V01.Field2 (offs=0x08) -> V04 tmp2         
                                                  +--*    int    V01.Field3 (offs=0x10) -> V05 tmp3         
               [000002] ------------              \--*  CNS_INT   int    0

fgMorphInitBlock: using field by field initialization.
fgMorphInitBlock (after):
               [000025] -A--G+------              *  COMMA     void  
               [000021] -A--G-------              +--*  COMMA     void  
               [000017] -A--G-------              |  +--*  ASG       long  
               [000015] D---G--N----              |  |  +--*  LCL_VAR   long  (AX) V03 tmp1         
               [000016] ------------              |  |  \--*  CNS_INT   long   0
               [000020] -A--G-------              |  \--*  ASG       long  
               [000018] D---G--N----              |     +--*  LCL_VAR   long  (AX) V04 tmp2         
               [000019] ------------              |     \--*  CNS_INT   long   0
               [000024] -A--G-------              \--*  ASG       int   
               [000022] D---G--N----                 +--*  LCL_VAR   int   (AX) V05 tmp3         
               [000023] ------------                 \--*  CNS_INT   int    0

and this is expected,
but the first init happens in prolog, because the parent V01 has lvMustInit flag set from:

runtime/src/coreclr/src/jit/codegencommon.cpp

Lines 4633 to 4637 in 2755362

	if ((!varDsc->lvTracked || (varDsc->lvType == TYP_STRUCT)) && varDsc->lvOnFrame &&
	(!varDsc->lvIsTemp || varDsc->HasGCPtr()))
	{
	varDsc->lvMustInit = true;

and we come there because varDsc->lvTracked is not set, because it is promoted.
Possibly we need to check varDsc->lvPromoted and do not set lvMustInit for such variables.
GitHub1007.txt

[sandreenko]

PTAL @erozenfeld @dotnet/jit-contrib

[jduncanator]

Just another example.

Here the stack space for the int is cleared in the instruction directly before the instruction that moves 0x2 into the same location:

using System.Runtime.CompilerServices;

public class C {
    public void M() {
        int buf = 2;
        
        Hey(ref buf);
    }
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    public void Hey(ref int buf)
    {
        // Prevent Optimizations
    }
}

C.M()
    L0000: sub rsp, 0x28
    L0004: xor eax, eax
    L0006: mov [rsp+0x20], rax
    L000b: mov dword [rsp+0x20], 0x2
    L0013: lea rdx, [rsp+0x20]
    L0018: call C.Hey(Int32 ByRef)
    L001d: nop
    L001e: add rsp, 0x28
    L0022: ret

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]

Thanks @jduncanator for the issue and so good repro cases.

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?

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]

@erozenfeld going to mark this as 5.0 since you are planning to look into this area.

[CarolEidt]

Possibly we need to check varDsc->lvPromoted and do not set lvMustInit for such variables.

I'm not sure whether that is the right strategy. If we do that, we need to ensure that lvMustInit is set, as needed, on its fields, which won't be tracked if the variable is marked lvDoNotEnregister. I think another approach would be to effectively "un-promote" a variable when it is marked lvDoNotEnregister, 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.SyntaxNode In 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:this in 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.