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

Unify JIT interface getTypeLayout between VM and crossgen2; refine what is considered "significant padding" #88238

Merged
merged 12 commits into from
Jul 1, 2023

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jun 30, 2023

  • Stop considering auto layout types to have significant padding in the VM. This was already the behavior in crossgen2. This fixes one of the points of CORINFO_FLG_CUSTOMLAYOUT semantics #71711
  • Remove special case where we never considered structs with GC pointers to have significant padding. After the above change this has no additional diffs.
  • Fix the inline array layout expansion; the numFields of the inline array node was computed incorrectly, and the parent indices of descendants were not updated correctly

Example C#:

private (long, int) _tuple;

[MethodImpl(MethodImplOptions.NoInlining)]
private void Foo()
{
    _tuple = (5, 10);
}

Before:

;  V00 this         [V00,T00] (  3,  3   )     ref  ->  rcx         this class-hnd single-def
;# V01 OutArgs      [V01    ] (  1,  1   )  struct ( 0) [rsp+00H]   do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;  V02 tmp1         [V02    ] (  4,  8   )  struct (16) [rsp+08H]   do-not-enreg[SB] ld-addr-op "NewObj constructor temp"
;  V03 tmp2         [V03,T01] (  3,  5   )    long  ->  [rsp+08H]   do-not-enreg[] "field V02.Item1 (fldOffset=0x0)" P-DEP
;  V04 tmp3         [V04,T02] (  3,  5   )     int  ->  [rsp+10H]   do-not-enreg[] "field V02.Item2 (fldOffset=0x8)" P-DEP
;
; Lcl frame size = 24

G_M52879_IG01:  ;; offset=0000H
       sub      rsp, 24
       vzeroupper 
						;; size=7 bbWeight=1 PerfScore 1.25
G_M52879_IG02:  ;; offset=0007H
       vxorps   xmm0, xmm0, xmm0
       vmovups  xmmword ptr [rsp+08H], xmm0
       mov      qword ptr [rsp+08H], 5
       mov      dword ptr [rsp+10H], 10
       vmovups  xmm0, xmmword ptr [rsp+08H]
       vmovups  xmmword ptr [rcx+08H], xmm0
						;; size=38 bbWeight=1 PerfScore 8.33
G_M52879_IG03:  ;; offset=002DH
       add      rsp, 24
       ret      
						;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 50, prolog size 7, PerfScore 15.83, instruction count 10, allocated bytes for code 50 (MethodHash=1da13170) for method Program:Foo():this (FullOpts)

After:

;  V00 this         [V00,T00] (  4,  4   )     ref  ->  rcx         this class-hnd single-def
;# V01 OutArgs      [V01    ] (  1,  1   )  struct ( 0) [rsp+00H]   do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V02 tmp1         [V02    ] (  0,  0   )  struct (16) zero-ref    ld-addr-op "NewObj constructor temp"
;* V03 tmp2         [V03,T01] (  0,  0   )    long  ->  zero-ref    "field V02.Item1 (fldOffset=0x0)" P-INDEP
;* V04 tmp3         [V04,T02] (  0,  0   )     int  ->  zero-ref    "field V02.Item2 (fldOffset=0x8)" P-INDEP
;
; Lcl frame size = 0

G_M52879_IG01:  ;; offset=0000H
						;; size=0 bbWeight=1 PerfScore 0.00
G_M52879_IG02:  ;; offset=0000H
       mov      qword ptr [rcx+08H], 5
       mov      dword ptr [rcx+10H], 10
						;; size=15 bbWeight=1 PerfScore 2.00
G_M52879_IG03:  ;; offset=000FH
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00

; Total bytes of code 16, prolog size 0, PerfScore 4.60, instruction count 3, allocated bytes for code 16 (MethodHash=1da13170) for method Program:Foo():this (FullOpts)

This fixes the other point in dotnet#71711 by adjusting the check in the VM
for whether types have significant padding or not. It effectively
unifies the VM logic with crossgen2's check.

Example C#:
```csharp
private (long, int) _tuple;

[MethodImpl(MethodImplOptions.NoInlining)]
private void Foo()
{
    _tuple = (5, 10);
}
```

Before:
```asm
;  V00 this         [V00,T00] (  3,  3   )     ref  ->  rcx         this class-hnd single-def
;# V01 OutArgs      [V01    ] (  1,  1   )  struct ( 0) [rsp+00H]   do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;  V02 tmp1         [V02    ] (  4,  8   )  struct (16) [rsp+08H]   do-not-enreg[SB] ld-addr-op "NewObj constructor temp"
;  V03 tmp2         [V03,T01] (  3,  5   )    long  ->  [rsp+08H]   do-not-enreg[] "field V02.Item1 (fldOffset=0x0)" P-DEP
;  V04 tmp3         [V04,T02] (  3,  5   )     int  ->  [rsp+10H]   do-not-enreg[] "field V02.Item2 (fldOffset=0x8)" P-DEP
;
; Lcl frame size = 24

G_M52879_IG01:  ;; offset=0000H
       sub      rsp, 24
       vzeroupper
						;; size=7 bbWeight=1 PerfScore 1.25
G_M52879_IG02:  ;; offset=0007H
       vxorps   xmm0, xmm0, xmm0
       vmovups  xmmword ptr [rsp+08H], xmm0
       mov      qword ptr [rsp+08H], 5
       mov      dword ptr [rsp+10H], 10
       vmovups  xmm0, xmmword ptr [rsp+08H]
       vmovups  xmmword ptr [rcx+08H], xmm0
						;; size=38 bbWeight=1 PerfScore 8.33
G_M52879_IG03:  ;; offset=002DH
       add      rsp, 24
       ret
						;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 50, prolog size 7, PerfScore 15.83, instruction count 10, allocated bytes for code 50 (MethodHash=1da13170) for method Program:Foo():this (FullOpts)
```

After:
```asm
;  V00 this         [V00,T00] (  4,  4   )     ref  ->  rcx         this class-hnd single-def
;# V01 OutArgs      [V01    ] (  1,  1   )  struct ( 0) [rsp+00H]   do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V02 tmp1         [V02    ] (  0,  0   )  struct (16) zero-ref    ld-addr-op "NewObj constructor temp"
;* V03 tmp2         [V03,T01] (  0,  0   )    long  ->  zero-ref    "field V02.Item1 (fldOffset=0x0)" P-INDEP
;* V04 tmp3         [V04,T02] (  0,  0   )     int  ->  zero-ref    "field V02.Item2 (fldOffset=0x8)" P-INDEP
;
; Lcl frame size = 0

G_M52879_IG01:  ;; offset=0000H
						;; size=0 bbWeight=1 PerfScore 0.00
G_M52879_IG02:  ;; offset=0000H
       mov      qword ptr [rcx+08H], 5
       mov      dword ptr [rcx+10H], 10
						;; size=15 bbWeight=1 PerfScore 2.00
G_M52879_IG03:  ;; offset=000FH
       ret
						;; size=1 bbWeight=1 PerfScore 1.00

; Total bytes of code 16, prolog size 0, PerfScore 4.60, instruction count 3, allocated bytes for code 16 (MethodHash=1da13170) for method Program:Foo():this (FullOpts)
```
@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 Jun 30, 2023
@ghost ghost assigned jakobbotsch Jun 30, 2023
@ghost
Copy link

ghost commented Jun 30, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

This fixes the other point in #71711 by adjusting the check in the VM for whether types have significant padding or not. It effectively unifies the VM logic with crossgen2's check.

Example C#:

private (long, int) _tuple;

[MethodImpl(MethodImplOptions.NoInlining)]
private void Foo()
{
    _tuple = (5, 10);
}

Before:

;  V00 this         [V00,T00] (  3,  3   )     ref  ->  rcx         this class-hnd single-def
;# V01 OutArgs      [V01    ] (  1,  1   )  struct ( 0) [rsp+00H]   do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;  V02 tmp1         [V02    ] (  4,  8   )  struct (16) [rsp+08H]   do-not-enreg[SB] ld-addr-op "NewObj constructor temp"
;  V03 tmp2         [V03,T01] (  3,  5   )    long  ->  [rsp+08H]   do-not-enreg[] "field V02.Item1 (fldOffset=0x0)" P-DEP
;  V04 tmp3         [V04,T02] (  3,  5   )     int  ->  [rsp+10H]   do-not-enreg[] "field V02.Item2 (fldOffset=0x8)" P-DEP
;
; Lcl frame size = 24

G_M52879_IG01:  ;; offset=0000H
       sub      rsp, 24
       vzeroupper 
						;; size=7 bbWeight=1 PerfScore 1.25
G_M52879_IG02:  ;; offset=0007H
       vxorps   xmm0, xmm0, xmm0
       vmovups  xmmword ptr [rsp+08H], xmm0
       mov      qword ptr [rsp+08H], 5
       mov      dword ptr [rsp+10H], 10
       vmovups  xmm0, xmmword ptr [rsp+08H]
       vmovups  xmmword ptr [rcx+08H], xmm0
						;; size=38 bbWeight=1 PerfScore 8.33
G_M52879_IG03:  ;; offset=002DH
       add      rsp, 24
       ret      
						;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 50, prolog size 7, PerfScore 15.83, instruction count 10, allocated bytes for code 50 (MethodHash=1da13170) for method Program:Foo():this (FullOpts)

After:

;  V00 this         [V00,T00] (  4,  4   )     ref  ->  rcx         this class-hnd single-def
;# V01 OutArgs      [V01    ] (  1,  1   )  struct ( 0) [rsp+00H]   do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V02 tmp1         [V02    ] (  0,  0   )  struct (16) zero-ref    ld-addr-op "NewObj constructor temp"
;* V03 tmp2         [V03,T01] (  0,  0   )    long  ->  zero-ref    "field V02.Item1 (fldOffset=0x0)" P-INDEP
;* V04 tmp3         [V04,T02] (  0,  0   )     int  ->  zero-ref    "field V02.Item2 (fldOffset=0x8)" P-INDEP
;
; Lcl frame size = 0

G_M52879_IG01:  ;; offset=0000H
						;; size=0 bbWeight=1 PerfScore 0.00
G_M52879_IG02:  ;; offset=0000H
       mov      qword ptr [rcx+08H], 5
       mov      dword ptr [rcx+10H], 10
						;; size=15 bbWeight=1 PerfScore 2.00
G_M52879_IG03:  ;; offset=000FH
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00

; Total bytes of code 16, prolog size 0, PerfScore 4.60, instruction count 3, allocated bytes for code 16 (MethodHash=1da13170) for method Program:Foo():this (FullOpts)
Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch jakobbotsch requested a review from jkotas June 30, 2023 10:34
@jakobbotsch
Copy link
Member Author

Will post jit-diffs a bit later tonight.

@jkotas
Copy link
Member

jkotas commented Jun 30, 2023

It effectively unifies the VM logic with crossgen2's check.

The crossgen2 check excludes byref-like types as well.

// layout attributes. This retains the more liberal treatment and
// lets the JIT still optimize these cases.
if (!pMT->ContainsPointers() && pMT != g_TypedReferenceMT)
if (pClass->HasExplicitFieldOffsetLayout() || (pClass->IsManagedSequential() && pClass->HasExplicitSize()) ||
Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to skip this for InlineArrays with GC references? I would expect that the padding in InlineArrays must be always treated as significant by the JIT.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's safe since getTypeLayout expands inline array layouts precisely; having the check is conservative. However, today it does not matter for CQ as the JIT does not recognize the IR produced for even constant-based indexing as field accesses. But given I'm here I will remove the check anyway.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Number of outstanding questions.

@ghost ghost added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jun 30, 2023
@jakobbotsch
Copy link
Member Author

It effectively unifies the VM logic with crossgen2's check.

The crossgen2 check excludes byref-like types as well.

Hmm, I'm not sure why this difference exists. Do you have any preferences on the direction in which to unify the VM and crossgen2?

@jakobbotsch
Copy link
Member Author

Actually I introduced that difference in #87917. The correct check here that corresponds to the previous JIT check is the VM version.

jakobbotsch and others added 7 commits June 30, 2023 21:41
…#88182)

Introduce a LocationAccess helper class to create derived accesses off
of the destination and source locations for the store. Unify all the
code that looks for regularly promoted fields in this class, and use it
consistently for all the derived accesses.

Also update terminology from "assignment" to "store" in a few places,
and add a "(last use)" string for fields when decomposing block stores.
…l promotion

The remainder may be separated by a bit of padding but still fit into a
primitive; in this case it is still beneficial to copy it all as a
primitive, instead of falling back to a full block copy.

Example.
Before:
```
Processing block operation [000737] that involves replacements
  dst+000 <- V101 (V47.[000..008)) (last use)
  dst+008 <- V102 (V47.[008..016)) (last use)
  dst+016 <- V103 (V47.[016..024)) (last use)
  dst+024 <- V104 (V47.[024..028)) (last use)
  dst+028 <- V105 (V47.[028..029)) (last use)
  Block op remainder: [032..033) [036..040)
  => Remainder strategy: retain a full block op

```

After:
```
Processing block operation [000737] that involves replacements
  dst+000 <- V101 (V47.[000..008)) (last use)
  dst+008 <- V102 (V47.[008..016)) (last use)
  dst+016 <- V103 (V47.[016..024)) (last use)
  dst+024 <- V104 (V47.[024..028)) (last use)
  dst+028 <- V105 (V47.[028..029)) (last use)
  Block op remainder: [032..033) [036..040)
  => Remainder strategy: long at +032
```

This leads to ASM diffs like the following:
```diff
        xor      edx, edx
        mov      qword ptr [rsp+148H], rdx
-						;; size=20 bbWeight=0.33 PerfScore 1.57
-G_M6338_IG24:        ; bbWeight=0.33, nogc, extend
-       vmovdqu  ymm0, ymmword ptr [rsp+128H]
-       vmovdqu  ymmword ptr [rsp+D8H], ymm0
-       mov      rdx, qword ptr [rsp+148H]
-       mov      qword ptr [rsp+F8H], rdx
-						;; size=34 bbWeight=0.33 PerfScore 2.32
-G_M6338_IG25:        ; bbWeight=0.33, extend
        mov      gword ptr [rsp+D8H], r12
-       xor      rdx, rdx
-       ; gcrRegs +[rdx]
+						;; size=28 bbWeight=0.33 PerfScore 1.90
+G_M6338_IG24:        ; bbWeight=0.33, gcrefRegs=1028 {rbx rbp r12}, byrefRegs=4000 {r14}, byref
        mov      gword ptr [rsp+E0H], rdx
-						;; size=18 bbWeight=0.33 PerfScore 0.75
-G_M6338_IG26:        ; bbWeight=0.33, gcrefRegs=1028 {rbx rbp r12}, byrefRegs=4000 {r14}, byref
-       ; gcrRegs -[rdx]
+						;; size=8 bbWeight=0.33 PerfScore 0.33
+G_M6338_IG25:        ; bbWeight=0.33, gcrefRegs=1028 {rbx rbp r12}, byrefRegs=4000 {r14}, byref
        mov      gword ptr [rsp+E8H], rdx
        mov      dword ptr [rsp+F0H], r9d
        mov      byte  ptr [rsp+F4H], 0
+						;; size=24 bbWeight=0.33 PerfScore 0.99
+G_M6338_IG26:        ; bbWeight=0.33, gcrefRegs=1028 {rbx rbp r12}, byrefRegs=4000 {r14}, byref
+       mov      qword ptr [rsp+F8H], rdx
```

We have to be careful, however, since the covering segment can now
contain promoted fields. If this happens we need to make sure we write
the promoted field _after_ the remainder.

Unfortunately doing this requires quite a bit of refactoring. I have
extracted all common code for handling creation of derived accesses of
the destination/source into a common class called LocationAccess.
@jakobbotsch
Copy link
Member Author

jakobbotsch commented Jun 30, 2023

@jkotas I've addressed the feedback discussed, removed the special case and inline array check, and also fixed bugs I've noticed for the inline array layout expansion in the computation of the number of fields, and for the parent index of each repeated element. Can you please take another look?

PMI diffs with #88109 applied in base and diff (without it, giving the JIT more precise information can cause some regressions):

Found 372 files with textual diffs.

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 66387996
Total bytes of diff: 66357338
Total bytes of delta: -30658 (-0.05 % of base)
Total relative delta: -122.28
    diff is an improvement.
    relative diff is an improvement.


Total byte diff includes 559 bytes from reconciling methods
        Base had    7 unique methods,      871 unique bytes
        Diff had    9 unique methods,     1430 unique bytes

Top file regressions (bytes):
         304 : ILCompiler.Reflection.ReadyToRun.dasm (0.13% of base)
         246 : System.DirectoryServices.dasm (0.06% of base)
         150 : System.Management.dasm (0.04% of base)
          98 : System.Net.HttpListener.dasm (0.05% of base)
          77 : System.Diagnostics.EventLog.dasm (0.06% of base)
          56 : Microsoft.CodeAnalysis.VisualBasic.dasm (0.00% of base)
          49 : System.Security.Principal.Windows.dasm (0.08% of base)
          10 : System.Net.Ping.dasm (0.02% of base)
           8 : System.DirectoryServices.Protocols.dasm (0.01% of base)
           5 : System.IO.Compression.dasm (0.01% of base)
           3 : System.IO.Packaging.dasm (0.00% of base)
           3 : System.Data.OleDb.dasm (0.00% of base)
           3 : System.Reflection.MetadataLoadContext.dasm (0.00% of base)
           1 : Microsoft.NET.WebAssembly.Webcil.dasm (0.00% of base)

Top file improvements (bytes):
      -18126 : Microsoft.CodeAnalysis.dasm (-0.38% of base)
       -2215 : System.Private.CoreLib.dasm (-0.04% of base)
       -1809 : System.Private.Xml.dasm (-0.05% of base)
       -1265 : System.Net.Http.dasm (-0.15% of base)
       -1001 : xunit.runner.utility.netcoreapp10.dasm (-0.52% of base)
        -950 : Newtonsoft.Json.dasm (-0.10% of base)
        -611 : System.Security.Cryptography.dasm (-0.06% of base)
        -538 : System.Collections.dasm (-0.11% of base)
        -418 : System.Data.Common.dasm (-0.03% of base)
        -399 : System.Text.Json.dasm (-0.03% of base)
        -394 : System.ServiceModel.Syndication.dasm (-0.26% of base)
        -391 : Microsoft.Extensions.Logging.Console.dasm (-0.52% of base)
        -358 : Microsoft.Extensions.Caching.Memory.dasm (-1.76% of base)
        -333 : System.Net.Requests.dasm (-0.26% of base)
        -326 : Newtonsoft.Json.Bson.dasm (-0.30% of base)
        -311 : System.Text.RegularExpressions.dasm (-0.05% of base)
        -254 : System.Formats.Tar.dasm (-0.24% of base)
        -243 : System.Security.Cryptography.Pkcs.dasm (-0.05% of base)
        -210 : System.Formats.Asn1.dasm (-0.23% of base)
        -183 : System.Runtime.Caching.dasm (-0.29% of base)

54 total files with Code Size differences (40 improved, 14 regressed), 223 unchanged.

Top method regressions (bytes):
         523 (      of base) : System.Security.Cryptography.dasm - System.Security.Cryptography.X509Certificates.X500DirectoryStringHelper:ReadAnyAsnString(byref):System.String (FullOpts) (0 base, 1 diff methods)
         348 (      of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo:TryFindSystemTimeZoneById(System.String,byref,byref):int (FullOpts) (0 base, 1 diff methods)
         251 (      of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:.ctor(System.Resources.ResourceManager,int):this (FullOpts) (0 base, 1 diff methods)
         150 ( 0.79% of base) : System.Management.dasm - System.Management.ManagementClassGenerator:GenerateMethods():this (FullOpts)
         123 ( 7.84% of base) : System.DirectoryServices.dasm - System.DirectoryServices.ActiveDirectory.ActiveDirectorySchema:GetAllClasses(System.DirectoryServices.ActiveDirectory.DirectoryContext,System.DirectoryServices.DirectoryEntry,System.String):System.DirectoryServices.ActiveDirectory.ReadOnlyActiveDirectorySchemaClassCollection (FullOpts)
         123 ( 7.90% of base) : System.DirectoryServices.dasm - System.DirectoryServices.ActiveDirectory.ActiveDirectorySchema:GetAllProperties(System.DirectoryServices.ActiveDirectory.DirectoryContext,System.DirectoryServices.DirectoryEntry,System.String):System.DirectoryServices.ActiveDirectory.ReadOnlyActiveDirectorySchemaPropertyCollection (FullOpts)
         108 (      of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo+<>c:<GetSystemTimeZones>b__70_0(System.TimeZoneInfo,System.TimeZoneInfo):int:this (FullOpts) (0 base, 1 diff methods)
          90 (      of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder+<>c:<CreateManifestString>b__19_1(System.Collections.Generic.KeyValuePair`2[int,System.Diagnostics.Tracing.ManifestBuilder+ChannelInfo],System.Collections.Generic.KeyValuePair`2[int,System.Diagnostics.Tracing.ManifestBuilder+ChannelInfo]):int:this (FullOpts) (0 base, 1 diff methods)
          87 (19.00% of base) : ILCompiler.Reflection.ReadyToRun.dasm - Internal.Pgo.PgoProcessor:Merge[System.__Canon,System.Nullable`1[int]](System.ReadOnlySpan`1[Internal.Pgo.PgoSchemaElem[]]):Internal.Pgo.PgoSchemaElem[] (FullOpts)
          87 (10.43% of base) : System.Text.Json.dasm - System.Text.Json.Utf8JsonReader:CopyString(System.Span`1[ubyte]):int:this (FullOpts)
          84 ( 1.78% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.EventSource:CreateManifestAndDescriptors(System.Type,System.String,System.Diagnostics.Tracing.EventSource,int):ubyte[] (FullOpts)
          71 (38.17% of base) : System.Text.Json.dasm - System.Text.Json.Utf8JsonWriter:WriteCommentValue(System.ReadOnlySpan`1[ubyte]):this (FullOpts)
          57 ( 4.71% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:StartEvent(System.String,System.Diagnostics.Tracing.EventAttribute):this (FullOpts)
          53 ( 3.86% of base) : System.Diagnostics.EventLog.dasm - Microsoft.Win32.UnsafeNativeMethods:EvtFormatMessage(System.Diagnostics.Eventing.Reader.EventLogHandle,System.Diagnostics.Eventing.Reader.EventLogHandle,uint,int,Microsoft.Win32.UnsafeNativeMethods+EvtStringVariant[],int,int,ushort[],byref):bool (FullOpts)
          53 ( 3.10% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:AddEventParameter(System.Type,System.String):this (FullOpts)
          52 ( 2.20% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - Microsoft.CodeAnalysis.VisualBasic.Binder:DecodePropertyParameterList(Microsoft.CodeAnalysis.VisualBasic.Symbols.PropertySymbol,Microsoft.CodeAnalysis.VisualBasic.Syntax.ParameterListSyntax,Microsoft.CodeAnalysis.VisualBasic.BindingDiagnosticBag):System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.VisualBasic.Symbols.ParameterSymbol]:this (FullOpts)
          49 ( 3.97% of base) : System.Security.Principal.Windows.dasm - Interop+Advapi32:LsaLookupNames2(Microsoft.Win32.SafeHandles.SafeLsaPolicyHandle,int,int,Interop+Advapi32+MARSHALLED_UNICODE_STRING[],byref,byref):uint (FullOpts)
          49 ( 3.99% of base) : System.Net.HttpListener.dasm - Interop+WebSocket:WebSocketBeginClientHandshake(System.Runtime.InteropServices.SafeHandle,long,uint,long,uint,Interop+WebSocket+HttpHeader[],uint,byref,byref):int (FullOpts)
          49 ( 4.03% of base) : System.Net.HttpListener.dasm - Interop+WebSocket:WebSocketBeginServerHandshake(System.Runtime.InteropServices.SafeHandle,long,long,uint,Interop+WebSocket+HttpHeader[],uint,byref,byref):int (FullOpts)
          43 ( 4.94% of base) : System.Private.CoreLib.dasm - System.Reflection.RuntimeAssembly:GetTypeCore(System.Runtime.CompilerServices.QCallAssembly,System.String,System.ReadOnlySpan`1[System.String],int,System.Runtime.CompilerServices.ObjectHandleOnStack) (FullOpts)

Top method improvements (bytes):
        -523 (-100.00% of base) : System.Security.Cryptography.dasm - System.Security.Cryptography.X509Certificates.X500DictionaryStringHelper:ReadAnyAsnString(byref):System.String (FullOpts) (1 base, 0 diff methods)
        -492 (-6.10% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:CreateManifestString():System.String:this (FullOpts)
        -371 (-63.42% of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo:FindSystemTimeZoneById(System.String):System.TimeZoneInfo (FullOpts)
        -358 (-12.27% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.NodeStateTable`1+Builder[double]:TryModifyEntries(System.Collections.Immutable.ImmutableArray`1[double],System.Collections.Generic.IEqualityComparer`1[double],System.TimeSpan,System.Collections.Immutable.ImmutableArray`1[System.ValueTuple`2[Microsoft.CodeAnalysis.IncrementalGeneratorRunStep,int]],int):bool:this (FullOpts)
        -311 (-37.79% of base) : System.Text.RegularExpressions.dasm - System.Text.RegularExpressions.Regex:RunSingleMatch(int,int,System.ReadOnlySpan`1[ushort],int):System.ValueTuple`4[bool,int,int,int]:this (FullOpts)
        -260 (-8.80% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.NodeStateTable`1+Builder[int]:TryModifyEntries(System.Collections.Immutable.ImmutableArray`1[int],System.Collections.Generic.IEqualityComparer`1[int],System.TimeSpan,System.Collections.Immutable.ImmutableArray`1[System.ValueTuple`2[Microsoft.CodeAnalysis.IncrementalGeneratorRunStep,int]],int):bool:this (FullOpts)
        -259 (-11.86% of base) : Microsoft.Extensions.Caching.Memory.dasm - Microsoft.Extensions.Caching.Memory.MemoryCache:Compact(long,System.Func`2[Microsoft.Extensions.Caching.Memory.CacheEntry,long],Microsoft.Extensions.Caching.Memory.MemoryCache+CoherentState):this (FullOpts)
        -255 (-6.97% of base) : System.Private.Xml.dasm - System.Xml.XmlTextReaderImpl+<_ParseTextAsync>d__534:MoveNext():this (FullOpts)
        -239 (-8.05% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.NodeStateTable`1+Builder[long]:TryModifyEntries(System.Collections.Immutable.ImmutableArray`1[long],System.Collections.Generic.IEqualityComparer`1[long],System.TimeSpan,System.Collections.Immutable.ImmutableArray`1[System.ValueTuple`2[Microsoft.CodeAnalysis.IncrementalGeneratorRunStep,int]],int):bool:this (FullOpts)
        -232 (-8.59% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.CombineNode`2[System.Numerics.Vector`1[float],System.Nullable`1[int]]:UpdateStateTable(Microsoft.CodeAnalysis.DriverStateTable+Builder,Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[System.Numerics.Vector`1[float],System.Nullable`1[int]]],System.Threading.CancellationToken):Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[System.Numerics.Vector`1[float],System.Nullable`1[int]]]:this (FullOpts)
        -223 (-9.39% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.CombineNode`2[long,System.Nullable`1[int]]:UpdateStateTable(Microsoft.CodeAnalysis.DriverStateTable+Builder,Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[long,System.Nullable`1[int]]],System.Threading.CancellationToken):Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[long,System.Nullable`1[int]]]:this (FullOpts)
        -222 (-9.35% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.CombineNode`2[ubyte,System.Nullable`1[int]]:UpdateStateTable(Microsoft.CodeAnalysis.DriverStateTable+Builder,Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[ubyte,System.Nullable`1[int]]],System.Threading.CancellationToken):Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[ubyte,System.Nullable`1[int]]]:this (FullOpts)
        -221 (-9.32% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.CombineNode`2[int,System.Nullable`1[int]]:UpdateStateTable(Microsoft.CodeAnalysis.DriverStateTable+Builder,Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[int,System.Nullable`1[int]]],System.Threading.CancellationToken):Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[int,System.Nullable`1[int]]]:this (FullOpts)
        -221 (-9.31% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.CombineNode`2[short,System.Nullable`1[int]]:UpdateStateTable(Microsoft.CodeAnalysis.DriverStateTable+Builder,Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[short,System.Nullable`1[int]]],System.Threading.CancellationToken):Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[short,System.Nullable`1[int]]]:this (FullOpts)
        -218 (-22.09% of base) : Newtonsoft.Json.dasm - Newtonsoft.Json.Linq.JToken:op_Explicit(Newtonsoft.Json.Linq.JToken):System.Nullable`1[System.DateTimeOffset] (FullOpts)
        -214 (-8.79% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.CombineNode`2[double,System.Nullable`1[int]]:UpdateStateTable(Microsoft.CodeAnalysis.DriverStateTable+Builder,Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[double,System.Nullable`1[int]]],System.Threading.CancellationToken):Microsoft.CodeAnalysis.NodeStateTable`1[System.ValueTuple`2[double,System.Nullable`1[int]]]:this (FullOpts)
        -206 (-7.10% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.NodeStateTable`1+Builder[short]:TryModifyEntries(System.Collections.Immutable.ImmutableArray`1[short],System.Collections.Generic.IEqualityComparer`1[short],System.TimeSpan,System.Collections.Immutable.ImmutableArray`1[System.ValueTuple`2[Microsoft.CodeAnalysis.IncrementalGeneratorRunStep,int]],int):bool:this (FullOpts)
        -204 (-7.05% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.NodeStateTable`1+Builder[ubyte]:TryModifyEntries(System.Collections.Immutable.ImmutableArray`1[ubyte],System.Collections.Generic.IEqualityComparer`1[ubyte],System.TimeSpan,System.Collections.Immutable.ImmutableArray`1[System.ValueTuple`2[Microsoft.CodeAnalysis.IncrementalGeneratorRunStep,int]],int):bool:this (FullOpts)
        -198 (-38.37% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.ForToLoopOperation:MoveNextReversed(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
        -197 (-10.11% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:.ctor(System.String,System.Guid,System.String,System.Resources.ResourceManager,int):this (FullOpts)

Top method regressions (percentages):
         251 (      of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:.ctor(System.Resources.ResourceManager,int):this (FullOpts) (0 base, 1 diff methods)
          19 (      of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:<CreateManifestString>g__GetEnumFields|19_0(System.Type):System.Reflection.FieldInfo[] (FullOpts) (0 base, 1 diff methods)
          90 (      of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder+<>c:<CreateManifestString>b__19_1(System.Collections.Generic.KeyValuePair`2[int,System.Diagnostics.Tracing.ManifestBuilder+ChannelInfo],System.Collections.Generic.KeyValuePair`2[int,System.Diagnostics.Tracing.ManifestBuilder+ChannelInfo]):int:this (FullOpts) (0 base, 1 diff methods)
         523 (      of base) : System.Security.Cryptography.dasm - System.Security.Cryptography.X509Certificates.X500DirectoryStringHelper:ReadAnyAsnString(byref):System.String (FullOpts) (0 base, 1 diff methods)
          16 (      of base) : System.Private.CoreLib.dasm - System.SR:get_Security_CannotReadFileData():System.String (FullOpts) (0 base, 1 diff methods)
         348 (      of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo:TryFindSystemTimeZoneById(System.String,byref,byref):int (FullOpts) (0 base, 1 diff methods)
          35 (      of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo:TryFindSystemTimeZoneById(System.String,byref):bool (FullOpts) (0 base, 1 diff methods)
          40 (      of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo:TryGetTimeZone(System.String,byref,byref,System.TimeZoneInfo+CachedData):int (FullOpts) (0 base, 1 diff methods)
         108 (      of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo+<>c:<GetSystemTimeZones>b__70_0(System.TimeZoneInfo,System.TimeZoneInfo):int:this (FullOpts) (0 base, 1 diff methods)
          26 (72.22% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:CreateManifest():ubyte[]:this (FullOpts)
           8 (66.67% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.OptimizationLevelFacts:get_DefaultValues():System.ValueTuple`2[int,bool] (FullOpts)
          71 (38.17% of base) : System.Text.Json.dasm - System.Text.Json.Utf8JsonWriter:WriteCommentValue(System.ReadOnlySpan`1[ubyte]):this (FullOpts)
          87 (19.00% of base) : ILCompiler.Reflection.ReadyToRun.dasm - Internal.Pgo.PgoProcessor:Merge[System.__Canon,System.Nullable`1[int]](System.ReadOnlySpan`1[Internal.Pgo.PgoSchemaElem[]]):Internal.Pgo.PgoSchemaElem[] (FullOpts)
          87 (10.43% of base) : System.Text.Json.dasm - System.Text.Json.Utf8JsonReader:CopyString(System.Span`1[ubyte]):int:this (FullOpts)
          18 ( 8.11% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:AppendLevelName(System.Text.StringBuilder,int) (FullOpts)
         123 ( 7.90% of base) : System.DirectoryServices.dasm - System.DirectoryServices.ActiveDirectory.ActiveDirectorySchema:GetAllProperties(System.DirectoryServices.ActiveDirectory.DirectoryContext,System.DirectoryServices.DirectoryEntry,System.String):System.DirectoryServices.ActiveDirectory.ReadOnlyActiveDirectorySchemaPropertyCollection (FullOpts)
         123 ( 7.84% of base) : System.DirectoryServices.dasm - System.DirectoryServices.ActiveDirectory.ActiveDirectorySchema:GetAllClasses(System.DirectoryServices.ActiveDirectory.DirectoryContext,System.DirectoryServices.DirectoryEntry,System.String):System.DirectoryServices.ActiveDirectory.ReadOnlyActiveDirectorySchemaClassCollection (FullOpts)
          31 ( 7.67% of base) : ILCompiler.Reflection.ReadyToRun.dasm - Internal.Pgo.PgoProcessor:Merge[double,System.Nullable`1[int]](System.ReadOnlySpan`1[Internal.Pgo.PgoSchemaElem[]]):Internal.Pgo.PgoSchemaElem[] (FullOpts)
          31 ( 7.67% of base) : ILCompiler.Reflection.ReadyToRun.dasm - Internal.Pgo.PgoProcessor:Merge[int,System.Nullable`1[int]](System.ReadOnlySpan`1[Internal.Pgo.PgoSchemaElem[]]):Internal.Pgo.PgoSchemaElem[] (FullOpts)
          31 ( 7.67% of base) : ILCompiler.Reflection.ReadyToRun.dasm - Internal.Pgo.PgoProcessor:Merge[long,System.Nullable`1[int]](System.ReadOnlySpan`1[Internal.Pgo.PgoSchemaElem[]]):Internal.Pgo.PgoSchemaElem[] (FullOpts)

Top method improvements (percentages):
         -99 (-100.00% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.EventWrittenEventArgs:.cctor() (Tier0-MinOpts) (1 base, 0 diff methods)
         -19 (-100.00% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder:<CreateManifestString>g__GetEnumFields|18_0(System.Type):System.Reflection.FieldInfo[] (FullOpts) (1 base, 0 diff methods)
         -90 (-100.00% of base) : System.Private.CoreLib.dasm - System.Diagnostics.Tracing.ManifestBuilder+<>c:<CreateManifestString>b__18_1(System.Collections.Generic.KeyValuePair`2[int,System.Diagnostics.Tracing.ManifestBuilder+ChannelInfo],System.Collections.Generic.KeyValuePair`2[int,System.Diagnostics.Tracing.ManifestBuilder+ChannelInfo]):int:this (FullOpts) (1 base, 0 diff methods)
        -523 (-100.00% of base) : System.Security.Cryptography.dasm - System.Security.Cryptography.X509Certificates.X500DictionaryStringHelper:ReadAnyAsnString(byref):System.String (FullOpts) (1 base, 0 diff methods)
         -16 (-100.00% of base) : System.Private.CoreLib.dasm - System.SR:get_InvalidTimeZone_InvalidRegistryData():System.String (FullOpts) (1 base, 0 diff methods)
         -16 (-100.00% of base) : System.Private.CoreLib.dasm - System.SR:get_Security_CannotReadRegistryData():System.String (FullOpts) (1 base, 0 diff methods)
        -108 (-100.00% of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo+<>c:<GetSystemTimeZones>b__66_0(System.TimeZoneInfo,System.TimeZoneInfo):int:this (FullOpts) (1 base, 0 diff methods)
         -13 (-76.47% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.CodeGen.PrivateImplementationDetails+<>c:<Freeze>b__29_2(System.Collections.Generic.KeyValuePair`2[System.ValueTuple`2[uint,ushort],Microsoft.Cci.ITypeReference]):uint:this (FullOpts)
         -14 (-73.68% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.CodeGen.PrivateImplementationDetails+<>c:<Freeze>b__29_3(System.Collections.Generic.KeyValuePair`2[System.ValueTuple`2[uint,ushort],Microsoft.Cci.ITypeReference]):ushort:this (FullOpts)
         -36 (-66.67% of base) : System.Private.Xml.dasm - System.Xml.XmlTextReaderImpl:ParseText_NoValue(int,int):System.ValueTuple`4[int,int,int,bool] (FullOpts)
        -371 (-63.42% of base) : System.Private.CoreLib.dasm - System.TimeZoneInfo:FindSystemTimeZoneById(System.String):System.TimeZoneInfo (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.BranchOperation:MoveNext(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.BranchOperation:MoveNextReversed(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.CaughtExceptionOperation:MoveNext(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.CaughtExceptionOperation:MoveNextReversed(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.ConditionalAccessInstanceOperation:MoveNext(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.ConditionalAccessInstanceOperation:MoveNextReversed(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.DeclarationPatternOperation:MoveNext(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.DeclarationPatternOperation:MoveNextReversed(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)
         -30 (-58.82% of base) : Microsoft.CodeAnalysis.dasm - Microsoft.CodeAnalysis.Operations.DefaultCaseClauseOperation:MoveNext(int,int):System.ValueTuple`3[bool,int,int]:this (FullOpts)

793 total methods with Code Size differences (715 improved, 78 regressed), 408906 unchanged.

@jakobbotsch jakobbotsch changed the title Make padding in auto-layout types insignificant Unify JIT interface getTypeLayout between VM and crossgen2; refine what is considered "significant padding" Jun 30, 2023
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

3 participants