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: Non-void ThrowHelpers #48589

Merged
merged 19 commits into from
Mar 10, 2021
Merged

JIT: Non-void ThrowHelpers #48589

merged 19 commits into from
Mar 10, 2021

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Feb 22, 2021

If a method is "never returns" and has a non-void type - its basic-block won't be cold and converted to BBJ_THROW.

public void Test(bool condition)
{
    Console.WriteLine(condition ? SomeProperty : 100);
}

public int SomeProperty => 
    throw new NotImplementedException("not implemented on this platform");

Codegen:

; Method P:Test(bool):this
G_M12917_IG01:             
       4883EC28             sub      rsp, 40
G_M12917_IG02:              
       84D2                 test     dl, dl
       7507                 jne      SHORT G_M12917_IG04
G_M12917_IG03:              
       B964000000           mov      ecx, 100
       EB07                 jmp      SHORT G_M12917_IG05
G_M12917_IG04:              
       E89CE1FFFF           call     P:get_SomeProperty():int:this
       8BC8                 mov      ecx, eax
G_M12917_IG05:              
       4883C428             add      rsp, 40
       E951F5FFFF           jmp      System.Console:WriteLine(int)
; Total bytes of code: 31

New codegen:

; Method P:Test(System.Object):int:this
G_M12917_IG01:             
       4883EC28             sub      rsp, 40
G_M12917_IG02:              
       84D2                 test     dl, dl
       750E                 jne      SHORT G_M12917_IG04
       B964000000           mov      ecx, 100
G_M12917_IG03:              
       4883C428             add      rsp, 40
       E9B2F6FFFF           jmp      System.Console:WriteLine(int)
G_M12917_IG04:              
       E835DBFFFF           call     P:get_SomeProperty():int:this
       CC                   int3     
; Total bytes of code: 28

Jit-diff (--pmi -f):

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

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 53821921
Total bytes of diff: 53816840
Total bytes of delta: -5081 (-0.01% of base)
    diff is an improvement.


Top file regressions (bytes):
          23 : Microsoft.Diagnostics.Tracing.TraceEvent.dasm (0.00% of base)

Top file improvements (bytes):
       -1887 : FSharp.Core.dasm (-0.05% of base)
       -1076 : System.Transactions.Local.dasm (-0.95% of base)
       -1062 : System.Private.DataContractSerialization.dasm (-0.14% of base)
        -786 : System.Private.Xml.dasm (-0.02% of base)
         -75 : System.CodeDom.dasm (-0.04% of base)
         -47 : System.Private.CoreLib.dasm (-0.00% of base)
         -46 : System.Net.Primitives.dasm (-0.07% of base)
         -27 : System.Net.Ping.dasm (-0.16% of base)
         -20 : System.Net.Quic.dasm (-0.03% of base)
         -15 : System.Data.Odbc.dasm (-0.01% of base)
         -15 : System.Data.OleDb.dasm (-0.01% of base)
         -15 : Microsoft.CodeAnalysis.dasm (-0.00% of base)
         -14 : System.Net.Http.dasm (-0.00% of base)
         -10 : System.Net.Sockets.dasm (-0.01% of base)
          -5 : Microsoft.CodeAnalysis.VisualBasic.dasm (-0.00% of base)
          -4 : System.Text.Json.dasm (-0.00% of base)

17 total files with Code Size differences (16 improved, 1 regressed), 254 unchanged.

Top method regressions (bytes):
         123 ( 3.11% of base) : FSharp.Core.dasm - ArrayModule:transposeArrays(ref):ref (8 methods)
          41 ( 1.24% of base) : Microsoft.Diagnostics.Tracing.TraceEvent.dasm - TraceEvent:PayloadString(int,IFormatProvider):String:this
          25 ( 0.96% of base) : FSharp.Core.dasm - ArrayModule:Skip(int,ref):ref (8 methods)
          21 ( 0.37% of base) : FSharp.Core.dasm - ArrayModule:GetSubArray(ref,int,int):ref (8 methods)
          11 ( 0.29% of base) : FSharp.Core.dasm - ArrayModule:Zip(ref,ref):ref (8 methods)
          11 ( 4.49% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):__Canon
          11 ( 5.16% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):Vector`1
          11 ( 6.04% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):Nullable`1
           8 ( 0.32% of base) : FSharp.Core.dasm - ArrayModule:Iterate2(FSharpFunc`2,ref,ref) (8 methods)
           8 ( 0.32% of base) : FSharp.Core.dasm - ArrayModule:IterateIndexed2(FSharpFunc`2,ref,ref) (8 methods)
           8 ( 0.32% of base) : FSharp.Core.dasm - ArrayModule:Exists2(FSharpFunc`2,ref,ref):bool (8 methods)
           8 ( 0.32% of base) : FSharp.Core.dasm - ArrayModule:ForAll2(FSharpFunc`2,ref,ref):bool (8 methods)
           7 ( 0.43% of base) : System.Private.CoreLib.dasm - ModuleBuilder:GetMethodTokenNoLock(MethodInfo,bool):int:this
           7 ( 4.14% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):double
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToInt64(DateTime):long
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToUInt64(DateTime):long
           5 (16.67% of base) : System.Private.CoreLib.dasm - Convert:ToSingle(DateTime):float
           5 (16.67% of base) : System.Private.CoreLib.dasm - Convert:ToDouble(DateTime):double
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToByte(DateTime):ubyte
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToInt16(DateTime):short

Top method improvements (bytes):
        -661 (-5.01% of base) : FSharp.Core.dasm - Array2DModule:CopyTo(ref,int,int,ref,int,int,int,int) (8 methods)
        -449 (-44.37% of base) : System.Transactions.Local.dasm - TransactionStatePromoted:EnterState(InternalTransaction):this
        -308 (-19.82% of base) : System.Private.DataContractSerialization.dasm - XsdDataContractExporter:Export(ICollection`1):this (2 methods)
        -306 (-18.04% of base) : System.Private.DataContractSerialization.dasm - XsdDataContractExporter:CanExport(ICollection`1):bool:this (2 methods)
        -198 (-39.84% of base) : System.Private.DataContractSerialization.dasm - XsdDataContractExporter:AddKnownTypes():this
        -154 (-12.01% of base) : System.Private.Xml.dasm - <ReadElementContentAsBase64Async>d__38:MoveNext():this
        -154 (-12.01% of base) : System.Private.Xml.dasm - <ReadElementContentAsBinHexAsync>d__39:MoveNext():this
        -148 (-14.99% of base) : System.Private.Xml.dasm - <ReadContentAsBase64Async>d__36:MoveNext():this
        -148 (-14.99% of base) : System.Private.Xml.dasm - <ReadContentAsBinHexAsync>d__37:MoveNext():this
        -107 (-7.29% of base) : FSharp.Core.dasm - List:takeFreshConsTail(FSharpList`1,int,FSharpList`1) (8 methods)
        -106 (-38.69% of base) : System.Private.Xml.dasm - XmlEventCache:Dispose(bool):this
         -91 (-15.24% of base) : FSharp.Core.dasm - RuntimeHelpers:getCurr@311-1(FSharpRef`1,FSharpRef`1,Unit):IEnumerable`1 (8 methods)
         -77 (-4.42% of base) : System.Transactions.Local.dasm - TransactionManager:Reenlist(Guid,ref,IEnlistmentNotification):Enlistment
         -74 (-13.05% of base) : System.Transactions.Local.dasm - TransactionInterop:GetTransactionFromExportCookie(ref):Transaction
         -72 (-75.79% of base) : System.Private.DataContractSerialization.dasm - XmlObjectSerializerReadContextComplex:ResolveDataContractInSharedTypeMode(String,String,byref,byref):DataContract:this
         -64 (-65.98% of base) : System.Private.DataContractSerialization.dasm - XsdDataContractExporter:Export():this
         -63 (-18.42% of base) : System.Transactions.Local.dasm - TransactionInterop:GetExportCookie(Transaction,ref):ref
         -56 (-25.69% of base) : System.Transactions.Local.dasm - TransactionInterop:GetTransmitterPropagationToken(Transaction):ref
         -56 (-25.69% of base) : System.Transactions.Local.dasm - TransactionInterop:GetDtcTransaction(Transaction):IDtcTransaction
         -56 (-28.00% of base) : System.Transactions.Local.dasm - TransactionInterop:GetTransactionFromDtcTransaction(IDtcTransaction):Transaction

Top method regressions (percentages):
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToInt64(DateTime):long
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToUInt64(DateTime):long
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToByte(DateTime):ubyte
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToInt16(DateTime):short
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToUInt16(DateTime):ushort
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToInt32(DateTime):int
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToUInt32(DateTime):int
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToBoolean(DateTime):bool
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToChar(DateTime):ushort
           5 (18.52% of base) : System.Private.CoreLib.dasm - Convert:ToSByte(DateTime):byte
           5 (16.67% of base) : System.Private.CoreLib.dasm - Convert:ToSingle(DateTime):float
           5 (16.67% of base) : System.Private.CoreLib.dasm - Convert:ToDouble(DateTime):double
          11 ( 6.04% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):Nullable`1
          11 ( 5.16% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):Vector`1
          11 ( 4.49% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):__Canon
           7 ( 4.14% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):double
         123 ( 3.11% of base) : FSharp.Core.dasm - ArrayModule:transposeArrays(ref):ref (8 methods)
           5 ( 2.84% of base) : System.Net.Primitives.dasm - IPAddress:IsLoopback(IPAddress):bool
           3 ( 1.83% of base) : FSharp.Core.dasm - IEnumerator:getCurrent@238(FSharpFunc`2,FSharpRef`1,FSharpRef`1,Unit):int
           1 ( 1.82% of base) : System.Transactions.Local.dasm - TransactionStatePromotedBase:CreateBlockingClone(InternalTransaction):this

Top method improvements (percentages):
         -72 (-75.79% of base) : System.Private.DataContractSerialization.dasm - XmlObjectSerializerReadContextComplex:ResolveDataContractInSharedTypeMode(String,String,byref,byref):DataContract:this
         -64 (-65.98% of base) : System.Private.DataContractSerialization.dasm - XsdDataContractExporter:Export():this
        -449 (-44.37% of base) : System.Transactions.Local.dasm - TransactionStatePromoted:EnterState(InternalTransaction):this
        -198 (-39.84% of base) : System.Private.DataContractSerialization.dasm - XsdDataContractExporter:AddKnownTypes():this
        -106 (-38.69% of base) : System.Private.Xml.dasm - XmlEventCache:Dispose(bool):this
         -52 (-37.96% of base) : System.Transactions.Local.dasm - TransactionInterop:GetWhereabouts():ref
         -32 (-37.21% of base) : FSharp.Core.dasm - BaseRangeEnumerator`1:getCurrent(BaseRangeEnumerator`1):ubyte:this
         -32 (-37.21% of base) : FSharp.Core.dasm - BaseRangeEnumerator`1:getCurrent(BaseRangeEnumerator`1):short:this
         -32 (-37.21% of base) : FSharp.Core.dasm - BaseRangeEnumerator`1:getCurrent(BaseRangeEnumerator`1):int:this
         -32 (-37.21% of base) : FSharp.Core.dasm - BaseRangeEnumerator`1:getCurrent(BaseRangeEnumerator`1):long:this
         -32 (-37.21% of base) : FSharp.Core.dasm - BaseRangeEnumerator`1:getCurrent(BaseRangeEnumerator`1):Nullable`1:this
         -32 (-35.96% of base) : FSharp.Core.dasm - BaseRangeEnumerator`1:getCurrent(BaseRangeEnumerator`1):double:this
         -31 (-35.23% of base) : System.Net.Primitives.dasm - IPAddress:.ctor(ref,long):this
          -9 (-29.03% of base) : System.Private.DataContractSerialization.dasm - XsdDataContractExporter:AddType(Type):this
         -56 (-28.00% of base) : System.Transactions.Local.dasm - TransactionInterop:GetTransactionFromDtcTransaction(IDtcTransaction):Transaction
         -20 (-25.97% of base) : System.Net.Primitives.dasm - IPAddress:.ctor(ref):this
         -56 (-25.69% of base) : System.Transactions.Local.dasm - TransactionInterop:GetTransmitterPropagationToken(Transaction):ref
         -56 (-25.69% of base) : System.Transactions.Local.dasm - TransactionInterop:GetDtcTransaction(Transaction):IDtcTransaction
         -11 (-23.91% of base) : FSharp.Core.dasm - OperatorIntrinsics:current@5245(VariableStepIntegralRangeState`1,Unit):int
         -11 (-23.91% of base) : FSharp.Core.dasm - OperatorIntrinsics:current@5245-3(VariableStepIntegralRangeState`1,Unit):int

240 total methods with Code Size differences (203 improved, 37 regressed), 267870 unchanged.
--------------------------------------------------------------------------------
Completed analysis in 31.16s

Regressions -- more blocks are now cold (additional jumps)
Example: https://www.diffchecker.com/xiYGrCam (ToSingle will throw an exception for T=DateTime)

@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 Feb 22, 2021
@EgorBo EgorBo changed the title JIT: Make throw helpers like methods cold, don't materialize string literals JIT: Make ThrowHelper calls cold, don't materialize string literals Feb 22, 2021
@EgorBo EgorBo changed the title JIT: Make ThrowHelper calls cold, don't materialize string literals JIT: Non-void ThrowHelpers Feb 22, 2021
@EgorBo EgorBo marked this pull request as draft February 22, 2021 12:31
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/flowgraph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Show resolved Hide resolved
@EgorBo EgorBo marked this pull request as ready for review February 24, 2021 14:18
@EgorBo
Copy link
Member Author

EgorBo commented Feb 24, 2021

@dotnet/jit-contrib PTAL, will address LazyStringHelper problem separately, the new jit-diff is -5081

src/coreclr/jit/utils.cpp Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Do you need AllowUnsafeBlocks for your test case?

Base automatically changed from master to main March 1, 2021 09:08
@EgorBo EgorBo merged commit beb14c1 into dotnet:main Mar 10, 2021
thaystg added a commit to thaystg/runtime that referenced this pull request Mar 10, 2021
* upstream/main: (83 commits)
  Fix a crash in llvm if the sreg of a setret is not set because the methods ends with a throw. (dotnet#49122)
  [macOS-arm64] Disable failing libraries tests (dotnet#49400)
  improve PriorityQueue documentation (dotnet#49392)
  [wasm] Fix debugger tests (dotnet#49206)
  [mono] Fix the emission of EnumEqualityComparer instances into the corlib AOT image. (dotnet#49402)
  jitutils M2M renaming reaction (dotnet#49430)
  WinHttpHandler: Read HTTP/2 trailing headers
  [RyuJIT] Make casthelpers cold for sealed classes (dotnet#49295)
  JIT: Non-void ThrowHelpers (dotnet#48589)
  Update package index for servicing (dotnet#49417)
  Remove unnecessary check on polymorphic serialization (dotnet#48464)
  Remove release build cron triggers from jitstress jobs (dotnet#49333)
  [main] Update dependencies from dotnet/arcade dotnet/llvm-project dotnet/runtime-assets (dotnet#49359)
  Implement AppleCryptoNative_X509GetRawData using SecCertificateCopyData
  [AndroidCrypto] Support a zero-length salt for HMACs. (dotnet#49384)
  Use managed implementation of pbkdf2 for Android's one-shot implementation. (dotnet#49314)
  Make 303 redirects do GET like Net Framework (dotnet#49095)
  Make sure event generation is incremental (dotnet#48903)
  Add amd and Surface arm64 perf runs (dotnet#49389)
  Enregister EH var that are single def (dotnet#47307)
  ...
@ghost ghost locked as resolved and limited conversation to collaborators Apr 9, 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.

None yet

4 participants