Suboptimal codegen for [value] vs new ReadOnlySpan<byte>(&value, 1)

[EgorBo]

I expect Test1 and Test2 to have exactly the same codegen in the following code snippet:

static unsafe void Test1(byte value) => Consume(new ReadOnlySpan<byte>(&value, 1));
static unsafe void Test2(byte value) => Consume([value]);

[MethodImpl(MethodImplOptions.NoInlining)]
public static void Consume(ReadOnlySpan<byte> span) {}

win-x64 codegen:

; Assembly listing for method Bench:Test1(ubyte) (FullOpts)
       sub      rsp, 56
       xor      eax, eax
       mov      qword ptr [rsp+0x28], rax
       mov      dword ptr [rsp+0x40], ecx
       lea      rcx, [rsp+0x40]
       mov      bword ptr [rsp+0x28], rcx
       mov      dword ptr [rsp+0x30], 1
       lea      rcx, [rsp+0x28]
       call     [Bench:Consume(System.ReadOnlySpan`1[ubyte])]
       nop      
       add      rsp, 56
       ret      
; Total bytes of code 50


; Assembly listing for method Bench:Test2(ubyte) (FullOpts)
       sub      rsp, 56
       xor      eax, eax
       mov      qword ptr [rsp+0x20], rax
       mov      qword ptr [rsp+0x28], rax
       movzx    rcx, cl
       mov      dword ptr [rsp+0x34], ecx
       lea      rcx, bword ptr [rsp+0x34]
       mov      bword ptr [rsp+0x20], rcx
       mov      dword ptr [rsp+0x28], 1
       lea      rcx, [rsp+0x20]
       call     [Bench:Consume(System.ReadOnlySpan`1[ubyte])]
       nop      
       add      rsp, 56
       ret      
; Total bytes of code 58

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[EgorBo]

I presume it could be fixed by a late forward sub (for loc2):

***** BB01 [0000]
STMT00000 ( 0x000[E-] ... 0x001 )
N003 (  7,  7) [000001] DA--G+-----                         *  STORE_LCL_VAR int   (AX) V01 loc0          $VN.Void
N002 (  3,  4) [000017] -----+-----                         \--*  CAST      int <- ubyte <- int $180
N001 (  2,  2) [000000] -----+-----                            \--*  LCL_VAR   int    V00 arg0         u:1 (last use) $80

STMT00002 ( 0x009[--] ... ??? )
N007 ( 32, 23) [000006] -ACXG------                         *  CALL      void   Bench:Consume(System.ReadOnlySpan`1[ubyte]) $VN.Void
N005 ( 12, 14) [000023] -A--G------ arg0 setup              +--*  COMMA     void   $VN.Void
N002 (  7,  8) [000020] UA--G------                         |  +--*  STORE_LCL_FLD byref (AX) V06 tmp4         [+0] $VN.Void
N001 (  3,  3) [000025] -----------                         |  |  \--*  LCL_ADDR  byref  V01 loc0         [+0]
N004 (  5,  6) [000022] UA--G------                         |  \--*  STORE_LCL_FLD int   (AX) V06 tmp4         [+8] $VN.Void
N003 (  1,  1) [000026] -----------                         |     \--*  CNS_INT   int    1 $45
N006 (  3,  3) [000024] ----------- arg0 rcx                \--*  LCL_ADDR  long   V06 tmp4         [+0] $201

cc @AndyAyersMS @jakobbotsch @dotnet/jit-contrib

[stephentoub]

reduce unsafe

new ReadOnlySpan<byte>(in value) produces the same quality as new ReadOnlySpan<byte>(&value, 1)

[EgorBo]

reduce unsafe

new ReadOnlySpan<byte>(in value) produces the same quality as new ReadOnlySpan<byte>(&value, 1)

but is not as cool as just [value], I wonder if analyzers already suggest rewriting to collection literals

[stephentoub]

but is not as cool as just [value]

It'd be great making the codegen for collection expressions better; nor arguing with that.

Just commenting on the "reduce unsafe" aspect, since new ReadOnlySpan<byte>(in value) is safe and has the same cq as the pointer-based one.

(Note that the cited collection expression will use the ctor I highlighted... it's just it ends up creating a copy of the local and taking a reference to that. Given it's an in, I'm not sure why the C# compiler feels compelled to copy it into a local.)

[teo-tsirpanis]

I'm not sure why the C# compiler feels compelled to copy it into a local.

I think in order to prevent the span's content from being modified. If you write new ReadOnlySpan(in value) and some other code changed value, the span's content will change, while the content of [value] should never change and hence the copy.

[stephentoub]

I'm not sure why the C# compiler feels compelled to copy it into a local.

I think in order to prevent the span's content from being modified. If you write new ReadOnlySpan(in value) and some other code changed value, the span's content will change, while the content of [value] should never change and hence the copy.

Yes, but nothing else reads from it after the call. I understand the reasoning in the general case, but not this one.