JIT: simple redundant compare optimization #43811

AndyAyersMS · 2020-10-25T21:35:18Z

For a relop, walk up the dominator tree to see if any dominating
block has a similar compare. If so, and there's just one path from
that block to the relop, the relop's value is known.

Closes #11909.

For a relop, walk up the dominator tree to see if any dominating block has a similar compare. If so, and there's just one path from that block to the relop, the relop's value is known. Closes dotnet#11909.

AndyAyersMS · 2020-10-25T21:39:27Z

@briansull PTAL.
cc @dotnet/jit-contrib

This will eventually feed into optimizing redundant class checks we'll see in dynamic PGO.

PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for x64 default jit

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 50312241
Total bytes of diff: 50302279
Total bytes of delta: -9962 (-0.02% of base)
    diff is an improvement.

Top file improvements (bytes):
       -4129 : System.Data.Common.dasm (-0.28% of base)
       -3542 : FSharp.Core.dasm (-0.11% of base)
        -637 : System.Private.CoreLib.dasm (-0.01% of base)
        -274 : Microsoft.CodeAnalysis.CSharp.dasm (-0.01% of base)
        -258 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.00% of base)
        -154 : System.Text.Json.dasm (-0.03% of base)
        -104 : System.Security.Cryptography.Pkcs.dasm (-0.03% of base)
         -89 : Microsoft.Extensions.DependencyModel.dasm (-0.19% of base)
         -75 : System.Formats.Asn1.dasm (-0.08% of base)
         -63 : System.Net.Http.dasm (-0.01% of base)
         -61 : System.DirectoryServices.AccountManagement.dasm (-0.02% of base)
         -59 : System.Linq.Expressions.dasm (-0.01% of base)
         -38 : Microsoft.Extensions.Logging.Console.dasm (-0.09% of base)
         -36 : System.Diagnostics.Process.dasm (-0.04% of base)
         -35 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (-0.00% of base)
         -33 : System.Private.Xml.dasm (-0.00% of base)
         -30 : System.Memory.dasm (-0.01% of base)
         -28 : System.Text.Encodings.Web.dasm (-0.09% of base)
         -27 : System.Security.Cryptography.Algorithms.dasm (-0.01% of base)
         -27 : System.Security.Cryptography.Cng.dasm (-0.02% of base)

42 total files with Code Size differences (42 improved, 0 regressed), 226 unchanged.

Top method improvements (bytes):
        -529 (-14.86% of base) : System.Data.Common.dasm - SqlDoubleStorage:Aggregate(ref,int):Object:this
        -449 (-12.49% of base) : System.Data.Common.dasm - SqlSingleStorage:Aggregate(ref,int):Object:this
        -446 (-13.78% of base) : System.Data.Common.dasm - SqlInt16Storage:Aggregate(ref,int):Object:this
        -446 (-13.79% of base) : System.Data.Common.dasm - SqlInt32Storage:Aggregate(ref,int):Object:this
        -446 (-12.31% of base) : System.Data.Common.dasm - SqlInt64Storage:Aggregate(ref,int):Object:this
        -446 (-11.65% of base) : System.Data.Common.dasm - SqlMoneyStorage:Aggregate(ref,int):Object:this
        -446 (-13.72% of base) : System.Data.Common.dasm - SqlByteStorage:Aggregate(ref,int):Object:this
        -440 (-11.14% of base) : System.Data.Common.dasm - SqlDecimalStorage:Aggregate(ref,int):Object:this
        -203 (-5.40% of base) : System.Private.CoreLib.dasm - TextInfo:ChangeCaseCommon(String):String:this (7 methods)
        -151 (-44.41% of base) : System.Data.Common.dasm - DataTable:FindByIndex(Index,ref):DataRow:this
        -139 (-20.20% of base) : System.Data.Common.dasm - DataView:System.ComponentModel.IBindingList.Find(PropertyDescriptor,Object):int:this
        -134 (-9.48% of base) : System.Data.Common.dasm - DataTable:LoadRow(ref,int,Index):DataRow:this
        -130 (-20.57% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkEqualsQmark(Nullable`1,Nullable`1):bool (6 methods)
        -127 (-12.99% of base) : System.Private.CoreLib.dasm - Vector128`1:<Equals>g__SoftwareFallback|12_0(byref,Vector128`1):bool (6 methods)
         -95 (-21.16% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkGreaterEqualsQmark(Nullable`1,Nullable`1):bool (6 methods)
         -95 (-21.16% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkGreaterQmark(Nullable`1,Nullable`1):bool (6 methods)
         -95 (-21.16% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkLessEqualsQmark(Nullable`1,Nullable`1):bool (6 methods)
         -95 (-21.16% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkLessQmark(Nullable`1,Nullable`1):bool (6 methods)
         -87 (-10.48% of base) : System.Private.CoreLib.dasm - Vector256`1:<Equals>g__SoftwareFallback|14_0(byref,Vector256`1):bool (6 methods)
         -82 (-3.63% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - CodeGenerator:EmitTryCastExpression(BoundTryCast,bool):this

Top method improvements (percentages):
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkGreaterEquals(Nullable`1,long):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkGreater(Nullable`1,long):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkLessEquals(Nullable`1,long):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkLess(Nullable`1,long):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkEquals(Nullable`1,long):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_GreaterEqualsQmark(long,Nullable`1):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_GreaterQmark(long,Nullable`1):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_LessEqualsQmark(long,Nullable`1):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_LessQmark(long,Nullable`1):bool
         -22 (-50.00% of base) : FSharp.Core.dasm - NullableOperators:op_EqualsQmark(long,Nullable`1):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkGreaterEquals(Nullable`1,double):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkGreater(Nullable`1,double):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkLessEquals(Nullable`1,double):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkLess(Nullable`1,double):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkEquals(Nullable`1,int):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_QmarkEquals(Nullable`1,double):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_GreaterEqualsQmark(double,Nullable`1):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_GreaterQmark(double,Nullable`1):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_LessEqualsQmark(double,Nullable`1):bool
         -22 (-45.83% of base) : FSharp.Core.dasm - NullableOperators:op_LessQmark(double,Nullable`1):bool

257 total methods with Code Size differences (257 improved, 0 regressed), 258511 unchanged.

Diff from #11909

; Assembly listing for method String:EndsWith(String,int):bool:this

;; BEFORE

G_M26632_IG11:
       mov      ebx, dword ptr [rdi+8]
       mov      r8d, ebx
       sub      r8d, r9d
       cmp      ebx, r8d
       jb       SHORT G_M26632_IG16
       cmp      ebx, r8d
       jb       G_M26632_IG22

G_M26632_IG12:
       add      rdi, 12

;; AFTER

G_M26632_IG11:
       mov      ebx, dword ptr [rdi+8]
       mov      r8d, ebx
       sub      r8d, r9d
       cmp      ebx, r8d
       jb       SHORT G_M26632_IG15
       add      rdi, 12

AndyAyersMS · 2020-10-25T21:46:11Z

Note it feels a bit odd to be doing this opt here, as it's not strictly related to assertion prop, but it doesn't seem to warrant its own phase. Feel free to suggest a better home.

AndyAyersMS · 2020-11-02T16:56:52Z

@dotnet/jit-contrib anyone able to take a look?

briansull

Approved with doc suggestion

briansull · 2020-11-02T20:32:21Z

src/coreclr/src/jit/assertionprop.cpp

@@ -3215,6 +3215,153 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen
    GenTree* op1     = tree->AsOp()->gtOp1;
    GenTree* op2     = tree->AsOp()->gtOp2;

+    // First, walk up the dom tree and see if any dominating block has branched on


You should update the method header to the standard header and under Note: add comment saying that we also do this special case optimization.

AndyAyersMS · 2020-11-04T02:40:31Z

One test failure on x64 osx: profiler/elt/slowpatheltleave/slowpatheltleave.sh

This test seems to fail sporadically once a day or so. It looks like the test "passes" but then perhaps the runtime crashes on exit?

@davmason is this on your radar?

davmason · 2020-11-04T05:52:03Z

@davmason is this on your radar?

No, I haven't seen that before. I'll take a look

AndyAyersMS · 2020-11-04T22:56:58Z

I'm going to merge as the one failure above does not seem related.

benaadams · 2020-11-14T02:43:05Z

@AndyAyersMS should have also fixed #35348

JIT: simple redundant compare optimization

fc57631

For a relop, walk up the dominator tree to see if any dominating block has a similar compare. If so, and there's just one path from that block to the relop, the relop's value is known. Closes dotnet#11909.

Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 25, 2020

briansull approved these changes Nov 2, 2020

View reviewed changes

update header comment per feedback

9d27468

AndyAyersMS merged commit 879b904 into dotnet:master Nov 4, 2020

AndyAyersMS deleted the RedundantBranches branch November 4, 2020 22:57

This was referenced Nov 5, 2020

profiler/elt/slowpatheltleave/slowpatheltleave.sh failing in CI #44309

Closed

Test failure: profiler/elt/slowpatheltenter/slowpatheltenter.sh #44055

Closed

This was referenced Nov 14, 2020

[Jit] Redundant comparisons are not eliminated #35348

Closed

Redundant length check when using Span<byte> => new byte[] #12571

Closed

ghost locked as resolved and limited conversation to collaborators Dec 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT: simple redundant compare optimization #43811

JIT: simple redundant compare optimization #43811

AndyAyersMS commented Oct 25, 2020

AndyAyersMS commented Oct 25, 2020

AndyAyersMS commented Oct 25, 2020

AndyAyersMS commented Nov 2, 2020

briansull left a comment

briansull Nov 2, 2020

AndyAyersMS commented Nov 4, 2020

davmason commented Nov 4, 2020

AndyAyersMS commented Nov 4, 2020

benaadams commented Nov 14, 2020

JIT: simple redundant compare optimization #43811

JIT: simple redundant compare optimization #43811

Conversation

AndyAyersMS commented Oct 25, 2020

AndyAyersMS commented Oct 25, 2020

AndyAyersMS commented Oct 25, 2020

AndyAyersMS commented Nov 2, 2020

briansull left a comment

Choose a reason for hiding this comment

briansull Nov 2, 2020

Choose a reason for hiding this comment

AndyAyersMS commented Nov 4, 2020

davmason commented Nov 4, 2020

AndyAyersMS commented Nov 4, 2020

benaadams commented Nov 14, 2020