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

Unpin locals #70264

Merged
merged 3 commits into from
Jun 11, 2022
Merged

Unpin locals #70264

merged 3 commits into from
Jun 11, 2022

Conversation

aromaa
Copy link
Contributor

@aromaa aromaa commented Jun 5, 2022

This unpins pinned locals when the pinned address is pointing to the stack. This does not cover Spans created with stackalloc's as tracking them requires more extensive state keeping.

Diffs

Example:

 ; 0 inlinees with PGO data; 1 single block inlinees; 0 inlinees without PGO data
 ; Final local variable assignments
 ;
-;  V00 loc0         [V00    ] (  2,  2   )    long  ->  [rsp+30H]   do-not-enreg[X] addr-exposed ld-addr-op
+;  V00 loc0         [V00    ] (  2,  2   )    long  ->  [rsp+20H]   do-not-enreg[X] addr-exposed ld-addr-op
 ;  V01 OutArgs      [V01    ] (  1,  1   )  lclBlk (32) [rsp+00H]   "OutgoingArgSpace"
-;  V02 tmp1         [V02    ] (  4,  4   )   byref  ->  [rsp+28H]   pinned "Inline stloc first use temp"
+;  V02 tmp1         [V02,T01] (  2,  2   )   byref  ->  rcx         "Inline stloc first use temp"
 ;* V03 tmp2         [V03    ] (  0,  0   )    long  ->  zero-ref    "Inline stloc first use temp"
 ;  V04 tmp3         [V04,T00] (  2,  4   )    long  ->  rcx         "Cast away GC"
 ;
-; Lcl frame size = 56
+; Lcl frame size = 40

 G_M59712_IG01:              ;; offset=0000H
-       4883EC38             sub      rsp, 56
+       4883EC28             sub      rsp, 40
                                                 ;; size=4 bbWeight=1    PerfScore 0.25
 G_M59712_IG02:              ;; offset=0004H
-       488D4C2430           lea      rcx, bword ptr [rsp+30H]
-       48894C2428           mov      bword ptr [rsp+28H], rcx
-       488B4C2428           mov      rcx, bword ptr [rsp+28H]
+       488D4C2420           lea      rcx, bword ptr [rsp+20H]
        48B8105C861DFF7F0000 mov      rax, 0x7FFF1D865C10
-                                                ;; size=25 bbWeight=1    PerfScore 2.75
-G_M59712_IG03:              ;; offset=001DH
+                                                ;; size=15 bbWeight=1    PerfScore 0.75
+G_M59712_IG03:              ;; offset=0013H
        FFD0                 call     rax ; Program:QueryPerformanceCounter(long):int
-       33C0                 xor      eax, eax
-       4889442428           mov      bword ptr [rsp+28H], rax
-       4889442428           mov      bword ptr [rsp+28H], rax
-       833DC6E8FE5F00       cmp      dword ptr [(reloc 0x7ffe32111cf8)], 0
+       833DDCE8016000       cmp      dword ptr [(reloc 0x7ffe32111cf8)], 0
        750A                 jne      SHORT G_M59712_IG06
-                                                ;; size=23 bbWeight=1    PerfScore 9.25
-G_M59712_IG04:              ;; offset=0034H
-       488B442430           mov      rax, qword ptr [rsp+30H]
+                                                ;; size=11 bbWeight=1    PerfScore 7.00
+G_M59712_IG04:              ;; offset=001EH
+       488B442420           mov      rax, qword ptr [rsp+20H]
                                                 ;; size=5 bbWeight=1    PerfScore 1.00
-G_M59712_IG05:              ;; offset=0039H
-       4883C438             add      rsp, 56
+G_M59712_IG05:              ;; offset=0023H
+       4883C428             add      rsp, 40
        C3                   ret
                                                 ;; size=5 bbWeight=1    PerfScore 1.25
-G_M59712_IG06:              ;; offset=003EH
-       E88D77715E           call     CORINFO_HELP_POLL_GC
+G_M59712_IG06:              ;; offset=0028H
+       E8A377745E           call     CORINFO_HELP_POLL_GC
        EBEF                 jmp      SHORT G_M59712_IG04
                                                 ;; size=7 bbWeight=0    PerfScore 0.00

-; Total bytes of code 69, prolog size 4, PerfScore 21.40, instruction count 16, allocated bytes for code 69 (MethodHash=6ac616bf) for method Program:JitDisam():long
-; ============================================================
-
+; Total bytes of code 47, prolog size 4, PerfScore 14.95, instruction count 11, allocated bytes for code 47 (MethodHash=6ac616bf) for method Program:JitDisam():long

Contributes #40553

@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 5, 2022
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 5, 2022
@ghost
Copy link

ghost commented Jun 5, 2022

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

Issue Details

This unpins pinned locals when the pinned address is pointing to the stack. This does not cover Spans created with stackalloc's as tracking them requires more extensive state keeping.

Example:

 ; 0 inlinees with PGO data; 1 single block inlinees; 0 inlinees without PGO data
 ; Final local variable assignments
 ;
-;  V00 loc0         [V00    ] (  2,  2   )    long  ->  [rsp+30H]   do-not-enreg[X] addr-exposed ld-addr-op
+;  V00 loc0         [V00    ] (  2,  2   )    long  ->  [rsp+20H]   do-not-enreg[X] addr-exposed ld-addr-op
 ;  V01 OutArgs      [V01    ] (  1,  1   )  lclBlk (32) [rsp+00H]   "OutgoingArgSpace"
-;  V02 tmp1         [V02    ] (  4,  4   )   byref  ->  [rsp+28H]   pinned "Inline stloc first use temp"
+;  V02 tmp1         [V02,T01] (  2,  2   )   byref  ->  rcx         "Inline stloc first use temp"
 ;* V03 tmp2         [V03    ] (  0,  0   )    long  ->  zero-ref    "Inline stloc first use temp"
 ;  V04 tmp3         [V04,T00] (  2,  4   )    long  ->  rcx         "Cast away GC"
 ;
-; Lcl frame size = 56
+; Lcl frame size = 40

 G_M59712_IG01:              ;; offset=0000H
-       4883EC38             sub      rsp, 56
+       4883EC28             sub      rsp, 40
                                                 ;; size=4 bbWeight=1    PerfScore 0.25
 G_M59712_IG02:              ;; offset=0004H
-       488D4C2430           lea      rcx, bword ptr [rsp+30H]
-       48894C2428           mov      bword ptr [rsp+28H], rcx
-       488B4C2428           mov      rcx, bword ptr [rsp+28H]
+       488D4C2420           lea      rcx, bword ptr [rsp+20H]
        48B8105C861DFF7F0000 mov      rax, 0x7FFF1D865C10
-                                                ;; size=25 bbWeight=1    PerfScore 2.75
-G_M59712_IG03:              ;; offset=001DH
+                                                ;; size=15 bbWeight=1    PerfScore 0.75
+G_M59712_IG03:              ;; offset=0013H
        FFD0                 call     rax ; Program:QueryPerformanceCounter(long):int
-       33C0                 xor      eax, eax
-       4889442428           mov      bword ptr [rsp+28H], rax
-       4889442428           mov      bword ptr [rsp+28H], rax
-       833DC6E8FE5F00       cmp      dword ptr [(reloc 0x7ffe32111cf8)], 0
+       833DDCE8016000       cmp      dword ptr [(reloc 0x7ffe32111cf8)], 0
        750A                 jne      SHORT G_M59712_IG06
-                                                ;; size=23 bbWeight=1    PerfScore 9.25
-G_M59712_IG04:              ;; offset=0034H
-       488B442430           mov      rax, qword ptr [rsp+30H]
+                                                ;; size=11 bbWeight=1    PerfScore 7.00
+G_M59712_IG04:              ;; offset=001EH
+       488B442420           mov      rax, qword ptr [rsp+20H]
                                                 ;; size=5 bbWeight=1    PerfScore 1.00
-G_M59712_IG05:              ;; offset=0039H
-       4883C438             add      rsp, 56
+G_M59712_IG05:              ;; offset=0023H
+       4883C428             add      rsp, 40
        C3                   ret
                                                 ;; size=5 bbWeight=1    PerfScore 1.25
-G_M59712_IG06:              ;; offset=003EH
-       E88D77715E           call     CORINFO_HELP_POLL_GC
+G_M59712_IG06:              ;; offset=0028H
+       E8A377745E           call     CORINFO_HELP_POLL_GC
        EBEF                 jmp      SHORT G_M59712_IG04
                                                 ;; size=7 bbWeight=0    PerfScore 0.00

-; Total bytes of code 69, prolog size 4, PerfScore 21.40, instruction count 16, allocated bytes for code 69 (MethodHash=6ac616bf) for method Program:JitDisam():long
-; ============================================================
-
+; Total bytes of code 47, prolog size 4, PerfScore 14.95, instruction count 11, allocated bytes for code 47 (MethodHash=6ac616bf) for method Program:JitDisam():long

Contributes #40553

Author: aromaa
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

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.

Looks good, and diffs look good too.

I've suggested a few minor changes.

Also not sure how I feel about IsNotGcDef ... let me think about it a bit more.

src/coreclr/jit/lclvars.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/compiler.h Outdated Show resolved Hide resolved
@JulieLeeMSFT JulieLeeMSFT added this to the 7.0.0 milestone Jun 10, 2022
@AndyAyersMS
Copy link
Member

Rerunning SPMI as we likely got caught in the middle of a jit GUID change.

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.

Thanks.

Still trying to get a clean test run. Running into CI issues, nothing specific with your changes.

@AndyAyersMS
Copy link
Member

Going to bounce this as SPMI seems confused.

@AndyAyersMS AndyAyersMS reopened this Jun 10, 2022
@AndyAyersMS
Copy link
Member

Installer build & test failure AppHost.Bundle.Tests_net7.0_x64.html seems unique.
Native AOT failure on arm64 seems less so (though no open issue for it).

Am going to see if these repro.

@AndyAyersMS
Copy link
Member

Native AOT timed out (#70549)

Running assembly:System.Runtime.Tests, Version=7.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
[SKIP] System.ComponentModel.Tests.DefaultValueAttributeTests.Ctor_TypeDescriptorNotFound_ExceptionFallback
[SKIP] System.Tests.ArraySegment_Tests_string.IGenericSharedAPI_SerializeDeserialize
[SKIP] System.Text.Tests.EncodingTests.GetEncoding_FromProvider_ByCodePage_WithDisallowedEncoding_Throws
[SKIP] System.Text.Tests.EncodingTests.GetEncoding_FromProvider_ByEncodingName_WithDisallowedEncoding_Throws
[SKIP] System.Text.Tests.EncodingTests.GetEncodings_FromProvider_DoesNotContainDisallowedEncodings
[SKIP] System.Text.Tests.EncodingTests.RegisterProvider_EncodingsAreUsable

...
[EXECUTION TIMED OUT]
Exit Code:-3Executor timed out after 2700 seconds and was killed
['System.Runtime.Tests' END OF WORK ITEM LOG: Command timed out, and was killed]

@AndyAyersMS AndyAyersMS merged commit b1c2275 into dotnet:main Jun 11, 2022
@aromaa aromaa deleted the opts/unpin-locals branch June 11, 2022 16:25
jkotas added a commit to jkotas/runtime that referenced this pull request Jun 12, 2022
jkotas added a commit that referenced this pull request Jun 13, 2022
aromaa added a commit to aromaa/runtime that referenced this pull request Jun 13, 2022
@aromaa aromaa mentioned this pull request Jun 13, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 11, 2022
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 community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants