Optimize u>=1 to u!=0 and u<1 to u==0

[EgorBo]

JIT already applies these tricks for signed types. This PR adds support for unsigned (extends existing logic). The diff looks big because I removed the surrounding if-else block

using System.Runtime.CompilerServices;

class MyTests
{
    // make sure my changes don't regress signed types:
    bool C1(int a) => a >= 1;   // to "a > 0"
    bool C2(int a) => a < 1;    // to "a <= 0"
    bool C3(int a) => a <= -1;  // to "a < 0"
    bool C4(int a) => a > -1;   // to "a >= 0"

    // new cases:
    bool UC1(uint a) => a >= 1; // to "a != 0"
    bool UC2(uint a) => a < 1;  // to "a == 0"
}

; Method MyTests:C1(int):bool:this
G_M54558_IG02:
       test     edx, edx
       setg     al
       movzx    rax, al
; Total bytes of code: 9


; Method MyTests:C2(int):bool:this
G_M54557_IG02:
       test     edx, edx
       setle    al
       movzx    rax, al
; Total bytes of code: 9


; Method MyTests:C3(int):bool:this
G_M54556_IG02:
       test     edx, edx
       setl     al                  ; LLVM generates shr eax, 31
       movzx    rax, al
; Total bytes of code: 9


; Method MyTests:C4(int):bool:this
G_M54555_IG02:
       test     edx, edx
       setge    al
       movzx    rax, al
; Total bytes of code: 9


; Method MyTests:UC1(int):bool:this
G_M19883_IG02:
-      cmp      edx, 1
-      setae    al
+      test     edx, edx
+      setne    al
       movzx    rax, al
-; Total bytes of code: 10
+; Total bytes of code: 9


; Method MyTests:UC2(int):bool:this
G_M19880_IG02:
-      cmp      edx, 1
-      setb     al
+      test     edx, edx
+      sete     al
       movzx    rax, al
-; Total bytes of code: 10
+; Total bytes of code: 9

/cc: @mikedn

[mikedn]

Any diffs?

[EgorBo]

@mikedn is there a doc on how to do it properly for the whole coreclr/corefx? I am only using COMPlus_JitDisasm. And my own add-in for VS for simple diffs 🙂:

[mikedn]

For diffs you need jit-diff from the jitutils repository: https://github.com/dotnet/jitutils/blob/master/doc/diffs.md

Running jit-diff itself shouldn't be too difficult but you need a coreclr "baseline". The docs recommend using a separate coreclr clone but I find that to be overkill. I normally just build master and save the clrjit.dll in a "base" folder and pass that to jit-diff.

[mikedn]

Here's the x64 FX diff:

Found 3 files with textual diffs.
Crossgen Diffs for System.Private.CoreLib.dll, framework assemblies for x64 default jit
Summary:
(Lower is better)
Total bytes of diff: -280 (-0.00% of base)
    diff is an improvement.
Top file improvements by size (bytes):
        -272 : System.Private.CoreLib.dasm (-0.01% of base)
          -8 : System.Private.Xml.dasm (-0.00% of base)
2 total files with size differences (2 improved, 0 regressed), 127 unchanged.
Top method improvements by size (bytes):
        -198 (-4.88% of base) : System.Private.CoreLib.dasm - Vector64`1:ToString():ref:this (11 methods)
         -52 (-3.44% of base) : System.Private.CoreLib.dasm - Vector64`1:Equals(struct):bool:this (11 methods)
         -10 (-11.36% of base) : System.Private.CoreLib.dasm - Vector64:GetElement(struct,int):long (2 methods)
          -7 (-0.92% of base) : System.Private.Xml.dasm - BigNumber:Mul(byref):this
          -6 (-0.61% of base) : System.Private.CoreLib.dasm - Vector64`1:GetHashCode():int:this (11 methods)
Top method improvements by size (percentage):
         -10 (-11.36% of base) : System.Private.CoreLib.dasm - Vector64:GetElement(struct,int):long (2 methods)
          -5 (-11.11% of base) : System.Private.CoreLib.dasm - Vector64:GetElement(struct,int):double
        -198 (-4.88% of base) : System.Private.CoreLib.dasm - Vector64`1:ToString():ref:this (11 methods)
         -52 (-3.44% of base) : System.Private.CoreLib.dasm - Vector64`1:Equals(struct):bool:this (11 methods)
          -7 (-0.92% of base) : System.Private.Xml.dasm - BigNumber:Mul(byref):this
8 total methods with size differences (8 improved, 0 regressed), 146770 unchanged.
1 files had text diffs but not size diffs.
Microsoft.CodeAnalysis.VisualBasic.dasm had 12 diffs

[ArtBlnd]

Maybe better replace it to outside of else if (!tree->IsUnsigned() && cns2->IsIntegralConst(-1)) scope and also its end of if (op2->gtOper == GT_CNS_INT)?

[BruceForstall]

@dotnet/jit-contrib

[sandreenko]

the change looks good, thank you!

Sorry , it took me longer than I expected to reach that PR.
Could you please rebase/fix the conflicts and we will merge that?

[EgorBo]

@sandreenko sure!

[sandreenko]

@EgorBo could you please clone https://github.com/dotnet/jitutils, run bootstrap.cmd and then run jit-format to fix the formatting errors, like:

seandree@SEANDREEPC C:\git\tools\jitutils\bin
$ jit-format.bat --fix -c E:\git\coreclr