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: Allow more permissive forward subbing into call arguments #71161

Merged

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jun 22, 2022

After #70893 and #70931 we should be able to allow this.

Diffs

@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 22, 2022
@ghost ghost assigned jakobbotsch Jun 22, 2022
@ghost
Copy link

ghost commented Jun 22, 2022

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

Issue Details

After #70893 and #70931 we should be able to allow this.

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

/azp run Fuzzlyn

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch jakobbotsch marked this pull request as ready for review June 23, 2022 08:53
@jakobbotsch
Copy link
Member Author

The failures are #71193 and #71200.

cc @dotnet/jit-contrib PTAL @AndyAyersMS

@AndyAyersMS
Copy link
Member

Have you looked into some of these large percentage regressions (x64 win)?

Top method regressions (percentages):
           8 (61.54 % of base) : 120885.dasm - System.Drawing.CharacterRange:GetHashCode():int:this
           8 (61.54 % of base) : 198086.dasm - System.Drawing.Point:GetHashCode():int:this
           8 (61.54 % of base) : 198218.dasm - System.Drawing.Size:GetHashCode():int:this

@jakobbotsch
Copy link
Member Author

Have you looked into some of these large percentage regressions (x64 win)?

Spot checking the regressions they all look like the following:

@@ -7,9 +7,9 @@
 ; 0 inlinees with PGO data; 2 single block inlinees; 0 inlinees without PGO data
 ; Final local variable assignments
 ;
-;  V00 this         [V00,T00] (  4,  4   )   byref  ->  rcx         this single-def
+;  V00 this         [V00,T00] (  4,  4   )   byref  ->  [rsp+08H]   this single-def
 ;# V01 OutArgs      [V01    ] (  1,  1   )  lclBlk ( 0) [rsp+00H]   "OutgoingArgSpace"
-;  V02 tmp1         [V02,T01] (  2,  4   )     int  ->  rax         "impAppendStmt"
+;* V02 tmp1         [V02    ] (  0,  0   )     int  ->  zero-ref    "impAppendStmt"
 ;
 ; Lcl frame size = 0
 
@@ -17,17 +17,21 @@ G_M4318_IG01:        ; gcrefRegs=00000000 {}, byrefRegs=00000000 {}, byref, nogc
 						;; size=0 bbWeight=1    PerfScore 0.00
 G_M4318_IG02:        ; gcrefRegs=00000000 {}, byrefRegs=00000002 {rcx}, byref
        ; byrRegs +[rcx]
-       mov      eax, dword ptr [rcx]
-       mov      edx, dword ptr [rcx+4]
-       mov      ecx, eax
+       mov      bword ptr [rsp+08H], rcx
+       ; GC ptr vars +{V00}
+       mov      ecx, dword ptr [rcx]
        ; byrRegs -[rcx]
-						;; size=7 bbWeight=1    PerfScore 4.25
+       mov      rdx, bword ptr [rsp+08H]
+       ; byrRegs +[rdx]
+       mov      edx, dword ptr [rdx+4]
+       ; byrRegs -[rdx]
+						;; size=15 bbWeight=1    PerfScore 6.00
 G_M4318_IG03:        ; , epilog, nogc, extend
        tail.jmp [System.HashCode:Combine(int,int):int]
        ; gcr arg pop 0
 						;; size=6 bbWeight=1    PerfScore 2.00
 
-; Total bytes of code 13, prolog size 0, PerfScore 7.55, instruction count 4, allocated bytes for code 13 (MethodHash=68b0ef21) for method System.Drawing.CharacterRange:GetHashCode():int:this
+; Total bytes of code 21, prolog size 0, PerfScore 10.10, instruction count 5, allocated bytes for code 21 (MethodHash=68b0ef21) for method System.Drawing.CharacterRange:GetHashCode():int:this
 ; ============================================================
 
 Unwind Info:

This is one of the cases I think we have talked about before where we could make the sort better by taking into account that placing an argument may clobber a register used by a future argument. I hope to look into that in the future.

@jakobbotsch
Copy link
Member Author

Is there anything else you think I should do here @AndyAyersMS?

@AndyAyersMS
Copy link
Member

Is there anything else you think I should do here @AndyAyersMS?

No, this looks good.

@jakobbotsch jakobbotsch merged commit 8fe39db into dotnet:main Jun 27, 2022
@jakobbotsch jakobbotsch deleted the loosen-forward-sub-callargs-restrictions branch June 27, 2022 17:19
@ghost ghost locked as resolved and limited conversation to collaborators Jul 27, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants