[AMDGPU] Fix v_swap_b32 formation on physical registers

[foad]

Summary

As explained in the comments, matchSwap matches:

mov t, x
mov x, y
// mov y, t

and turns it into:

mov t, x (t is potentially dead and move eliminated)
v_swap_b32 x, y

On physical registers we don't have full use-def chains so the check
for T being live-out was not working properly with subregs/superregs.

[arsenm]

We don't need to rely on kill flags (and in fact should not, since they are semi-deprecated). We should be using liveness scans starting from the bottom of the block which do not need kill flags

[foad]

In D101546#2726051, @arsenm wrote:

We don't need to rely on kill flags (and in fact should not, since they are semi-deprecated). We should be using liveness scans starting from the bottom of the block which do not need kill flags

I'm not relying on the kill flag being present for correctness, I'm just taking advantage of it if it does happen to be there (and assuming that it is correct if it is present). If kill flags are not present then I guess you'd need to run MachineDCE again afterwards to clean up.

llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp

576–577

Does this part make sense? I am really not too sure what kind of liveness information we can rely on here for virtual and/or physical registers.

[arsenm]

In D101546#2726096, @foad wrote:

In D101546#2726051, @arsenm wrote:

We don't need to rely on kill flags (and in fact should not, since they are semi-deprecated). We should be using liveness scans starting from the bottom of the block which do not need kill flags

I'm not relying on the kill flag being present for correctness, I'm just taking advantage of it if it does happen to be there (and assuming that it is correct if it is present). If kill flags are not present then I guess you'd need to run MachineDCE again afterwards to clean up.

I don't mean rely on for correctness, I mean use at all. Ideally we should be working to eliminate dependence on kill flags for optimizations.

[foad]

Don't use kill flags.

[foad]

We should be using liveness scans starting from the bottom of the block which do not need kill flags

To do this efficiently I guess the whole pass needs to be changed to work bottom-up in each basic block. I'd prefer to leave that for a separate patch.

[rampitec]

LGTM

[arsenm]

You can use the kill flags for now but it is something to migrate away from

llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp

for (unsigned I = 0; I < Size; ++I) {
         MIB->RemoveOperand(MIB->getNumExplicitOperands());
         MIB->copyImplicitOps(*MBB.getParent(), *MovX);
       }
     }
     MovX->eraseFromParent();
     dropInstructionKeepingImpDefs(*MovY, TII);
     MachineInstr *Next = &*std::next(MovT.getIterator());
 
-    if (MRI.use_nodbg_empty(T)) {
+    if (T.isVirtual() && MRI.use_nodbg_empty(T)) {
       dropInstructionKeepingImpDefs(MovT, TII);
     } else {
       Xop.setIsKill(false);
       for (int I = MovT.getNumImplicitOperands() - 1; I >= 0; --I ) {
         unsigned OpNo = MovT.getNumExplicitOperands() + I;
         const MachineOperand &Op = MovT.getOperand(OpNo);
         if (Op.isKill() && TRI.regsOverlap(X, Op.getReg()))
           MovT.RemoveOperand(OpNo);
       }

llvm/test/CodeGen/AMDGPU/v_swap_b32.mir

 # RUN: llc -march=amdgcn -mcpu=gfx900 -run-pass si-shrink-instructions -verify-machineinstrs %s -o - | FileCheck -check-prefix=GCN %s
 
 # GCN-LABEL: name: swap_phys_condensed
 # GCN: bb.0:
+# GCN-NEXT: $vgpr2 = V_MOV_B32_e32 $vgpr0, implicit $exec
 # GCN-NEXT: $vgpr0, $vgpr1 = V_SWAP_B32 $vgpr1, $vgpr0, implicit $exec
 # GCN-NEXT: S_SETPC_B64_return
 ---
 name:            swap_phys_condensed
 body:             |
   bb.0:
     liveins: $vgpr0, $vgpr1, $sgpr30_sgpr31
     $vgpr2 = V_MOV_B32_e32 killed $vgpr0, implicit $exec
     $vgpr0 = V_MOV_B32_e32 killed $vgpr1, implicit $exec
     $vgpr1 = V_MOV_B32_e32 killed $vgpr2, implicit $exec
     S_SETPC_B64_return $sgpr30_sgpr31, implicit $vgpr0, implicit $vgpr1
 ...
 
 # GCN-LABEL: name: swap_phys_sparse
 # GCN: bb.0:
+# GCN-NEXT: $vgpr2 = V_MOV_B32_e32 $vgpr0, implicit $exec
 # GCN-NEXT: $vgpr3 = V_MOV_B32_e32 killed $vgpr4, implicit $exec
 # GCN-NEXT: $vgpr0, $vgpr1 = V_SWAP_B32 $vgpr1, $vgpr0, implicit $exec
 # GCN-NEXT: $vgpr5 = V_MOV_B32_e32 killed $vgpr6, implicit $exec
 # GCN-NEXT: S_SETPC_B64_return
 ---
 name:            swap_phys_sparse
 body:             |
   bb.0:
body:             |
   bb.0:
     liveins: $vgpr0, $vgpr1, $sgpr30_sgpr31
     $vgpr2 = V_MOV_B32_e32 killed $vgpr0, implicit $exec
     $vgpr0 = V_MOV_B32_e32 killed $vgpr1, implicit $exec
     $vgpr1 = V_MOV_B32_e32 killed $vgpr2, implicit $exec
     S_SETPC_B64_return $sgpr30_sgpr31, implicit $vgpr0, implicit $vgpr2, implicit $vgpr1
 ...
 
-# FIXME: should not remove the def of $vgpr2 because $vgpr2_vgpr3 is live out
 # GCN-LABEL: name: swap_phys_liveout_superreg
 # GCN: bb.0:
+# GCN-NEXT: $vgpr2 = V_MOV_B32_e32 $vgpr0, implicit $exec
 # GCN-NEXT: $vgpr0, $vgpr1 = V_SWAP_B32 $vgpr1, $vgpr0, implicit $exec
 # GCN-NEXT: S_SETPC_B64_return
 ---
 name:            swap_phys_liveout_superreg
 body:             |
   bb.0:
     liveins: $vgpr0, $vgpr1, $sgpr30_sgpr31
     $vgpr2 = V_MOV_B32_e32 killed $vgpr0, implicit $exec
     $vgpr0 = V_MOV_B32_e32 killed $vgpr1, implicit $exec
     $vgpr1 = V_MOV_B32_e32 killed $vgpr2, implicit $exec
     S_SETPC_B64_return $sgpr30_sgpr31, implicit $vgpr0, implicit $vgpr2_vgpr3, implicit $vgpr1
 ...
 
 # GCN-LABEL: name: swap_phys_b64
 # GCN: bb.0:
+# GCN-NEXT: $vgpr4_vgpr5 = COPY $vgpr0_vgpr1
 # GCN-NEXT: $vgpr0, $vgpr2 = V_SWAP_B32 $vgpr2, $vgpr0, implicit $exec
 # GCN-NEXT: $vgpr1, $vgpr3 = V_SWAP_B32 $vgpr3, $vgpr1, implicit $exec
 ---
 name:            swap_phys_b64
 body:             |
   bb.0:
     $vgpr4_vgpr5 = COPY killed $vgpr0_vgpr1
     $vgpr0_vgpr1 = COPY killed $vgpr2_vgpr3
bb.0:
     %0 = IMPLICIT_DEF
     %1 = IMPLICIT_DEF
     %2 = COPY %0
     %0 = COPY %1, implicit $vgpr0
     %1 = COPY %2
 ...
 
 # GCN-LABEL: implicit_ops_mov_t_swap_b32
-# GCN:      $vgpr1 = IMPLICIT_DEF
+# GCN:      $vgpr3 = V_MOV_B32_e32 $vgpr0, implicit $exec, implicit $vgpr2, implicit killed $vgpr1_vgpr2, implicit-def $vgpr1
 # GCN-NEXT: $vgpr0, $vgpr1 = V_SWAP_B32 $vgpr1, $vgpr0, implicit $exec
 
 ---
 name:            implicit_ops_mov_t_swap_b32
 body:             |
   bb.0:
     liveins: $vgpr0, $vgpr1, $sgpr30_sgpr31
     $vgpr3 = V_MOV_B32_e32 killed $vgpr0, implicit $exec, implicit $vgpr2, implicit killed $vgpr1_vgpr2, implicit-def $vgpr1