JIT: invoke nullable box optimizations earlier #32269

AndyAyersMS · 2020-02-13T23:48:08Z

Move the logic in fgMorphRecognizeBoxNullable into gtFoldExprSpecial
so it can be invoked earlier. This may prevent the original struct
from becoming address exposed and allow subsequent optimizations when the
hasValue field has a known value.

Fixes #31661.

Move the logic in `fgMorphRecognizeBoxNullable` into `gtFoldExprSpecial` so it can be invoked earlier. This may prevent the original struct from becoming address exposed and allow subsequent optimizations when the `hasValue` field has a known value. Fixes dotnet#31661.

AndyAyersMS · 2020-02-13T23:49:59Z

cc @dotnet/jit-contrib

Normal PMI diffs only show a few hits:

Total bytes of diff: -187 (-0.00% of base)
    diff is an improvement.
Top file improvements (bytes):
        -187 : Newtonsoft.Json.dasm (-0.02% of base)
1 total files with Code Size differences (1 improved, 0 regressed), 162 unchanged.
Top method regressions (bytes):
           1 ( 0.15% of base) : Newtonsoft.Json.dasm - <DoReadAsDateTimeAsync>d__45:MoveNext():this
Top method improvements (bytes):
         -53 (-1.22% of base) : Newtonsoft.Json.dasm - <DoReadAsBooleanAsync>d__40:MoveNext():this
         -38 (-5.30% of base) : Newtonsoft.Json.dasm - <DoReadAsDoubleAsync>d__51:MoveNext():this
         -37 (-4.71% of base) : Newtonsoft.Json.dasm - <DoReadAsDateTimeOffsetAsync>d__47:MoveNext():this
         -35 (-4.55% of base) : Newtonsoft.Json.dasm - <DoReadAsDecimalAsync>d__49:MoveNext():this
         -25 (-3.81% of base) : Newtonsoft.Json.dasm - <DoReadAsInt32Async>d__53:MoveNext():this
Top method regressions (percentages):
           1 ( 0.15% of base) : Newtonsoft.Json.dasm - <DoReadAsDateTimeAsync>d__45:MoveNext():this
Top method improvements (percentages):
         -38 (-5.30% of base) : Newtonsoft.Json.dasm - <DoReadAsDoubleAsync>d__51:MoveNext():this
         -37 (-4.71% of base) : Newtonsoft.Json.dasm - <DoReadAsDateTimeOffsetAsync>d__47:MoveNext():this
         -35 (-4.55% of base) : Newtonsoft.Json.dasm - <DoReadAsDecimalAsync>d__49:MoveNext():this
         -25 (-3.81% of base) : Newtonsoft.Json.dasm - <DoReadAsInt32Async>d__53:MoveNext():this
         -53 (-1.22% of base) : Newtonsoft.Json.dasm - <DoReadAsBooleanAsync>d__40:MoveNext():this
6 total methods with Code Size differences (5 improved, 1 regressed), 244342 unchanged.

however if I modify PMI to use int? as one of the types to try in generics, we see more widespread improvement:

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 diff: -16305 (-0.03% of base)
    diff is an improvement.
Top file regressions (bytes):
           8 : System.DirectoryServices.AccountManagement.dasm (0.00% of base)
Top file improvements (bytes):
       -6476 : System.Linq.dasm (-0.54% of base)
       -3855 : System.Private.CoreLib.dasm (-0.08% of base)
       -3773 : System.Collections.dasm (-0.57% of base)
        -696 : System.Collections.Immutable.dasm (-0.05% of base)
        -477 : System.Collections.Concurrent.dasm (-0.12% of base)
        -203 : System.Numerics.Tensors.dasm (-0.07% of base)
        -197 : System.Net.Http.dasm (-0.03% of base)
        -187 : Newtonsoft.Json.dasm (-0.02% of base)
        -149 : System.Linq.Expressions.dasm (-0.02% of base)
        -118 : System.Linq.Parallel.dasm (-0.01% of base)
         -99 : System.Memory.dasm (-0.04% of base)
         -35 : xunit.core.dasm (-0.05% of base)
         -28 : System.Text.RegularExpressions.dasm (-0.01% of base)
         -19 : Microsoft.CodeAnalysis.dasm (-0.00% of base)
          -1 : System.ComponentModel.Composition.dasm (-0.00% of base)
16 total files with Code Size differences (15 improved, 1 regressed), 147 unchanged.
Top method regressions (bytes):
           4 ( 0.06% of base) : System.Private.CoreLib.dasm - String:Concat(IEnumerable`1):String (9 methods)
           4 ( 0.06% of base) : System.Private.CoreLib.dasm - String:JoinCore(long,int,IEnumerable`1):String (8 methods)
           4 ( 0.10% of base) : System.Private.CoreLib.dasm - StringBuilder:AppendJoinCore(long,int,IEnumerable`1):StringBuilder:this (8 methods)
           2 ( 1.90% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalValueCollection`1:Add(Nullable`1):this
           2 ( 1.90% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalValueCollection`1:Contains(Nullable`1):bool:this
           2 ( 0.85% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalValueCollection`1:Insert(int,Nullable`1):this
           2 ( 1.90% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalValueCollection`1:Remove(Nullable`1):bool:this
           2 ( 0.09% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (8 methods)
           2 ( 1.80% of base) : System.Private.CoreLib.dasm - IMapViewToIReadOnlyDictionaryAdapter:ContainsKey(Nullable`1):bool:this
           2 ( 1.80% of base) : System.Private.CoreLib.dasm - MapToDictionaryAdapter:ContainsKey(Nullable`1):bool:this
           1 ( 0.15% of base) : Newtonsoft.Json.dasm - <DoReadAsDateTimeAsync>d__45:MoveNext():this
Top method improvements (bytes):
       -2911 (-8.01% of base) : System.Linq.dasm - Enumerable:Max(IEnumerable`1,Func`2):Nullable`1 (48 methods)
       -2911 (-9.10% of base) : System.Linq.dasm - Enumerable:Min(IEnumerable`1,Func`2):Nullable`1 (48 methods)
       -1312 (-38.50% of base) : System.Private.CoreLib.dasm - Dictionary`2:ContainsValue(Nullable`1):bool:this (8 methods)
        -664 (-15.04% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.set_Item(Object,Object):this (8 methods)
        -664 (-15.04% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.Add(Object,Object):this (8 methods)
        -664 (-19.42% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.set_Item(Object,Object):this (8 methods)
        -640 (-13.39% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.Add(Object,Object):this (8 methods)
        -345 (-53.82% of base) : System.Private.CoreLib.dasm - ICustomPropertyProviderProxy`2:ConvertTo(Object):Nullable`1 (8 methods)
        -327 (-8.39% of base) : System.Linq.dasm - Enumerable:Max(IEnumerable`1):Nullable`1 (6 methods)
        -327 (-9.48% of base) : System.Linq.dasm - Enumerable:Min(IEnumerable`1):Nullable`1 (6 methods)
        -296 (-10.32% of base) : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.IDictionary.set_Item(Object,Object):this (8 methods)
        -296 (-10.32% of base) : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.IDictionary.Add(Object,Object):this (8 methods)
        -285 (-29.97% of base) : System.Collections.dasm - HashSet`1:InternalIndexOf(Nullable`1):int:this
        -280 (-29.44% of base) : System.Collections.dasm - HashSet`1:Contains(Nullable`1):bool:this
        -277 (-23.98% of base) : System.Collections.dasm - HashSet`1:Remove(Nullable`1):bool:this
        -269 (-23.74% of base) : System.Collections.dasm - HashSet`1:AddIfNotPresent(Nullable`1):bool:this
        -240 (-14.10% of base) : System.Private.CoreLib.dasm - ICustomPropertyProviderProxy`2:System.Runtime.InteropServices.WindowsRuntime.IBindableVectorView.IndexOf(Object,byref):bool:this (8 methods)
        -207 (-18.09% of base) : System.Private.CoreLib.dasm - Dictionary`2:TryInsert(Nullable`1,Nullable`1,ubyte):bool:this
        -197 (-3.91% of base) : System.Net.Http.dasm - <WaitAndReturnAsync>d__76`2:MoveNext():this (8 methods)
        -172 (-6.61% of base) : System.Collections.Concurrent.dasm - ConcurrentDictionary`2:System.Collections.IDictionary.set_Item(Object,Object):this (8 methods)
Top method regressions (percentages):
           2 ( 1.90% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalValueCollection`1:Add(Nullable`1):this
           2 ( 1.90% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalValueCollection`1:Contains(Nullable`1):bool:this
           2 ( 1.90% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalValueCollection`1:Remove(Nullable`1):bool:this
           2 ( 1.80% of base) : System.Private.CoreLib.dasm - IMapViewToIReadOnlyDictionaryAdapter:ContainsKey(Nullable`1):bool:this
           2 ( 1.80% of base) : System.Private.CoreLib.dasm - MapToDictionaryAdapter:ContainsKey(Nullable`1):bool:this
           2 ( 0.85% of base) : System.DirectoryServices.AccountManagement.dasm - PrincipalValueCollection`1:Insert(int,Nullable`1):this
           1 ( 0.15% of base) : Newtonsoft.Json.dasm - <DoReadAsDateTimeAsync>d__45:MoveNext():this
           4 ( 0.10% of base) : System.Private.CoreLib.dasm - StringBuilder:AppendJoinCore(long,int,IEnumerable`1):StringBuilder:this (8 methods)
           2 ( 0.09% of base) : System.Private.CoreLib.dasm - ValueTask`1:ToString():String:this (8 methods)
           4 ( 0.06% of base) : System.Private.CoreLib.dasm - String:JoinCore(long,int,IEnumerable`1):String (8 methods)
           4 ( 0.06% of base) : System.Private.CoreLib.dasm - String:Concat(IEnumerable`1):String (9 methods)
Top method improvements (percentages):
        -345 (-53.82% of base) : System.Private.CoreLib.dasm - ICustomPropertyProviderProxy`2:ConvertTo(Object):Nullable`1 (8 methods)
       -1312 (-38.50% of base) : System.Private.CoreLib.dasm - Dictionary`2:ContainsValue(Nullable`1):bool:this (8 methods)
          -9 (-30.00% of base) : System.Collections.Immutable.dasm - Requires:NotNullAllowStructs(Nullable`1,String)
        -285 (-29.97% of base) : System.Collections.dasm - HashSet`1:InternalIndexOf(Nullable`1):int:this
        -280 (-29.44% of base) : System.Collections.dasm - HashSet`1:Contains(Nullable`1):bool:this
        -112 (-24.03% of base) : System.Collections.Concurrent.dasm - ConcurrentDictionary`2:ThrowIfInvalidObjectValue(Object) (8 methods)
        -277 (-23.98% of base) : System.Collections.dasm - HashSet`1:Remove(Nullable`1):bool:this
        -269 (-23.74% of base) : System.Collections.dasm - HashSet`1:AddIfNotPresent(Nullable`1):bool:this
         -35 (-20.71% of base) : System.Private.CoreLib.dasm - ThrowHelper:IfNullAndNullsAreIllegalThenThrow(Object,int) (8 methods)
         -14 (-20.59% of base) : System.Collections.Immutable.dasm - SecurePooledObject`1:.ctor(Nullable`1):this
        -664 (-19.42% of base) : System.Collections.dasm - SortedList`2:System.Collections.IDictionary.set_Item(Object,Object):this (8 methods)
        -116 (-19.40% of base) : System.Private.CoreLib.dasm - Dictionary`2:FindValue(Nullable`1):byref:this
        -207 (-18.09% of base) : System.Private.CoreLib.dasm - Dictionary`2:TryInsert(Nullable`1,Nullable`1,ubyte):bool:this
         -15 (-16.13% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary`2:ValueRef(Nullable`1):byref:this
         -15 (-16.13% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary`2:ContainsKey(Nullable`1):bool:this
         -15 (-16.13% of base) : System.Collections.Immutable.dasm - Builder:ValueRef(Nullable`1):byref:this
         -15 (-15.62% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary`2:TryGetValue(Nullable`1,byref):bool:this
         -15 (-15.62% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary`2:TryGetKey(Nullable`1,byref):bool:this
        -136 (-15.37% of base) : System.Linq.Expressions.dasm - ReadOnlyCollectionBuilder`1:ValidateNullValue(Object,String) (8 methods)
        -664 (-15.04% of base) : System.Collections.dasm - SortedDictionary`2:System.Collections.IDictionary.set_Item(Object,Object):this (8 methods)
192 total methods with Code Size differences (181 improved, 11 regressed), 247491 unchanged.

AndyAyersMS · 2020-02-13T23:53:08Z

;; Example from 31661

;; before

G_M35686_IG01:
       push     rax
       xor      rax, rax
       mov      qword ptr [rsp], rax

G_M35686_IG02:
       xor      eax, eax
       mov      qword ptr [rsp], rax
       cmp      byte  ptr [rsp], 0
       jne      SHORT G_M35686_IG05

G_M35686_IG03:
       mov      eax, 1

G_M35686_IG04:
       add      rsp, 8
       ret      

G_M35686_IG05:
       mov      eax, 2

G_M35686_IG06:
       add      rsp, 8
       ret      

;; after

G_M35686_IG01:

G_M35686_IG02:
       mov      eax, 1

G_M35686_IG03:
       ret

AndyAyersMS · 2020-02-14T01:25:28Z

Perhaps gtFoldExprSpecial is too broadly aggressive; let me look into these failures.

AndyAyersMS · 2020-02-14T02:40:13Z

FWIW some of these live builds do strange things -- though I suppose things that should work -- like crossgenning a debug mono SPC.

AndyAyersMS · 2020-02-19T18:28:17Z

@dotnet/jit-contrib PTAL

briansull · 2020-02-19T18:37:46Z

src/coreclr/src/jit/gentree.cpp

+
+    if (val != 0)
+    {
+        return tree;


We should check for this precondition first, before checking for op->IsCall()

briansull · 2020-02-19T18:43:46Z

src/coreclr/src/jit/gentree.cpp

+{
+    assert(tree->OperKind() & GTK_BINOP);
+    genTreeOps const oper = tree->OperGet();
+


This method should either assert or check that oper is GT_EQ, GT_NE or GT_GT rather than implicitly relying on the caller to do this check.

rework to not optimize too much in minopts/debug

4949a83

AndyAyersMS added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 18, 2020

briansull approved these changes Feb 19, 2020

View reviewed changes

review feedback

7053a3f

AndyAyersMS merged commit ddf4f4b into dotnet:master Feb 20, 2020

AndyAyersMS deleted the RefactorNullableOpt branch February 20, 2020 00:17

AndyAyersMS mentioned this pull request Feb 21, 2020

Getting underlying value of nullable without code duplication in generics #12568

Open

EgorBo mentioned this pull request Apr 13, 2020

JIT: "default(T) == null" does not emit a constant when T is Nullable<> #34906

Closed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT: invoke nullable box optimizations earlier #32269

JIT: invoke nullable box optimizations earlier #32269

AndyAyersMS commented Feb 13, 2020

AndyAyersMS commented Feb 13, 2020

AndyAyersMS commented Feb 13, 2020

AndyAyersMS commented Feb 14, 2020

AndyAyersMS commented Feb 14, 2020

AndyAyersMS commented Feb 19, 2020

briansull Feb 19, 2020

briansull Feb 19, 2020

JIT: invoke nullable box optimizations earlier #32269

JIT: invoke nullable box optimizations earlier #32269

Conversation

AndyAyersMS commented Feb 13, 2020

AndyAyersMS commented Feb 13, 2020

AndyAyersMS commented Feb 13, 2020

AndyAyersMS commented Feb 14, 2020

AndyAyersMS commented Feb 14, 2020

AndyAyersMS commented Feb 19, 2020

briansull Feb 19, 2020

Choose a reason for hiding this comment

briansull Feb 19, 2020

Choose a reason for hiding this comment