JIT: Fold null checks against initialized static readonly fields #50000

EgorBo · 2021-03-22T15:48:49Z

Follow up to #49930

This PR folds nullchecks for static readonly fields of ref types (already statically inited, known to be not null), e.g.:

class Program
{
    private static string PathEnvVar { get; } = Environment.GetEnvironmentVariable("PATH");

    public bool IsNotNull() => PathEnvVar != null;
}

Current codegen for IsNotNull (tier1):

; Assembly listing for method IsNotNull():bool:this
G_M52841_IG01:             
G_M52841_IG02:            
       48B8382DF5C507020000 mov      rax, 0x207C5F52D38
       48833800             cmp      gword ptr [rax], 0
       0F95C0               setne    al
       0FB6C0               movzx    rax, al
G_M52841_IG03:              
       C3                   ret      
; Total bytes of code 21

New codegen for IsNotNull:

; Assembly listing for method IsNotNull():bool:this
G_M52841_IG01:
G_M52841_IG02:             
       B801000000           mov      eax, 1
G_M52841_IG03:              
       C3                   ret      
; Total bytes of code 6

jit-diff (-f -cctors)

PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies [invoking .cctors] for  default jit

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 52242582
Total bytes of diff: 52203720
Total bytes of delta: -38862 (-0.07% of base)
    diff is an improvement.


Top file improvements (bytes):
       -8900 : FSharp.Core.dasm (-0.25% of base)
       -3924 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.07% of base)
       -3681 : System.Threading.Tasks.Dataflow.dasm (-0.39% of base)
       -3018 : System.Private.Xml.dasm (-0.09% of base)
       -2009 : Microsoft.CodeAnalysis.CSharp.dasm (-0.05% of base)
       -1626 : System.Private.CoreLib.dasm (-0.04% of base)
       -1286 : Microsoft.CodeAnalysis.dasm (-0.07% of base)
       -1166 : System.Data.Common.dasm (-0.08% of base)
       -1060 : System.Collections.Immutable.dasm (-0.08% of base)
        -983 : CommandLine.dasm (-0.20% of base)
        -631 : System.Linq.Parallel.dasm (-0.03% of base)
        -571 : Newtonsoft.Json.dasm (-0.07% of base)
        -567 : System.Net.Http.dasm (-0.09% of base)
        -519 : System.Data.OleDb.dasm (-0.19% of base)
        -508 : System.Configuration.ConfigurationManager.dasm (-0.15% of base)
        -498 : System.Speech.dasm (-0.12% of base)
        -376 : System.Net.Mail.dasm (-0.20% of base)
        -374 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (-0.01% of base)
        -367 : xunit.execution.dotnet.dasm (-0.16% of base)
        -366 : System.CodeDom.dasm (-0.21% of base)

106 total files with Code Size differences (106 improved, 0 regressed), 165 unchanged.

Top method regressions (bytes):
           6 ( 0.16% of base) : Microsoft.Diagnostics.Tracing.TraceEvent.dasm - TraceEventSession:EnableProvider(Guid,int,long,TraceEventProviderOptions):bool:this

Top method improvements (bytes):
       -2160 (-7.68% of base) : FSharp.Core.dasm - FSharpMailboxProcessor`1:PostAndTryAsyncReply(FSharpFunc`2,FSharpOption`1):FSharpAsync`1:this (64 methods)
       -1143 (-11.13% of base) : FSharp.Core.dasm - Mailbox`1:TryScan(FSharpFunc`2,int):FSharpAsync`1:this (64 methods)
       -1143 (-11.13% of base) : FSharp.Core.dasm - Mailbox`1:Scan(FSharpFunc`2,int):FSharpAsync`1:this (64 methods)
        -863 (-3.21% of base) : FSharp.Core.dasm - FSharpMailboxProcessor`1:PostAndAsyncReply(FSharpFunc`2,FSharpOption`1):FSharpAsync`1:this (64 methods)
        -575 (-6.21% of base) : System.Threading.Tasks.Dataflow.dasm - ReceiveTarget`1:CleanupAndComplete(int):this (8 methods)
        -384 (-3.07% of base) : FSharp.Core.dasm - Array2DModule:CopyTo(ref,int,int,ref,int,int,int,int) (8 methods)
        -277 (-9.84% of base) : FSharp.Core.dasm - FSharpAsync:AwaitAndBindChildResult(CancellationTokenSource,ResultCell`1,FSharpOption`1):FSharpAsync`1 (8 methods)
        -252 (-6.51% of base) : System.Collections.Immutable.dasm - Builder:set_KeyComparer(IEqualityComparer`1):this (16 methods)
        -245 (-5.25% of base) : System.ComponentModel.TypeConverter.dasm - <>c:<get_IntrinsicTypeConverters>b__23_0():Dictionary`2:this
        -236 (-6.94% of base) : CommandLine.dasm - <>c:<EnforceRequired>b__2_0(IEnumerable`1):IEnumerable`1:this
        -233 (-4.71% of base) : System.Threading.Tasks.Dataflow.dasm - TargetCore`1:ProcessAsyncIfNecessary_Slow(bool):this (8 methods)
        -223 (-9.93% of base) : System.Private.Xml.dasm - XmlUntypedStringConverter:StringToListType(String,Type,IXmlNamespaceResolver):Object:this
        -218 (-7.48% of base) : CommandLine.dasm - <>c__DisplayClass4_0:<EnforceSingle>b__0(IEnumerable`1):IEnumerable`1:this
        -217 (-18.72% of base) : Microsoft.CodeAnalysis.dasm - UICultureUtilities:WithCurrentUICulture(Action`1):Action`1 (8 methods)
        -217 (-18.72% of base) : Microsoft.CodeAnalysis.dasm - UICultureUtilities:WithCurrentUICulture(Func`1):Func`1 (8 methods)
        -215 (-12.38% of base) : System.Threading.Tasks.Dataflow.dasm - OutputAvailableAsyncTarget`1:CancelAndUnlink(Object) (8 methods)
        -215 (-11.58% of base) : System.Threading.Tasks.Dataflow.dasm - TargetCore`1:CompleteBlockIfPossible_Slow():this (8 methods)
        -173 (-7.44% of base) : System.Threading.Tasks.Dataflow.dasm - DataflowBlock:TryChooseFromSource(ISourceBlock`1,Action`1,int,TaskScheduler,byref):bool (8 methods)
        -168 (-6.34% of base) : System.Collections.Immutable.dasm - ImmutableSortedDictionary`2:WithComparers(IComparer`1,IEqualityComparer`1):ImmutableSortedDictionary`2:this (8 methods)
        -168 (-18.71% of base) : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey,TValue>>.Contains(KeyValuePair`2):bool:this (8 methods)

Top method regressions (percentages):
           6 ( 0.16% of base) : Microsoft.Diagnostics.Tracing.TraceEvent.dasm - TraceEventSession:EnableProvider(Guid,int,long,TraceEventProviderOptions):bool:this

Top method improvements (percentages):
         -19 (-95.00% of base) : System.Net.Http.WinHttpHandler.dasm - NetEventSource:DebugValidateArg(Object)
         -50 (-89.29% of base) : Microsoft.CodeAnalysis.dasm - HashAlgorithm:get_SupportsTransform():bool:this
         -15 (-65.22% of base) : Microsoft.Extensions.DependencyModel.dasm - EnvironmentWrapper:GetAppContextData(String):Object:this
         -48 (-57.83% of base) : System.Net.Mail.dasm - HeaderCollection:.ctor():this
         -48 (-57.83% of base) : System.Web.HttpUtility.dasm - HttpQSCollection:.ctor():this
         -48 (-57.83% of base) : System.Configuration.ConfigurationManager.dasm - ConfigurationValues:.ctor():this
        -114 (-39.18% of base) : System.Private.Xml.dasm - XmlILOptimizerVisitor:IsPrimitiveNumeric(XmlQueryType):bool:this
        -114 (-39.18% of base) : System.Private.Xml.dasm - XmlILOptimizerVisitor:MatchesContentTest(XmlQueryType):bool:this
         -28 (-38.36% of base) : System.Security.Cryptography.Cng.dasm - Rc2CbcParameters:GetEffectiveKeyBits():int:this
         -28 (-38.36% of base) : System.Security.Cryptography.Pkcs.dasm - Rc2CbcParameters:GetEffectiveKeyBits():int:this
         -28 (-38.36% of base) : System.Security.Cryptography.Algorithms.dasm - Rc2CbcParameters:GetEffectiveKeyBits():int:this
         -20 (-36.36% of base) : System.Collections.Specialized.dasm - NameObjectCollectionBase:.ctor():this
         -20 (-36.36% of base) : System.Collections.Specialized.dasm - NameValueCollection:.ctor():this
         -15 (-29.41% of base) : System.Configuration.ConfigurationManager.dasm - SettingsPropertyValue:IsHostedInAspnet():bool:this
         -40 (-27.59% of base) : System.Configuration.ConfigurationManager.dasm - ConfigurationElement:.ctor():this
         -16 (-26.23% of base) : System.IO.Packaging.dasm - ContentType:IsAllowedCharacter(ushort):bool
         -16 (-26.23% of base) : System.Reflection.MetadataLoadContext.dasm - Helpers:NeedsEscapingInTypeName(ushort):bool
         -18 (-25.35% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - _Closure$__:_Lambda$__0-3(Symbol):String:this
         -44 (-24.72% of base) : System.Net.Mail.dasm - Message:get_Headers():HeaderCollection:this
         -44 (-24.72% of base) : System.Net.Mail.dasm - Message:get_EnvelopeHeaders():HeaderCollection:this

1939 total methods with Code Size differences (1938 improved, 1 regressed), 266648 unchanged.

--------------------------------------------------------------------------------
Completed analysis in 36.81s

EgorBo · 2021-03-22T15:50:28Z

Id: 50000 🎉

antonfirsov · 2021-03-22T15:52:28Z

I'm here only because mine was #49999 unluckily

EgorBo · 2021-03-22T18:05:11Z

@dotnet/jit-contrib PTAL

sandreenko

LGTM, the diffs look awesome!

src/coreclr/jit/valuenum.cpp

src/coreclr/jit/valuenumfuncs.h

src/coreclr/jit/gentree.h

src/coreclr/jit/valuenum.cpp

Co-authored-by: Andy Ayers <[email protected]>

Co-authored-by: Sergey Andreenko <[email protected]>

EgorBo added 3 commits March 22, 2021 17:28

Introduce NonNullIndirect VN func

7ff23b1

Add a comment

ba52b20

Add an assert

684214c

ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2021

sandreenko approved these changes Mar 22, 2021

View reviewed changes

src/coreclr/jit/valuenum.cpp Outdated Show resolved Hide resolved

src/coreclr/jit/valuenum.cpp Show resolved Hide resolved

src/coreclr/jit/valuenumfuncs.h Outdated Show resolved Hide resolved

AndyAyersMS reviewed Mar 22, 2021

View reviewed changes

src/coreclr/jit/gentree.h Outdated Show resolved Hide resolved

src/coreclr/jit/valuenum.cpp Show resolved Hide resolved

EgorBo and others added 4 commits March 22, 2021 21:28

Update src/coreclr/jit/gentree.h

23498e0

Co-authored-by: Andy Ayers <[email protected]>

Apply suggestions from code review

1d99043

Co-authored-by: Sergey Andreenko <[email protected]>

Update valuenumfuncs.h

4e5fb87

Address feedback

a6d7e33

EgorBo merged commit 65814eb into dotnet:main Mar 23, 2021

ghost locked as resolved and limited conversation to collaborators Apr 22, 2021

JulieLeeMSFT assigned EgorBo Apr 27, 2021

JulieLeeMSFT added this to the 6.0.0 milestone Apr 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

JIT: Fold null checks against initialized static readonly fields #50000

JIT: Fold null checks against initialized static readonly fields #50000

Uh oh!

EgorBo commented Mar 22, 2021 •

edited

Loading

Uh oh!

EgorBo commented Mar 22, 2021

Uh oh!

antonfirsov commented Mar 22, 2021 •

edited

Loading

Uh oh!

EgorBo commented Mar 22, 2021

Uh oh!

sandreenko left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JIT: Fold null checks against initialized static readonly fields #50000

JIT: Fold null checks against initialized static readonly fields #50000

Uh oh!

Conversation

EgorBo commented Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EgorBo commented Mar 22, 2021

Uh oh!

antonfirsov commented Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EgorBo commented Mar 22, 2021

Uh oh!

sandreenko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

EgorBo commented Mar 22, 2021 •

edited

Loading

antonfirsov commented Mar 22, 2021 •

edited

Loading