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 for const strings #49930

Merged
merged 5 commits into from
Mar 22, 2021
Merged

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Mar 20, 2021

Similar to #37245 but now at the VN level so we can fold more, example:

class Program
{
    public string Name;

    public void Foo(string name)
    {
        Name = name ?? throw new ArgumentNullException(nameof(name));
    }

    public void Test() => Foo("hello");
}

Current codegen for Test:

; Method Program:Test():this
G_M36831_IG01:         
       56                   push     rsi
       4883EC20             sub      rsp, 32
G_M36831_IG02:             
       48BAA85D6DCD69020000 mov      rdx, 0x269CD6D5DA8
       488B12               mov      rdx, gword ptr [rdx]
       4885D2               test     rdx, rdx
       7410                 je       SHORT G_M36831_IG05
G_M36831_IG03:      
       488D4908             lea      rcx, bword ptr [rcx+8]
       E82075515F           call     CORINFO_HELP_ASSIGN_REF
       90                   nop      
G_M36831_IG04:              
       4883C420             add      rsp, 32
       5E                   pop      rsi
       C3                   ret      
G_M36831_IG05:          
       48B9104AFBE7F97F0000 mov      rcx, 0x7FF9E7FB4A10
       E84A79515F           call     CORINFO_HELP_NEWSFAST
       488BF0               mov      rsi, rax
       B901000000           mov      ecx, 1
       48BAE0E235E8F97F0000 mov      rdx, 0x7FF9E835E2E0
       E8A3D9225F           call     CORINFO_HELP_STRCNS
       488BD0               mov      rdx, rax
       488BCE               mov      rcx, rsi
       E85874ADFF           call     System.ArgumentNullException:.ctor(System.String):this
       488BCE               mov      rcx, rsi
       E8B04C235F           call     CORINFO_HELP_THROW
       CC                   int3     

New codegen:

; Method Program:Test():this
G_M36831_IG01:             
G_M36831_IG02:              
       48BAA85D71DA51010000 mov      rdx, 0x151DA715DA8
       488B12               mov      rdx, gword ptr [rdx]
       488D4908             lea      rcx, bword ptr [rcx+8]
       E82A75525F           call     CORINFO_HELP_ASSIGN_REF
       90                   nop      
G_M36831_IG03:              
       C3                   ret      
; Total bytes of code: 24

jit-diff (--pmi -f):

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 53004925
Total bytes of diff: 52999397
Total bytes of delta: -5528 (-0.01% of base)
    diff is an improvement.


Top file improvements (bytes):
       -2164 : System.Private.Xml.dasm (-0.06% of base)
        -576 : System.Data.Common.dasm (-0.04% of base)
        -502 : System.Private.DataContractSerialization.dasm (-0.07% of base)
        -385 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.01% of base)
        -345 : Newtonsoft.Json.dasm (-0.04% of base)
        -332 : System.Speech.dasm (-0.08% of base)
        -201 : Microsoft.Extensions.Logging.Console.dasm (-0.44% of base)
        -192 : System.DirectoryServices.AccountManagement.dasm (-0.05% of base)
        -178 : System.DirectoryServices.dasm (-0.04% of base)
        -142 : System.Composition.AttributedModel.dasm (-18.11% of base)
        -119 : Microsoft.Extensions.Configuration.Xml.dasm (-1.11% of base)
         -80 : Microsoft.CodeAnalysis.CSharp.dasm (-0.00% of base)
         -72 : System.Private.CoreLib.dasm (-0.00% of base)
         -62 : System.DirectoryServices.Protocols.dasm (-0.07% of base)
         -46 : System.Configuration.ConfigurationManager.dasm (-0.01% of base)
         -42 : System.Net.Primitives.dasm (-0.07% of base)
         -18 : System.Private.Uri.dasm (-0.02% of base)
         -18 : Microsoft.CSharp.dasm (-0.00% of base)
         -18 : xunit.runner.utility.netcoreapp10.dasm (-0.01% of base)
         -15 : CommandLine.dasm (-0.00% of base)

23 total files with Code Size differences (23 improved, 0 regressed), 248 unchanged.

Top method improvements (bytes):
        -954 (-21.54% of base) : System.Private.Xml.dasm - TypeScope:AddSoapEncodedTypes(String)
        -230 (-3.54% of base) : Newtonsoft.Json.dasm - <ExecuteFilter>d__4:MoveNext():bool:this (8 methods)
        -216 (-10.15% of base) : System.Private.Xml.dasm - DatatypeImplementation:CreateBuiltinTypes()
        -216 (-11.99% of base) : System.Private.Xml.dasm - Preprocessor:GetBuildInSchema():XmlSchema
        -144 (-13.30% of base) : System.Private.Xml.dasm - XmlSchemaValidator:BuildXsiAttributes()
        -144 (-16.29% of base) : System.Speech.dasm - SAPICategories:CompareTokenVersions(ObjectToken,ObjectToken,byref):int
        -119 (-4.45% of base) : Microsoft.Extensions.Configuration.Xml.dasm - XmlStreamConfigurationProvider:Read(Stream,XmlDocumentDecryptor):IDictionary`2
        -115 (-5.94% of base) : Newtonsoft.Json.dasm - <ExecuteFilter>d__12:MoveNext():bool:this
         -94 (-6.62% of base) : System.Private.Xml.dasm - XmlSchemaObjectComparer:NameOf(XmlSchemaObject):XmlQualifiedName
         -72 (-7.28% of base) : System.Speech.dasm - VoiceInfo:.ctor(ObjectToken):this
         -71 (-67.62% of base) : System.Composition.AttributedModel.dasm - SharedAttribute:.ctor():this
         -71 (-60.68% of base) : System.Composition.AttributedModel.dasm - SharedAttribute:.ctor(String):this
         -67 (-30.59% of base) : Microsoft.Extensions.Logging.Console.dasm - JsonConsoleFormatter:.ctor(IOptionsMonitor`1):this
         -67 (-30.59% of base) : Microsoft.Extensions.Logging.Console.dasm - SimpleConsoleFormatter:.ctor(IOptionsMonitor`1):this
         -67 (-30.59% of base) : Microsoft.Extensions.Logging.Console.dasm - SystemdConsoleFormatter:.ctor(IOptionsMonitor`1):this
         -62 (-12.53% of base) : System.DirectoryServices.Protocols.dasm - AddRequest:.ctor(String,String):this
         -60 (-8.16% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Scanner:ScanInterpolatedStringPunctuation():SyntaxToken:this
         -50 (-1.44% of base) : System.DirectoryServices.AccountManagement.dasm - ADStoreCtx:GetGroupsMemberOf(Principal,StoreCtx):ResultSet:this
         -46 (-2.54% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Scanner:ScanXmlElementInXmlDoc(int):SyntaxToken:this
         -44 (-1.07% of base) : System.Private.Xml.dasm - ReflectionXmlSerializationReader:WriteLiteralStructMethod(StructMapping,bool,bool,String):Object:this

Top method improvements (percentages):
         -71 (-67.62% of base) : System.Composition.AttributedModel.dasm - SharedAttribute:.ctor():this
         -71 (-60.68% of base) : System.Composition.AttributedModel.dasm - SharedAttribute:.ctor(String):this
         -28 (-50.00% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Scanner:MakeEndOfInterpolatedStringToken():SyntaxToken:this
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlChars:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlDateTime:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlDecimal:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlDouble:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlGuid:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlInt16:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlInt32:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlInt64:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlMoney:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlSingle:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlString:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlXml:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlBinary:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlBoolean:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlByte:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -36 (-32.14% of base) : System.Data.Common.dasm - SqlBytes:GetXsdType(XmlSchemaSet):XmlQualifiedName
         -67 (-30.59% of base) : Microsoft.Extensions.Logging.Console.dasm - JsonConsoleFormatter:.ctor(IOptionsMonitor`1):this

142 total methods with Code Size differences (142 improved, 0 regressed), 268410 unchanged.

@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 20, 2021
@davidfowl
Copy link
Member

Wow this looks great

@ShreyasJejurkar
Copy link
Contributor

ShreyasJejurkar commented Mar 21, 2021

Hi @EgorBo, this looks amazing! I always like folding code as much as possible, for faster results!
Some days ago, I pinged you with one of my finding, I tried to implement that but I failed because am quite to JIT stuff! 😅
Hope you will have some thoughts on it like it's better or bad to have this much of folding.
Link to that tweet => https://twitter.com/MCCshreyas/status/1350021589987778566

src/coreclr/jit/valuenum.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/valuenumfuncs.h Show resolved Hide resolved
@EgorBo
Copy link
Member Author

EgorBo commented Mar 21, 2021

@MCCshreyas feel free to file an issue 🙂

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

@benaadams
Copy link
Member

🥳

@EgorBo EgorBo merged commit 96e45f3 into dotnet:main Mar 22, 2021
@JulieLeeMSFT JulieLeeMSFT added this to the 6.0.0 milestone Mar 24, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 23, 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.

7 participants