Optimize u>=1 to u!=0 and u<1 to u==0 by EgorBo · Pull Request #25458 · dotnet/coreclr

EgorBo · 2019-06-27T19:13:18Z

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 · 2019-06-28T06:37:24Z

Any diffs?

EgorBo · 2019-06-28T11:04:16Z

@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 · 2019-06-28T17:08:25Z

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 · 2019-06-28T17:17:58Z

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 · 2019-07-02T23:41:28Z

-                            // will still compute the same value as before
-                            tree->SetOper(oper, GenTree::PRESERVE_VN);
-                            cns2->gtIntCon.gtIconVal = 0;
+                    SET_OPER:


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 · 2019-08-03T00:04:23Z

@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 · 2019-10-16T01:42:47Z

@sandreenko sure!

…comp-fixes # Conflicts: # src/jit/morph.cpp

sandreenko · 2019-10-16T17:29:50Z

@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

EgorBo added 3 commits June 27, 2019 21:52

Optimize u >= 1 to u != 0 and u < 1 to u == 0 (u - unsigned)

b2cf465

undo some changes

0b5eea0

make clang-format happy

63b5043

mikedn reviewed Jun 28, 2019

View reviewed changes

Comment thread src/jit/morph.cpp Outdated

mikedn reviewed Jun 28, 2019

View reviewed changes

Comment thread src/jit/morph.cpp Outdated

mikedn reviewed Jun 28, 2019

View reviewed changes

Comment thread src/jit/morph.cpp Outdated

mikedn reviewed Jun 28, 2019

View reviewed changes

Comment thread src/jit/morph.cpp Outdated

Address feedback

f47bb30

ArtBlnd reviewed Jul 2, 2019

View reviewed changes

sandreenko self-requested a review September 23, 2019 04:55

sandreenko added the area-CodeGen label Sep 23, 2019

sandreenko approved these changes Oct 15, 2019

View reviewed changes

EgorBo force-pushed the jit-unsinged-comp-fixes branch from ac7842b to 55619df Compare October 16, 2019 10:15

Merge branch 'master' of github.com:dotnet/coreclr into jit-unsinged-…

5ea3065

…comp-fixes # Conflicts: # src/jit/morph.cpp

EgorBo force-pushed the jit-unsinged-comp-fixes branch from 55619df to 5ea3065 Compare October 16, 2019 10:18

Update morph.cpp

53ba808

sandreenko merged commit 5800248 into dotnet:master Oct 16, 2019

Navigation Menu

Uh oh!

Optimize u>=1 to u!=0 and u<1 to u==0 - #25458

Optimize u>=1 to u!=0 and u<1 to u==0 #25458
sandreenko merged 6 commits into
dotnet:masterfrom
EgorBo:jit-unsinged-comp-fixes

EgorBo commented Jun 27, 2019 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikedn commented Jun 28, 2019

Uh oh!

EgorBo commented Jun 28, 2019

Uh oh!

mikedn commented Jun 28, 2019

Uh oh!

mikedn commented Jun 28, 2019

Uh oh!

ArtBlnd Jul 2, 2019 •

edited

Loading

Uh oh!

BruceForstall commented Aug 3, 2019

Uh oh!

sandreenko left a comment

Uh oh!

EgorBo commented Oct 16, 2019

Uh oh!

sandreenko commented Oct 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

EgorBo commented Jun 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikedn commented Jun 28, 2019

Uh oh!

EgorBo commented Jun 28, 2019

Uh oh!

mikedn commented Jun 28, 2019

Uh oh!

mikedn commented Jun 28, 2019

Uh oh!

ArtBlnd Jul 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BruceForstall commented Aug 3, 2019

Uh oh!

sandreenko left a comment

Choose a reason for hiding this comment

Uh oh!

EgorBo commented Oct 16, 2019

Uh oh!

sandreenko commented Oct 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

EgorBo commented Jun 27, 2019 •

edited

Loading

ArtBlnd Jul 2, 2019 •

edited

Loading