JIT - slow generated code on Release for iterating simple array of struct

[krwq]

Byte array iteration code:

for (int i = 0; i < Iterations; i++)
{
    unchecked
    {
        for (int j = 0; j < ArraySize; j++)
        {
            arr[j] = 123;
        }
    }
}

Struct iteration code

[StructLayout(LayoutKind.Sequential, Pack = 1, Size = 1)]
struct MyByte
{
    private readonly byte _byte;
    public MyByte(byte b)
    {
        _byte = b;
    }

    public byte Value
    {
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        get { return _byte; }
    }
}
// ...
for (int i = 0; i < Iterations; i++)
{
    unchecked
    {
        for (int j = 0; j < ArraySize; j++)
        {
            arr[j] = new MyByte(123);
        }
    }
}

Generated code (byte array):

                    for (int j = 0; j < ArraySize; j++)
00523003  xor         edx,edx  
00523005  mov         ecx,dword ptr [esi+4]  
                    {
                        arr[j] = 123;
00523008  cmp         edx,ecx  
0052300A  jae         0052304F  
0052300C  mov         byte ptr [esi+edx+8],7Bh  
                    for (int j = 0; j < ArraySize; j++)
00523011  inc         edx  
00523012  cmp         edx,0F4240h  
00523018  jl          00523008  
            for (int i = 0; i < Iterations; i++)
0052301A  inc         edi  
0052301B  cmp         edi,7D0h  
00523021  jl          00523003  
                    }

Generated code for struct - (expecting similar generated code - this is the simplest optimization you can think of here, I even provided struct size and readonly to make it easier)

                    for (int j = 0; j < ArraySize; j++)
00522F0B  xor         edx,edx  
00522F0D  mov         ecx,dword ptr [edi+4]  
00522F10  mov         ebx,7Bh  
                    {
                        arr[j] = new MyByte(123);
00522F15  cmp         edx,ecx  
00522F17  jae         00522F63  
00522F19  lea         esi,[edi+edx+8]  
00522F1D  mov         byte ptr [esi],bl  
                    for (int j = 0; j < ArraySize; j++)
00522F1F  inc         edx  
00522F20  cmp         edx,0F4240h  
00522F26  jl          00522F10  
            for (int i = 0; i < Iterations; i++)
00522F28  inc         dword ptr [ebp-14h]  
00522F2B  cmp         dword ptr [ebp-14h],7D0h  
00522F32  jl          00522F0B  
                    }

This is more than 30% slower and this is just a simplest case.

category:cq
theme:assertion-prop
skill-level:expert
cost:extra-large

[mikedn]

This is 32 bit code, it's not generated by RyuJIT.

For byte[] RyuJIT generates:

G_M32464_IG08:
       448B4008             mov      r8d, dword ptr [rax+8]
       413BC8               cmp      ecx, r8d
       7323                 jae      SHORT G_M32464_IG11
       4C63C1               movsxd   r8d, ecx
       42C64400107B         mov      byte  ptr [rax+r8+16], 123
       FFC1                 inc      ecx
       81F940420F00         cmp      ecx, 0xF4240
       7CE4                 jl       SHORT G_M32464_IG08

For MyByte[] RyuJIT generates:

G_M32464_IG04:
       41B87B000000         mov      r8d, 123
       448B4808             mov      r9d, dword ptr [rax+8]
       413BD1               cmp      edx, r9d
       7361                 jae      SHORT G_M32464_IG11
       4C63CA               movsxd   r9d, edx
       4688440810           mov      byte  ptr [rax+r9+16], r8b
       FFC2                 inc      edx
       81FA40420F00         cmp      edx, 0xF4240
       7CDF                 jl       SHORT G_M32464_IG04

The 2 versions have very similar codegen but even so the MyByte version is still some ~10% slower, probably due to the additional mov r8d, 123.

I even provided struct size and readonly to make it easier

That makes no difference. So does the use of unchcked.

[cmckinsey]

@krwq Thanks for the report. And @mikedn thanks for the analysis. There is definitely room for improvement. While this test case is just a synthetic example, it is actually representative of many CPU intensive inner loops that we care about.

I believe most of these are things the team is aware of. If I can summarize:

• Hoist invariant array-length loads out of loop
• Hoist the bounds-check out of the loop and rewrite in terms of IV-extent. Because there is a global side-effect in the loop visible up the call-stack, the possibility of exception means we must 2-version the loop to do this.
• Constant-propagation field assignment through struct-fields
• IV-widening of induction variables (this eliminates movsxd r9d, edx)
• Counted-loop generation (Downcount the loop and use dec/jne for the test)
• Depending upon the count (constant or symbolic), we'd likely want to unroll this loop.

However we'll leave this issue open to cover solely the difference between the scalar and struct version and issues around handling struct fields as well as scalars for opts. @CarolEidt has been looking into some improvements recently in this area.

[GSPP]

I think this benchmark highlights an architectural issue: Structs should not be inherently different from other primitives.

It might be architecturally necessary to treat them differently but any difference is a smell in my opinion.

[CarolEidt]

Looking at the dump, it appears that this is actually an optimization issue (not a struct issue). After value numbering, we have the following for the assignment to the MyByte array:

***** BB02, stmt 5 (after)
N017 (  2,  2) [000084] -------N----              ┌──▌  LCL_VAR   ubyte  V04 tmp3         u:3 (last use) $c5
N018 ( 25, 29) [000085] -A-XG-------              ▌  ASG       ubyte  $VN.Void
N016 ( 22, 26) [000083] *--XG--N----              └──▌  IND       ubyte  $c5
N014 (  1,  1) [000081] ------------                 │  ┌──▌  CNS_INT   long   0 Fseq[_byte] $442
N015 ( 20, 25) [000082] ---XG--N----                 └──▌  ADD       byref  $406
N012 ( 10, 12) [000073] ---XG-------                    │  ┌──▌  ADDR      byref  $81
N011 (  6,  7) [000074] a--XG-------                    │  │  └──▌  IND       struct <l:$480, c:$4c0>
N009 (  1,  1) [000080] ------------                    │  │     │  ┌──▌  CNS_INT   long   16 Fseq[#FirstElem] $440
N010 (  4,  5) [000075] -------N----                    │  │     └──▌  ADD       byref  $81
N007 (  2,  3) [000078] ------------                    │  │        │  ┌──▌  CAST      long <- int $3c0
N006 (  1,  1) [000079] i-----------                    │  │        │  │  └──▌  LCL_VAR   int    V00 loc0         u:4 $2c0
N008 (  3,  4) [000076] -------N----                    │  │        └──▌  ADD       byref  <l:$401, c:$400>
N005 (  1,  1) [000077] ------------                    │  │           └──▌  LCL_VAR   ref    V03 tmp2         u:3 (last use) <l:$1c1, c:$301>
N013 ( 18, 23) [000068] ---XG-------                    └──▌  COMMA     byref  <l:$405, c:$404>
N004 (  8, 11) [000072] ---X--------                       └──▌  ARR_BOUNDS_CHECK_Rng void   <l:$385, c:$384>
N001 (  1,  1) [000071] ------------                          ├──▌  LCL_VAR   int    V00 loc0         u:4 $2c0
N003 (  3,  3) [000069] ---X--------                          └──▌  ARR_LENGTH int    <l:$242, c:$241>
N002 (  1,  1) [000070] ------------                             └──▌  LCL_VAR   ref    V03 tmp2         u:3 <l:$1c1, c:$301>

where $c5 is the value number for the constant 123 but it seems that assertion propagation fails to replace LCL_VAR ubyte V04 with the constant.

[mikedn]

it seems that assertion propagation fails to replace LCL_VAR ubyte V04 with the constant.

There are 2 things that block this optimization. One is optVNConstantPropCurStmt checking for GTF_DONT_CSE on the lclvar node. The other is optVNConstantPropOnTree failure to handle small types.

The later seems easy to fix but I'm not sure what's up with the former - I did not check why GTF_DONT_CSE was set on that node and it's not readily obvious why constant propagation is blocked in this case.

[mikedn]

GTF_DONT_CSE seems to be set here: https://github.com/dotnet/coreclr/blob/ba110d51de1994dd5f1a38e224b442dd1609d24f/src/jit/morph.cpp#L11249-l11251

[CarolEidt]

@mikedn - thanks for tracking that down! That's presumably one of the vestiges of the IR changes I made to represent struct assignments with GT_ASG instead of GT_COPYBLK et al, while trying not to yet change the code being generated.

[SingleAccretion]

In .NET 8 (and 7) the code for both loops is identical.

VN-based constant propagation for small-typed locals in particular was enabled in #57726.