[JIT] Allow side-effects for the first test in switch recognition

[EgorBo]

static void Test(char ch)
{
    if (ch == '\r' || ch == '\n' || ch == '\t' || ch == ' ')
    {
        Console.WriteLine("special symbol");
    }
}

Codegen diff:

; Assembly listing for method Proga:Test(ushort) (FullOpts)
       push     rbx
       sub      rsp, 32
       movzx    rbx, cx
-      cmp      ebx, 13  ;; this "ch == '\r'" is supposed to be part of the bit-test below
-      je       SHORT G_M18843_IG05
       cmp      ebx, 32
       ja       SHORT G_M18843_IG06
-      mov      eax, 0xFFFFF9FF
+      mov      eax, 0xFFFFD9FF
       bt       rax, rbx
       jb       SHORT G_M18843_IG06
G_M18843_IG05:
       mov      rcx, 0x2127BF35718      ; 'special symbol'
       call     [System.Console:WriteLine(System.String)]
G_M18843_IG06:
       nop      
       add      rsp, 32
       pop      rbx
       ret      

JIT IR before the switch recognition:

---------------------------------------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight   [IL range]   [jump]                            [EH region]        [flags]
---------------------------------------------------------------------------------------------------------------------------------------------------------------------
BB01 [0000]  1                             1    [000..005)-> BB05(0.5),BB02(0.5)     ( cond )                     i
BB02 [0001]  1       BB01                  0.50 [005..00A)-> BB05(0.5),BB03(0.5)     ( cond )                     i
BB03 [0002]  1       BB02                  0.50 [00A..00F)-> BB05(0.5),BB04(0.5)     ( cond )                     i
BB04 [0003]  1       BB03                  0.50 [00F..014)-> BB06(0.5),BB05(0.5)     ( cond )                     i
BB05 [0004]  4       BB01,BB02,BB03,BB04   0.50 [014..01E)-> BB06(1)                 (always)                     i hascall gcsafe
BB06 [0005]  2       BB04,BB05             1    [01E..01F)                           (return)                     i
---------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------ BB01 [0000] [000..005) -> BB05(0.5),BB02(0.5) (cond), preds={} succs={BB02,BB05}

***** BB01 [0000]
STMT00000 ( 0x000[E-] ... 0x003 )
N008 (  8,  9) [000003] -A---+-----                         *  JTRUE     void   $VN.Void
N007 (  6,  7) [000002] JA---+-N---                         \--*  EQ        int    $101
N005 (  4,  5) [000026] -A---------                            +--*  COMMA     int    $100
N003 (  3,  4) [000024] DA---------                            |  +--*  STORE_LCL_VAR int    V02 cse0         d:1 $VN.Void
N002 (  3,  4) [000019] -----+-----                            |  |  \--*  CAST      int <- ushort <- int $100
N001 (  2,  2) [000000] -----+-----                            |  |     \--*  LCL_VAR   int    V00 arg0         u:1 $80
N004 (  1,  1) [000025] -----------                            |  \--*  LCL_VAR   int    V02 cse0         u:1 $100
N006 (  1,  1) [000001] -----+-----                            \--*  CNS_INT   int    13 $43

------------ BB02 [0001] [005..00A) -> BB05(0.5),BB03(0.5) (cond), preds={BB01} succs={BB03,BB05}

***** BB02 [0001]
STMT00003 ( 0x005[E-] ... 0x008 )
N004 (  5,  5) [000010] -----+-----                         *  JTRUE     void   $VN.Void
N003 (  3,  3) [000009] J----+-N---                         \--*  EQ        int    $102
N001 (  1,  1) [000027] -----------                            +--*  LCL_VAR   int    V02 cse0         u:1 $100
N002 (  1,  1) [000008] -----+-----                            \--*  CNS_INT   int    10 $42

------------ BB03 [0002] [00A..00F) -> BB05(0.5),BB04(0.5) (cond), preds={BB02} succs={BB04,BB05}

***** BB03 [0002]
STMT00004 ( 0x00A[E-] ... 0x00D )
N004 (  5,  5) [000014] -----+-----                         *  JTRUE     void   $VN.Void
N003 (  3,  3) [000013] J----+-N---                         \--*  EQ        int    $103
N001 (  1,  1) [000028] -----------                            +--*  LCL_VAR   int    V02 cse0         u:1 $100
N002 (  1,  1) [000012] -----+-----                            \--*  CNS_INT   int    9 $44

------------ BB04 [0003] [00F..014) -> BB06(0.5),BB05(0.5) (cond), preds={BB03} succs={BB05,BB06}

***** BB04 [0003]
STMT00005 ( 0x00F[E-] ... 0x012 )
N004 (  5,  5) [000018] -----+-----                         *  JTRUE     void   $VN.Void
N003 (  3,  3) [000017] N----+-N-U-                         \--*  NE        int    $104
N001 (  1,  1) [000029] -----------                            +--*  LCL_VAR   int    V02 cse0         u:1 $100
N002 (  1,  1) [000016] -----+-----                            \--*  CNS_INT   int    32 $45

------------ BB05 [0004] [014..01E) -> BB06(1) (always), preds={BB01,BB02,BB03,BB04} succs={BB06}

***** BB05 [0004]
STMT00001 ( 0x014[E-] ... 0x019 )
N002 ( 17, 16) [000005] --CXG+-----                         *  CALL      void   System.Console:WriteLine(System.String) $VN.Void
N001 (  3, 10) [000023] H----+----- arg0 in rcx             \--*  CNS_INT(h) ref     'special symbol' $140

------------ BB06 [0005] [01E..01F) (return), preds={BB04,BB05} succs={}

***** BB06 [0005]
STMT00002 ( 0x01E[E-] ... 0x01E )
N001 (  0,  0) [000006] -----+-----                         *  RETURN    void   $VN.Void

-------------------------------------------------------------------------------------------------------------------

We used to give up on BB01 due to COMMA, but in fact we shouldn't since we preserve the first block for the resulting switch.

Alternative fix: run "remove all commas" phase 🙂 SplitTreesRemoveCommas

[dotnet-policy-service]

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

[EgorBo]

@EgorBot -intel -keep --envvars DOTNET_JitDisasm:BitTest DOTNET_JitStdOutFile:jitdisasm.asm

using System;
using System.Runtime.CompilerServices;
using BenchmarkDotNet.Attributes;

public class Bencha
{
    string test = "qwertyuiopasdfghjjklzxcvbnmroigjeroijgiroejg";

    [Benchmark]
    public int Bench()
    {
        int count = 0;
        foreach (var ch in test)
        {
            if (BitTest(ch))
                count++;
        }
        return count;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static bool BitTest(char ch)
    {
        if (ch == 'a' || ch == 'c' || ch == 'n' || ch == 'h' || ch == 'z')
            return true;
        return false;
    }
}

[EgorBot]

Benchmark results on Intel

BenchmarkDotNet v0.14.0, Ubuntu 22.04.4 LTS (Jammy Jellyfish)
Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 16 logical and 8 physical cores
  Job-TOPTAN : .NET 10.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  Job-TUUGXJ : .NET 10.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
EnvironmentVariables=DOTNET_JitDisasm=BitTest,DOTNET_JitStdOutFile=jitdisasm.asm

Method	Toolchain	Mean	Error	Ratio
Bench	Main	71.28 ns	0.044 ns	1.00
Bench	PR	69.96 ns	0.010 ns	0.98

BDN_Artifacts.zip

[JulieLeeMSFT]

Is there any more side effect to support other than COMMA? If not, please remove the TODO comment becasue this PR will support COMMA side-effect.

[EgorBo]

Removed

[JulieLeeMSFT]

LGTM.

[JulieLeeMSFT]

Please backport to .NET 9 RC2.

[jakobbotsch]

Why do we clone nodeToTest below? Initially it seemed like we were cloning the side effects too, but it looks like we discard the old version, so no cloning would be necessary.

[EgorBo]

I think this clone was an artifact from the initial impl, I have removed it in the latest commit.

[jakobbotsch]

I think it looks fine.. may want to double check that we aren't cloning any side effect.

It's odd to me that this transformation depends on lexical order of the basic blocks. That seems unnecessary and something we have been trying to move away from. (Edit: Opened #107076 for this)

[EgorBo]

I think it looks fine.. may want to double check that we aren't cloning any side effect.

Fixed.

Please backport to .NET 9 RC2.

@JulieLeeMSFT are you sure this needs a backport? the perf impact seems to be minimal (around 1-2% in my benchmarks). or this needed simply for marketing purposes? cc @stephentoub