JIT: simple benchmark highlights some CQ issues

[AndyAyersMS]

Here are some simple methods where codegen could be improved. Original from aspnet/SignalR#1900.

    [MethodImpl(MethodImplOptions.NoInlining)]
    private bool InMethod(in bool? value1, in bool? value2, in bool? value3)
    {
        return value1.HasValue && value2.HasValue && value3.HasValue;
    }
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    private bool NormalMethod(bool? value1, bool? value2, bool? value3)
    {
        return value1.HasValue && value2.HasValue && value3.HasValue;
    }
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    private bool InMethod(in bool value1, in bool value2, in bool value3)
    {
        return value1 && value2 && value3;
    }
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    private bool NormalMethod(bool value1, bool value2, bool value3)
    {
        return value1 && value2 && value3;
    }

current codegen below

; Assembly listing for method P:InMethod(byref,byref,byref):bool:this
; Emitting BLENDED_CODE for X64 CPU with AVX
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;* V00 this         [V00    ] (  0,  0   )     ref  ->  zero-ref    this class-hnd
;  V01 arg1         [V01,T00] (  4,  4   )   byref  ->  rdx
;  V02 arg2         [V02,T01] (  4,  3   )   byref  ->   r8
;  V03 arg3         [V03,T02] (  4,  3   )   byref  ->   r9
;* V04 loc0         [V04    ] (  0,  0   )  struct ( 8) zero-ref    ld-addr-op
;  V05 tmp0         [V05,T03] (  6,  4   )    bool  ->  rax         V04.hasValue(offs=0x00) P-INDEP
;* V06 tmp1         [V06    ] (  0,  0   )    bool  ->  zero-ref    V04.value(offs=0x01) P-INDEP
;# V07 OutArgs      [V07    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]
;
; Lcl frame size = 0

G_M21426_IG01:

G_M21426_IG02:
       0FB602               movzx    rax, byte  ptr [rdx]
       0FB65201             movzx    rdx, byte  ptr [rdx+1]  // not needed
       84C0                 test     al, al
       741A                 je       SHORT G_M21426_IG04
       410FB600             movzx    rax, byte  ptr [r8]
       410FB65001           movzx    rdx, byte  ptr [r8+1]   // not needed
       84C0                 test     al, al
       740D                 je       SHORT G_M21426_IG04
       410FB601             movzx    rax, byte  ptr [r9]
       410FB65101           movzx    rdx, byte  ptr [r9+1]   // not needed
       0FB6C0               movzx    rax, al

G_M21426_IG03:
       C3                   ret

G_M21426_IG04:
       33C0                 xor      eax, eax

G_M21426_IG05:
       C3                   ret

; Total bytes of code 40, prolog size 0 for method P:InMethod(byref,byref,byref):bool:this
; ============================================================
; Assembly listing for method P:NormalMethod(struct,struct,struct):bool:this
; Emitting BLENDED_CODE for X64 CPU with AVX
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;* V00 this         [V00    ] (  0,  0   )     ref  ->  zero-ref    this class-hnd
;  V01 arg1         [V01,T00] (  3,  3   )  struct ( 8) [rsp+0x10]   do-not-enreg[SF] ld-addr-op
;  V02 arg2         [V02,T01] (  3,  2.50)  struct ( 8) [rsp+0x18]   do-not-enreg[SF] ld-addr-op
;  V03 arg3         [V03,T02] (  3,  2.50)  struct ( 8) [rsp+0x20]   do-not-enreg[SF] ld-addr-op
;# V04 OutArgs      [V04    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]
;
; Lcl frame size = 0

G_M43454_IG01:
       4889542410           mov      qword ptr [rsp+10H], rdx   // could avoid these stores
       4C89442418           mov      qword ptr [rsp+18H], r8
       4C894C2420           mov      qword ptr [rsp+20H], r9

G_M43454_IG02:
       807C241000           cmp      byte  ptr [rsp+10H], 0   // could be [rdx]
       740D                 je       SHORT G_M43454_IG04
       807C241800           cmp      byte  ptr [rsp+18H], 0   // could be [r8]
       7406                 je       SHORT G_M43454_IG04
       0FB6442420           movzx    rax, byte  ptr [rsp+20H]   // could be [r9]

G_M43454_IG03:
       C3                   ret

G_M43454_IG04:
       33C0                 xor      eax, eax

G_M43454_IG05:
       C3                   ret

; Total bytes of code 38, prolog size 0 for method P:NormalMethod(struct,struct,struct):bool:this
; ============================================================
; Assembly listing for method P:InMethod(byref,byref,byref):bool:this
; Emitting BLENDED_CODE for X64 CPU with AVX
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;* V00 this         [V00    ] (  0,  0   )     ref  ->  zero-ref    this class-hnd
;  V01 arg1         [V01,T00] (  3,  3   )   byref  ->  rdx
;  V02 arg2         [V02,T01] (  3,  3   )   byref  ->   r8
;  V03 arg3         [V03,T02] (  3,  3   )   byref  ->   r9
;# V04 OutArgs      [V04    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]
;
; Lcl frame size = 0

G_M21423_IG01:

G_M21423_IG02:
       0FB602               movzx    rax, byte  ptr [rdx]
       410FB610             movzx    rdx, byte  ptr [r8]
       23C2                 and      eax, edx
       410FB611             movzx    rdx, byte  ptr [r9]
       23C2                 and      eax, edx

G_M21423_IG03:
       C3                   ret

; Total bytes of code 16, prolog size 0 for method P:InMethod(byref,byref,byref):bool:this
; ============================================================
; Assembly listing for method P:NormalMethod(bool,bool,bool):bool:this
; Emitting BLENDED_CODE for X64 CPU with AVX
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;* V00 this         [V00    ] (  0,  0   )     ref  ->  zero-ref    this class-hnd
;  V01 arg1         [V01,T00] (  3,  3   )    bool  ->  rdx
;  V02 arg2         [V02,T01] (  3,  3   )    bool  ->   r8
;  V03 arg3         [V03,T02] (  3,  3   )    bool  ->   r9
;# V04 OutArgs      [V04    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]
;
; Lcl frame size = 0

G_M43451_IG01:

G_M43451_IG02:
       0FB6C2               movzx    rax, dl    // could avoid widening here
       410FB6D0             movzx    rdx, r8b   // and here
       23C2                 and      eax, edx
       410FB6D1             movzx    rdx, r9b   // and here
       23C2                 and      eax, edx
       0FB6C0               movzx    rax, al

G_M43451_IG03:
       C3                   ret

; Total bytes of code 19, prolog size 0 for method P:NormalMethod(bool,bool,bool):bool:this

This last methods codegen would ideally be:

mov al, dl
and al, r8b
and al, r9b
movzx, rax, al
ret

category:cq
theme:basic-cq
skill-level:intermediate
cost:medium

[mikedn]

First InMethod produces the following code for me:

G_M33477_IG01:
G_M33477_IG02:
       803A00               cmp      byte  ptr [rdx], 0
       740B                 je       SHORT G_M33477_IG04
       41803800             cmp      byte  ptr [r8], 0
       7405                 je       SHORT G_M33477_IG04
       410FB601             movzx    rax, byte  ptr [r9]
G_M33477_IG03:
       C3                   ret
G_M33477_IG04:
       33C0                 xor      eax, eax
G_M33477_IG05:
       C3                   ret

I wonder if there are differences between the C# compiler we are using.

For first NormalMethod your suggested use of [rdx] & co. is incorrect, these structs are not passed by ref, implicit or not. So the ideal code would be something like:

G_M43454_IG01:
G_M43454_IG02:
       807C241000           test     dl, dl
       740D                 je       SHORT G_M43454_IG04
       807C241800           test     r8l, r8l
       7406                 je       SHORT G_M43454_IG04
       0FB6442420           movzx    rax, r9l
G_M43454_IG03:
       C3                   ret
G_M43454_IG04:
       33C0                 xor      eax, eax
G_M43454_IG05:
       C3                   ret

The question is what the code should be for accessing Value rather than HasValue. Shift + movzx?

    mov eax, edx
    shr eax, 8
    movzx eax, al

Or just stick to going through memory? It's going to be tricky to get this one right.

For the second InMethod you probably expect it to produce:

       0FB602               movzx    rax, byte  ptr [rdx]
       23C2                 and      al, byte  ptr [r8]
       23C2                 and      al, byte  ptr [r9]

Both this and the last NormalMethod are problematic to optimize due to the fact that the JIT tends to use TYP_INT for everything and small integral types are only a sideshow. AFAIK there are only 2 places in the JIT were nodes are narrowed to TYP_UBYTE (or similar):

• LowerCompare sometimes narrows its operands for various reasons. This may result in small type AND/OR/XOR nodes being produced. I suppose cast lowering could do something similar but it's not clear if lowering is the best place for this. Maybe it is as it's a machine specific optimization and minimizes the chances that other parts of the JIT get to see these small type nodes.
• fgMorphSmpOp changes relop nodes to TYP_BYTE when their result is stored via a byte indir. Funnily enough, the same fgMorphSmpOp has to undo this transform if relop gets CSEd, storing the result of a byte sized node into a lclvar doesn't work so well. IMO this particular transform belongs in lowering.

[AndyAyersMS]

I typically use the csc from the repo, which reports as 2.6.0.62316 (fbe99784), and compiles /o+ /debug:pdbonly (and here /langversion:latest to unblock in).

[mikedn]

Right, I'm using the csc that's installed by Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\Roslyn\csc.exe and that one is 2.7. I tried the one from the repo but I get 2.8. I downloaded 2.6 from NuGet and the IL is indeed very different:

   .method private hidebysig instance bool 
-          Test(valuetype [System.Private.CoreLib]System.Nullable`1<bool>& value1,
-               valuetype [System.Private.CoreLib]System.Nullable`1<bool>& value2,
-               valuetype [System.Private.CoreLib]System.Nullable`1<bool>& value3) cil managed noinlining
+          Test([in] valuetype [System.Private.CoreLib]System.Nullable`1<bool>& value1,
+               [in] valuetype [System.Private.CoreLib]System.Nullable`1<bool>& value2,
+               [in] valuetype [System.Private.CoreLib]System.Nullable`1<bool>& value3) cil managed noinlining
   {
     .param [1]
     .custom instance void [System.Private.CoreLib]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) 
     .param [2]
     .custom instance void [System.Private.CoreLib]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) 
     .param [3]
     .custom instance void [System.Private.CoreLib]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) 
-    // Code size       49 (0x31)
-    .maxstack  1
-    .locals init (valuetype [System.Private.CoreLib]System.Nullable`1<bool> V_0)
+    // Code size       25 (0x19)
+    .maxstack  8
     IL_0000:  ldarg.1
-    IL_0001:  ldobj      valuetype [System.Private.CoreLib]System.Nullable`1<bool>
-    IL_0006:  stloc.0
+    IL_0001:  call       instance bool valuetype [System.Private.CoreLib]System.Nullable`1<bool>::get_HasValue()
+    IL_0006:  brfalse.s  IL_0017
 
-    IL_0007:  ldloca.s   V_0
+    IL_0008:  ldarg.2
     IL_0009:  call       instance bool valuetype [System.Private.CoreLib]System.Nullable`1<bool>::get_HasValue()
-    IL_000e:  brfalse.s  IL_002f
+    IL_000e:  brfalse.s  IL_0017
 
-    IL_0010:  ldarg.2
-    IL_0011:  ldobj      valuetype [System.Private.CoreLib]System.Nullable`1<bool>
-    IL_0016:  stloc.0
-    IL_0017:  ldloca.s   V_0
-    IL_0019:  call       instance bool valuetype [System.Private.CoreLib]System.Nullable`1<bool>::get_HasValue()
-    IL_001e:  brfalse.s  IL_002f
+    IL_0010:  ldarg.3
+    IL_0011:  call       instance bool valuetype [System.Private.CoreLib]System.Nullable`1<bool>::get_HasValue()
 
-    IL_0020:  ldarg.3
-    IL_0021:  ldobj      valuetype [System.Private.CoreLib]System.Nullable`1<bool>
-    IL_0026:  stloc.0
-    IL_0027:  ldloca.s   V_0
-    IL_0029:  call       instance bool valuetype [System.Private.CoreLib]System.Nullable`1<bool>::get_HasValue()
-    IL_002e:  ret
+    IL_0016:  ret
 
-    IL_002f:  ldc.i4.0
-    IL_0030:  ret
+    IL_0017:  ldc.i4.0
+    IL_0018:  ret
   } // end of method X::Test

[roterdam]

Seems like a very bad code-size regression from Roslyn. Is this expected? I thought Roslyn was mostly a C# -> IL converter ... didn't through fancy things.

[mikedn]

Hmm? The IL code size improved from 2.6 to 2.7. Basically they started treating Nullable<T> as readonly and elided unnecessary copies.

[AndyAyersMS]

.NET 9 codegen:

; Assembly listing for method P:InMethod(byref,byref,byref):ubyte:this (FullOpts)
; Emitting BLENDED_CODE for X64 with AVX - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 3 single block inlinees; 0 inlinees without PGO data

G_M000_IG01:                ;; offset=0x0000
 
G_M000_IG02:                ;; offset=0x0000
       cmp      byte  ptr [rdx], 0
       je       SHORT G_M000_IG04
 
G_M000_IG03:                ;; offset=0x0005
       cmp      byte  ptr [r8], 0
       jne      SHORT G_M000_IG06
 
G_M000_IG04:                ;; offset=0x000B
       xor      eax, eax
 
G_M000_IG05:                ;; offset=0x000D
       ret      
 
G_M000_IG06:                ;; offset=0x000E
       movzx    rax, byte  ptr [r9]
 
G_M000_IG07:                ;; offset=0x0012
       ret  

; Assembly listing for method P:NormalMethod(System.Nullable`1[ubyte],System.Nullable`1[ubyte],System.Nullable`1[ubyte]):ubyte:this (FullOpts)
; Emitting BLENDED_CODE for X64 with AVX - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 3 single block inlinees; 0 inlinees without PGO data

G_M000_IG01:                ;; offset=0x0000
       mov      dword ptr [rsp+0x10], edx
       mov      dword ptr [rsp+0x18], r8d
       mov      dword ptr [rsp+0x20], r9d
 
G_M000_IG02:                ;; offset=0x000E
       cmp      byte  ptr [rsp+0x10], 0
       je       SHORT G_M000_IG04
 
G_M000_IG03:                ;; offset=0x0015
       cmp      byte  ptr [rsp+0x18], 0
       jne      SHORT G_M000_IG06
 
G_M000_IG04:                ;; offset=0x001C
       xor      eax, eax
 
G_M000_IG05:                ;; offset=0x001E
       ret      
 
G_M000_IG06:                ;; offset=0x001F
       movzx    rax, byte  ptr [rsp+0x20]
 
G_M000_IG07:                ;; offset=0x0024
       ret  

; Assembly listing for method P:InMethod(byref,byref,byref):ubyte:this (FullOpts)
; Emitting BLENDED_CODE for X64 with AVX - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data

G_M000_IG01:                ;; offset=0x0000
 
G_M000_IG02:                ;; offset=0x0000
       movzx    rax, byte  ptr [rdx]
       movzx    rcx, byte  ptr [r8]
       and      eax, ecx
       movzx    rcx, byte  ptr [r9]
       and      eax, ecx
 
G_M000_IG03:                ;; offset=0x000F
       ret 

; Assembly listing for method P:NormalMethod(ubyte,ubyte,ubyte):ubyte:this (FullOpts)
; Emitting BLENDED_CODE for X64 with AVX - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data

G_M000_IG01:                ;; offset=0x0000
 
G_M000_IG02:                ;; offset=0x0000
       movzx    rax, dl
       movzx    rcx, r8b
       and      eax, ecx
       and      eax, r9d
       movzx    rax, al
 
G_M000_IG03:                ;; offset=0x000F
       ret

The second method's code hasn't changed, we have to spill the argument to look at a piece of it. I think this might be something @jakobbotsch was interested in fixing.