Skip to content

Conversation

@jakobbotsch
Copy link
Member

Allow last-use copy omission to kick in when the argument is a comma (typically created by physical promotion).

Also handle this pattern in arg evaluation: avoid creating a new local for the COMMA(..., LCL_ADDR) created for this pattern. As part of this rename the "needs temp" terminology to "needs early evaluation" since these cases do not actually need or create a temp. There were other cases already where we did not create a temp (specifically handling of GT_MKREFANY), so the terminology was already a bit misleading.

Fix #87471

Allow last-use copy omission to kick in when the argument is a comma
(typically created by physical promotion).

Also handle this pattern in arg evaluation: avoid creating a new local
for the COMMA(..., LCL_ADDR) created for this pattern. As part of this
rename the "needs temp" terminology to "needs early evaluation" since
these cases do not actually need or create a temp. There were other
cases already where we did not create a temp (specifically handling of
GT_MKREFANY), so the terminology was already a bit misleading.

Fix dotnet#87471
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 14, 2023
@ghost ghost assigned jakobbotsch Jun 14, 2023
@ghost
Copy link

ghost commented Jun 14, 2023

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

Issue Details

Allow last-use copy omission to kick in when the argument is a comma (typically created by physical promotion).

Also handle this pattern in arg evaluation: avoid creating a new local for the COMMA(..., LCL_ADDR) created for this pattern. As part of this rename the "needs temp" terminology to "needs early evaluation" since these cases do not actually need or create a temp. There were other cases already where we did not create a temp (specifically handling of GT_MKREFANY), so the terminology was already a bit misleading.

Fix #87471

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

jakobbotsch commented Jun 14, 2023

Example codegen diff:

 G_M55686_IG10:
-       mov      dword ptr [rsp+94H], r12d
-       vmovdqu  ymm0, ymmword ptr [rsp+88H]
-       vmovdqu  ymmword ptr [rsp+48H], ymm0
-       mov      dword ptr [rsp+74H], edx
-       vmovdqu  ymm0, ymmword ptr [rsp+68H]
-       vmovdqu  ymmword ptr [rsp+28H], ymm0
+       mov      dword ptr [rsp+54H], r12d
+       mov      dword ptr [rsp+34H], edx
        mov      rcx, gword ptr [rbx+08H]
        lea      rdx, [rsp+48H]
        lea      r8, [rsp+28H]
        call     [rbx+18H]System.Comparison`1[System.Collections.BigStruct]:Invoke(System.Collections.BigStruct,System.Collections.BigStruct):int:this

jitdump before:

fgMorphTree BB13, STMT00013 (before)
               [000129] DACXG------                           STORE_LCL_VAR int    V06 tmp1         
               [000037] -ACXG------                         └──▌  CALL      int    System.Comparison`1[System.Collections.BigStruct]:Invoke(System.Collections.BigStruct,System.Collections.BigStruct):int:this
               [000006] ----------- this                       ├──▌  LCL_VAR   ref    V01 arg1          (last use)
               [000207] -A--------- arg1                       ├──▌  COMMA     struct
               [000206] UA---------                              ├──▌  STORE_LCL_FLD int    V07 tmp2         [+12]
               [000205] -----------                                └──▌  LCL_VAR   int    V17 tmp12        
               [000107] -----------                              └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V07 tmp2          (last use)
               [000210] -A--------- arg2                       └──▌  COMMA     struct
               [000209] UA---------                               ├──▌  STORE_LCL_FLD int    V08 tmp3         [+12]
               [000208] -----------                                 └──▌  LCL_VAR   int    V18 tmp13        
               [000109] -----------                               └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V08 tmp3          (last use)
Initializing arg info for 37.CALL:
Args for call [000037] CALL after AddFinalArgsAndDetermineABIInfo:
CallArg[[000006].LCL_VAR ref (By value), 1 reg: rcx, byteAlignment=8, wellKnown[ThisPointer]]
CallArg[[000207].COMMA struct (By ref), 1 reg: rdx, byteAlignment=8, isStruct]
CallArg[[000210].COMMA struct (By ref), 1 reg: r8, byteAlignment=8, isStruct]

Morphing args for 37.CALL:
making an outgoing copy for struct arg

lvaGrabTemp returning 23 (V23 tmp18) called for by-value struct argument.
MorphCopyBlock:
PrepareDst for [000226] have found a local var V23.
GenTreeNode creates assertion:
               [000226] DA---------                           STORE_LCL_VAR struct<System.Collections.BigStruct, 32> V23 tmp18        
In BB13 New Local Copy     Assertion: V23 == V07, index = #01
block assignment to morph:
               [000226] DA---------                           STORE_LCL_VAR struct<System.Collections.BigStruct, 32> V23 tmp18        
               [000107] -----+-----                         └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V07 tmp2          (last use)
 with no promoted structs this requires a CopyBlock.
MorphCopyBlock (after):
               [000207] -A---+-----                           COMMA     void  
               [000206] UA---+-----                         ├──▌  STORE_LCL_FLD int    V07 tmp2         [+12]
               [000205] -----+-----                           └──▌  LCL_VAR   int    V17 tmp12        
               [000226] DA---+-----                         └──▌  STORE_LCL_VAR struct<System.Collections.BigStruct, 32> V23 tmp18        
               [000107] -----+-----                            └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V07 tmp2          (last use)
making an outgoing copy for struct arg

lvaGrabTemp returning 24 (V24 tmp19) called for by-value struct argument.
MorphCopyBlock:
PrepareDst for [000227] have found a local var V24.
GenTreeNode creates assertion:
               [000227] DA---------                           STORE_LCL_VAR struct<System.Collections.BigStruct, 32> V24 tmp19        
In BB13 New Local Copy     Assertion: V24 == V08, index = #02
block assignment to morph:
               [000227] DA---------                           STORE_LCL_VAR struct<System.Collections.BigStruct, 32> V24 tmp19        
               [000109] -----+-----                         └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V08 tmp3          (last use)
 with no promoted structs this requires a CopyBlock.
MorphCopyBlock (after):
               [000210] -A---+-----                           COMMA     void  
               [000209] UA---+-----                         ├──▌  STORE_LCL_FLD int    V08 tmp3         [+12]
               [000208] -----+-----                           └──▌  LCL_VAR   int    V18 tmp13        
               [000227] DA---+-----                         └──▌  STORE_LCL_VAR struct<System.Collections.BigStruct, 32> V24 tmp19        
               [000109] -----+-----                            └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V08 tmp3          (last use)

Sorting the arguments:
Argument with 'side effect'...
               [000006] -----+-----                           LCL_VAR   ref    V01 arg1          (last use)

lvaGrabTemp returning 25 (V25 tmp20) called for argument with side effect.

  Evaluate to a temp:
               [000228] DA---------                           STORE_LCL_VAR ref    V25 tmp20        
               [000006] -----+-----                         └──▌  LCL_VAR   ref    V01 arg1          (last use)

Local V23 should not be enregistered because: it is address exposed

Local V24 should not be enregistered because: it is address exposed

Register placement order:    rcx rdx r8 
Args for [000037].CALL after fgMorphArgs:
CallArg[[000229].LCL_VAR ref (By value), 1 reg: rcx, byteAlignment=8, isLate, tmpNum=V25, isTmp, processed, wellKnown[ThisPointer]]
CallArg[[000230].LCL_ADDR struct (By ref), 1 reg: rdx, byteAlignment=8, isLate, tmpNum=V23, isTmp, processed, isStruct]
CallArg[[000231].LCL_ADDR struct (By ref), 1 reg: r8, byteAlignment=8, isLate, tmpNum=V24, isTmp, processed, isStruct]
OutgoingArgsStackSize is 32


fgMorphTree BB13, STMT00013 (after)
               [000129] DACXG+-----                           STORE_LCL_VAR int    V06 tmp1         
               [000037] -ACXG+-----                         └──▌  CALL      int    System.Comparison`1[System.Collections.BigStruct]:Invoke(System.Collections.BigStruct,System.Collections.BigStruct):int:this
               [000228] DA--------- this setup                 ├──▌  STORE_LCL_VAR ref    V25 tmp20        
               [000006] -----+-----                              └──▌  LCL_VAR   ref    V01 arg1          (last use)
               [000207] -A---+----- arg1 setup                 ├──▌  COMMA     void  
               [000206] UA---+-----                              ├──▌  STORE_LCL_FLD int    V07 tmp2         [+12]
               [000205] -----+-----                                └──▌  LCL_VAR   int    V17 tmp12        
               [000226] DA---+-----                              └──▌  STORE_LCL_VAR struct<System.Collections.BigStruct, 32>(AX) V23 tmp18        
               [000107] -----+-----                                 └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V07 tmp2          (last use)
               [000210] -A---+----- arg2 setup                 ├──▌  COMMA     void  
               [000209] UA---+-----                              ├──▌  STORE_LCL_FLD int    V08 tmp3         [+12]
               [000208] -----+-----                                └──▌  LCL_VAR   int    V18 tmp13        
               [000227] DA---+-----                              └──▌  STORE_LCL_VAR struct<System.Collections.BigStruct, 32>(AX) V24 tmp19        
               [000109] -----+-----                                 └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V08 tmp3          (last use)
               [000229] ----------- this in rcx                ├──▌  LCL_VAR   ref    V25 tmp20        
               [000230] ----------- arg1 in rdx                ├──▌  LCL_ADDR  long   V23 tmp18        [+0]
               [000231] ----------- arg2 in r8                 └──▌  LCL_ADDR  long   V24 tmp19        [+0]

After:

fgMorphTree BB13, STMT00013 (before)
               [000129] DACXG------                           STORE_LCL_VAR int    V06 tmp1         
               [000037] -ACXG------                         └──▌  CALL      int    System.Comparison`1[System.Collections.BigStruct]:Invoke(System.Collections.BigStruct,System.Collections.BigStruct):int:this
               [000006] ----------- this                       ├──▌  LCL_VAR   ref    V01 arg1          (last use)
               [000207] -A--------- arg1                       ├──▌  COMMA     struct
               [000206] UA---------                              ├──▌  STORE_LCL_FLD int    V07 tmp2         [+12]
               [000205] -----------                                └──▌  LCL_VAR   int    V17 tmp12        
               [000107] -----------                              └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V07 tmp2          (last use)
               [000210] -A--------- arg2                       └──▌  COMMA     struct
               [000209] UA---------                               ├──▌  STORE_LCL_FLD int    V08 tmp3         [+12]
               [000208] -----------                                 └──▌  LCL_VAR   int    V18 tmp13        
               [000109] -----------                               └──▌  LCL_VAR   struct<System.Collections.BigStruct, 32> V08 tmp3          (last use)
Initializing arg info for 37.CALL:
Args for call [000037] CALL after AddFinalArgsAndDetermineABIInfo:
CallArg[[000006].LCL_VAR ref (By value), 1 reg: rcx, byteAlignment=8, wellKnown[ThisPointer]]
CallArg[[000207].COMMA struct (By ref), 1 reg: rdx, byteAlignment=8, isStruct]
CallArg[[000210].COMMA struct (By ref), 1 reg: r8, byteAlignment=8, isStruct]

Morphing args for 37.CALL:

Local V07 should not be enregistered because: it is address exposed
did not need to make outgoing copy for last use of V07

Local V08 should not be enregistered because: it is address exposed
did not need to make outgoing copy for last use of V08

Sorting the arguments:
Argument with 'side effect'...
               [000006] -----+-----                           LCL_VAR   ref    V01 arg1          (last use)

lvaGrabTemp returning 23 (V23 tmp18) called for argument with side effect.

  Evaluate to a temp:
               [000226] DA---------                           STORE_LCL_VAR ref    V23 tmp18        
               [000006] -----+-----                         └──▌  LCL_VAR   ref    V01 arg1          (last use)

Register placement order:    rcx rdx r8 
Args for [000037].CALL after fgMorphArgs:
CallArg[[000227].LCL_VAR ref (By value), 1 reg: rcx, byteAlignment=8, isLate, evaluateEarly, isTmp, tmpNum=V23, processed, wellKnown[ThisPointer]]
CallArg[[000107].LCL_ADDR struct (By ref), 1 reg: rdx, byteAlignment=8, isLate, evaluateEarly, processed, isStruct]
CallArg[[000109].LCL_ADDR struct (By ref), 1 reg: r8, byteAlignment=8, isLate, evaluateEarly, processed, isStruct]
OutgoingArgsStackSize is 32


fgMorphTree BB13, STMT00013 (after)
               [000129] DACXG+-----                           STORE_LCL_VAR int    V06 tmp1         
               [000037] -ACXG+-----                         └──▌  CALL      int    System.Comparison`1[System.Collections.BigStruct]:Invoke(System.Collections.BigStruct,System.Collections.BigStruct):int:this
               [000226] DA--------- this setup                 ├──▌  STORE_LCL_VAR ref    V23 tmp18        
               [000006] -----+-----                              └──▌  LCL_VAR   ref    V01 arg1          (last use)
               [000206] UA---+----- arg1 setup                 ├──▌  STORE_LCL_FLD int   (AX) V07 tmp2         [+12]
               [000205] -----+-----                              └──▌  LCL_VAR   int    V17 tmp12        
               [000209] UA---+----- arg2 setup                 ├──▌  STORE_LCL_FLD int   (AX) V08 tmp3         [+12]
               [000208] -----+-----                              └──▌  LCL_VAR   int    V18 tmp13        
               [000227] ----------- this in rcx                ├──▌  LCL_VAR   ref    V23 tmp18        
               [000107] -----+----- arg1 in rdx                ├──▌  LCL_ADDR  long   V07 tmp2         [+0]
               [000109] -----+----- arg2 in r8                 └──▌  LCL_ADDR  long   V08 tmp3         [+0]

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch
Copy link
Member Author

Subsumed by #87869

@EgorBo EgorBo mentioned this pull request Jul 5, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 2023
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.

JIT: Last-use copy elision does not handle IR created by physical promotion

1 participant