PostRA Machine Sink should take care of COPY defining register that is a sub-register by another COPY source operand

[alex-t]

Summary

This pass was created with the assumption that all the copy sequences like

x = COPY y
z = COPY x

were already handled by the coalescer and transformed to

z = COPY y

This was true until the sub-registers appeared.

Considering the following pattern:

x = COPY y
z = COPY s
w = COPY x_z

where x_z is a super-reg consisting of x and y

Current implementation does not take into account registers defined by another copies assuming that one COPY cannot define register that is used by another one. As a result in sinks the COPY defining sub-register to one successor but COPY that reads super-register to another providing this later with uninitialized register.
Proposed fix updates lane mask after sinking the COPY rather then merely add whole COPY source to live ins.

[lebedev.ri]

Is there a test missing?

[nhaehnle]

With this change, it looks as though accumulateUsedDefed is called multiple times on some paths. That seems wrong. It does seem plausible that we'd have to keep accumulating for all code paths.

Also, this needs a test.

[alex-t]

In D71132#1774315, @nhaehnle wrote:

With this change, it looks as though accumulateUsedDefed is called multiple times on some paths. That seems wrong.

Yes. In fact this fix is wrong. Will be looking for another way to handle this.

[alex-t]

Just correct solution + tests

[vpykhtin]

Almost LGTM. Do you need those liveins reorderings?

llvm/test/CodeGen/AArch64/bisect-post-ra-machine-sink.mir

12

is this really needed?

[alex-t]

llvm/test/CodeGen/AArch64/bisect-post-ra-machine-sink.mir

12

sortUniqueLiveIns does sort. Hence we're going to have w0 before w1

[vpykhtin]

LGTM.

[arsenm]

llvm/lib/CodeGen/MachineSink.cpp

1220

Should use MCRegUnitMaskIterator instead of getting the lane masks for each subreg

[alex-t]

Updated according to reviewer request

llvm/lib/CodeGen/MachineSink.cpp

static void updateLiveIn(MachineInstr *MI, MachineBasicBlock *SuccBB,
                          SmallVectorImpl<unsigned> &UsedOpsInCopy,
                          SmallVectorImpl<unsigned> &DefedRegsInCopy) {
   MachineFunction &MF = *SuccBB->getParent();
   const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
   for (unsigned DefReg : DefedRegsInCopy)
     for (MCSubRegIterator S(DefReg, TRI, true); S.isValid(); ++S)
       SuccBB->removeLiveIn(*S);
   for (auto U : UsedOpsInCopy) {
-    Register Reg = MI->getOperand(U).getReg();
-    if (!SuccBB->isLiveIn(Reg))
-      SuccBB->addLiveIn(Reg);
+    Register SrcReg = MI->getOperand(U).getReg();
+    LaneBitmask Mask;
+    for (MCRegUnitMaskIterator S(SrcReg, TRI); S.isValid(); ++S) {
+      Mask |= (*S).second;
     }
+    SuccBB->addLiveIn(SrcReg, Mask.any() ? Mask : LaneBitmask::getAll());
+  }
+  SuccBB->sortUniqueLiveIns();
 }
 
 static bool hasRegisterDependency(MachineInstr *MI,
                                   SmallVectorImpl<unsigned> &UsedOpsInCopy,
                                   SmallVectorImpl<unsigned> &DefedRegsInCopy,
                                   LiveRegUnits &ModifiedRegUnits,
                                   LiveRegUnits &UsedRegUnits) {
   bool HasRegDependency = false;

llvm/test/CodeGen/AArch64/bisect-post-ra-machine-sink.mir

 # RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass=postra-machine-sink -verify-machineinstrs  -o - %s | FileCheck -check-prefix=RUN-POSTRA %s
 # RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass=postra-machine-sink -opt-bisect-limit=0 -verify-machineinstrs  -o - %s | FileCheck -check-prefix=BISECT-NO-RUN-POSTRA %s
 
 ---
 
 # Make sure the pass is run.
 # Sink w19 to %bb.1.
 # RUN-POSTRA-LABEL: name: sinkcopy1
 # RUN-POSTRA: bb.0:
 # RUN-POSTRA-NOT: $w19 = COPY killed $w0
 # RUN-POSTRA: bb.1:
-# RUN-POSTRA: liveins: $w1, $w0
+# RUN-POSTRA: liveins: $w0, $w1
 # RUN-POSTRA: renamable $w19 = COPY killed $w0
 
 # Make sure the pass it not run.
 # BISECT-NO-RUN-POSTRA-LABEL: name: sinkcopy1
 # BISECT-NO-RUN-POSTRA: bb.0:
 # BISECT-NO-RUN-POSTRA: $w19 = COPY killed $w0
 # BISECT-NO-RUN-POSTRA: bb.1:
 # BISECT-NO-RUN-POSTRA: liveins: $w1, $w19

llvm/test/CodeGen/AArch64/post-ra-machine-sink.mir

 # RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass=postra-machine-sink -verify-machineinstrs  -o - %s | FileCheck %s
 
 ---
 # Sink w19 to %bb.1.
 # CHECK-LABEL: name: sinkcopy1
 # CHECK-LABEL: bb.0:
 # CHECK-NOT: $w19 = COPY killed $w0
 # CHECK-LABEL: bb.1:
-# CHECK: liveins: $w1, $w0
+# CHECK: liveins: $w0, $w1
 # CHECK: renamable $w19 = COPY killed $w0
 
 name: sinkcopy1
 tracksRegLiveness: true
 body: |
   bb.0:
     liveins: $w0, $w1
     $w1 = SUBSWri $w1, 1, 0, implicit-def $nzcv

 ...
 
 ---
 # Sink w19 to %bb.2.
 # CHECK-LABEL: name: sinkcopy2
 # CHECK-LABEL: bb.0:
 # CHECK-NOT: renamable $w19 = COPY killed $w0
 # CHECK-LABEL: bb.2:
-# CHECK: liveins: $w1, $w0
+# CHECK: liveins: $w0, $w1
 # CHECK: renamable $w19 = COPY killed $w0
 name: sinkcopy2
 tracksRegLiveness: true
 body: |
   bb.0:
     liveins: $w0, $w1
     $w1 = SUBSWri $w1, 1, 0, implicit-def $nzcv
     renamable $w19 = COPY killed $w0

 ...
 
 ---
 # Sink w19 and w20 to %bb.1.
 # CHECK-LABEL: name: sinkcopy3
 # CHECK-LABEL: bb.0:
 # CHECK-NOT: renamable $w19 = COPY killed $w0
 # CHECK-LABEL: bb.1:
-# CHECK: liveins: $w1, $w0
+# CHECK: liveins: $w0, $w1
 # CHECK: renamable $w19 = COPY killed $w0
 # CHECK: renamable $w20 = COPY killed $w1
 name: sinkcopy3
 tracksRegLiveness: true
 body: |
   bb.0:
     liveins: $w0, $w1
     $w1 = SUBSWri $w1, 1, 0, implicit-def $nzcv

 
 
 # Sink w19 to %bb.1 and w20 to %bb.2.
 # CHECK-LABEL: name: sinkcopy4
 # CHECK-LABEL: bb.0:
 # CHECK-NOT: renamable $w19 = COPY killed $w0
 # CHECK-NOT: renamable $w20 = COPY killed $w1
 # CHECK-LABEL: bb.1:
-# CHECK: liveins: $w1, $w0
+# CHECK: liveins: $w0, $w1
 # CHECK: renamable $w19 = COPY killed $w0
 # CHECK-LABEL: bb.2:
 # CHECK: liveins: $w0, $w1
 # CHECK: renamable $w20 = COPY killed $w1
 name: sinkcopy4
 tracksRegLiveness: true
 body: |
   bb.0:

 
 # Sink w19 to %bb.3 through %bb.2.
 # CHECK-LABEL: name: sinkcopy5
 # CHECK-LABEL: bb.0:
 # CHECK-NOT: renamable $w19 = COPY $w0
 # CHECK-LABEL: bb.2:
 # CHECK: $w1 = ADDWrr $w1, $w0
 # CHECK-LABEL: bb.3:
-# CHECK: liveins: $w1, $w0
+# CHECK: liveins: $w0, $w1
 # CHECK: renamable $w19 = COPY killed $w0
 name: sinkcopy5
 tracksRegLiveness: true
 body: |
   bb.0:
     liveins: $w0, $w1
     $w1 = SUBSWri $w1, 1, 0, implicit-def $nzcv
     renamable $w19 = COPY $w0

 ...
 
 # Sink w19 to %bb.3, but  through %bb.2.
 # CHECK-LABEL: name: sinkcopy6
 # CHECK-LABEL: bb.0:
 # CHECK-NOT: renamable $w19 = COPY $w0
 # CHECK-NOT: renamable $w20 = COPY $w0
 # CHECK-LABEL: bb.2:
-# CHECK: liveins: $w1, $w0
+# CHECK: liveins: $w0, $w1
 # CHECK: renamable $w19 = COPY $w0
 # CHECK: renamable $w20 = COPY $w19
 name: sinkcopy6
 tracksRegLiveness: true
 body: |
   bb.0:
     liveins: $w0, $w1
     $w1 = SUBSWri $w1, 1, 0, implicit-def $nzcv

 ---
 # Sink w19 to %bb.3 through %bb.2.
 # CHECK-LABEL: name: sinkcopy8
 # CHECK-LABEL: bb.0:
 # CHECK-NOT: renamable $w19 = COPY $w0, implicit-def $x19
 # CHECK-LABEL: bb.2:
 # CHECK: $w1 = ADDWrr $w1, $w0, implicit $x0
 # CHECK-LABEL: bb.3:
-# CHECK: liveins: $x1, $w0
+# CHECK: liveins: $w0, $x1
 # CHECK: renamable $w19 = COPY killed $w0, implicit-def $x19
 name: sinkcopy8
 tracksRegLiveness: true
 body: |
   bb.0:
     liveins: $w0, $x1
     $w1 = SUBSWri $w1, 1, 0, implicit-def $nzcv
     renamable $w19 = COPY $w0, implicit-def $x19

llvm/test/CodeGen/AMDGPU/postra-machine-sink.mir

+# RUN: llc -march=amdgcn -mcpu=gfx908 -run-pass=postra-machine-sink -verify-machineinstrs -o - %s | FileCheck %s
+
+# Don't sink copy that writes sub-register of another copy source register
+# CHECK-LABEL: name: donotsinkcopy
+# CHECK-LABEL: bb.0:
+# CHECK: renamable $sgpr1 = COPY renamable $sgpr2
+# CHECK-LABEL: bb.1:
+# CHECK: liveins: $sgpr0_sgpr1:0x00000003
+# CHECK: renamable $vgpr1_vgpr2 = COPY renamable $sgpr0_sgpr1
+
+---
+name: donotsinkcopy
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $sgpr0, $sgpr2, $vgpr3
+    S_CMP_LT_U32 renamable $sgpr2, target-flags(amdgpu-gotprel32-lo) 2, implicit-def $scc
+    renamable $sgpr1 = COPY renamable $sgpr2
+    renamable $vgpr1_vgpr2 = COPY renamable $sgpr0_sgpr1
+    S_CBRANCH_SCC0 %bb.2, implicit $scc
+
+  bb.1:
+    liveins: $vgpr2, $sgpr0_sgpr1:0x00000001
+    S_BRANCH %bb.4
+
+  bb.2:
+    liveins: $sgpr1
+
+  bb.4:
+   S_ENDPGM 0
+...