Enable eliding some bounds checks in the jit by nathan-moore · Pull Request #36263 · dotnet/runtime (original) (raw)

Overall it looks good, thank you @nathan-moore.
I like the diffs:

x64, arm64:
Crossgen CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for  default jit
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: -2860 (-0.01% of base)
Crossgen CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for  linuxnonjit.dll
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: -9376 (-0.01% of base)
Crossgen CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for  protononjit.dll
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: -8744 (-0.01% of base)

but there are a few regressions, that happens when we create a new function instead of a better old variant.
For example:

N004 (  5,  5) [000127] ------------              *  JTRUE     void  
N003 (  3,  3) [000124] J------N----              \--*  LT        int    $1c3
N001 (  1,  1) [000126] ------------                 +--*  LCL_VAR   int    V07 loc1         u:2 $1c1
N002 (  1,  1) [000153] ------------                 \--*  CNS_INT   int    0 $40

where LCL_VAR is a result of ADD:

N006 (  5,  5) [000012] -A-XG---R---              *  ASG       int    $1c2
N005 (  1,  1) [000011] D------N----              +--*  LCL_VAR   int    V07 loc1         d:2 $1c1
N004 (  5,  5) [000010] ---XG-------              \--*  ADD       int    $1c2
N002 (  3,  3) [000077] ---XG-------                 +--*  ARR_LENGTH int    $1c0
N001 (  1,  1) [000076] ------------                 |  \--*  LCL_VAR   ref    V13 tmp1         u:1 $83
N003 (  1,  1) [000009] ------------                 \--*  CNS_INT   int    -1 $46

your new condition catches it when without your changes we were creating Const_Loop_Bnd and
optimized it better.
How did you choose a place for else if (vnStore->IsVNCompareCheckedBoundArith(relopVN)), would it be better to move it down after

    // Cases where op1 holds the condition bound check and op2 is 0.
    // Loop condition like: "i < 100 == 0"
    // Assertion: "i < 100 == false"
    else if (hasTestAgainstZero && vnStore->IsVNConstantBound(op1VN))

?

I attached the regressed method logs.
base.txt
diff.txt

Full diffs:
branchRemoval.All.txt