Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Mar 23, 2021

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Mar 22, 2021

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

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2021
@EgorBo
Copy link
Member Author

EgorBo commented Mar 22, 2021

Id: 50000 🎉

@antonfirsov
Copy link
Member

antonfirsov commented Mar 22, 2021

I'm here only because mine was #49999 unluckily

@EgorBo
Copy link
Member Author

EgorBo commented Mar 22, 2021

@dotnet/jit-contrib PTAL

Copy link
Contributor

@sandreenko sandreenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the diffs look awesome!

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
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
Co-authored-by: Andy Ayers <andya@microsoft.com>
Co-authored-by: Sergey Andreenko <seandree@microsoft.com>
@EgorBo EgorBo merged commit 65814eb into dotnet:main Mar 23, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 22, 2021
@JulieLeeMSFT JulieLeeMSFT added this to the 6.0.0 milestone Apr 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants