[MachineCopyPropagation] Handle COPY with overlapping source/dest.

[efriedma]

Summary

MachineCopyPropagation::CopyPropagateBlock has a bunch of special handling for COPY instructions. This handling assumes that COPY instructions do not modify the source of the copy; this is wrong if the COPY destination overlaps the source.

To fix the bug, check explicitly for this situation, and fall back to the generic instruction handling.

This bug can't happen for most register classes because they don't have this sort of overlap, but there are a few register classes where this is possible. The testcase uses the AArch64 QQQQ register class.

[gberry]

I'm not sure I see what's wrong with removing the second COPY in either of these cases, unless the semantics of the multi-register COPY are that each component register is copied in order?

[efriedma]

COPY in registers is sort of like memmove in memory; the value in the destination is equivalent to the value that was in the source.

If you try copyprop5 on trunk, you'll find it eliminates the second COPY, but not the first, so q28 and q29 contain the wrong value at the point of the call.

[gberry]

I get it now. LGTM

lib/CodeGen/MachineCopyPropagation.cpp

 
 void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
   DEBUG(dbgs() << "MCP: CopyPropagateBlock " << MBB.getName() << "\n");
 
   for (MachineBasicBlock::iterator I = MBB.begin(), E = MBB.end(); I != E; ) {
     MachineInstr *MI = &*I;
     ++I;
 
-    if (MI->isCopy()) {
+    // Analyze copies (which don't overlap themselves).
+    if (MI->isCopy() && !TRI->regsOverlap(MI->getOperand(0).getReg(),
+                                          MI->getOperand(1).getReg())) {
       unsigned Def = MI->getOperand(0).getReg();
       unsigned Src = MI->getOperand(1).getReg();
 
       assert(!TargetRegisterInfo::isVirtualRegister(Def) &&
              !TargetRegisterInfo::isVirtualRegister(Src) &&
              "MachineCopyPropagation should be run after register allocation!");
 
       // The two copies cancel out and the source of the first copy

test/CodeGen/AArch64/machine-dead-copy.mir

 
 # RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass machine-cp  -verify-machineinstrs  -o - %s | FileCheck %s
 
 --- |
   define i32 @copyprop1(i32 %a, i32 %b) { ret i32 %a }
   define i32 @copyprop2(i32 %a, i32 %b) { ret i32 %a }
   define i32 @copyprop3(i32 %a, i32 %b) { ret i32 %a }
   define i32 @copyprop4(i32 %a, i32 %b) { ret i32 %a }
+  define i32 @copyprop5(i32 %a, i32 %b) { ret i32 %a }
+  define i32 @copyprop6(i32 %a, i32 %b) { ret i32 %a }
   declare i32 @foo(i32)
 ...
 ---
 # The first copy is dead copy which is not used.
 # CHECK-LABEL: name: copyprop1
 # CHECK: bb.0:
 # CHECK-NOT: $w20 = COPY
 name: copyprop1
body: |
   bb.0:
     liveins: $w0,  $w1
     $w20 = COPY $w0
     $w0 = COPY $w20
     BL @foo, csr_aarch64_aapcs, implicit $w0, implicit-def $w0
     RET_ReallyLR implicit $w0
 ...
 
+# Don't try to erase any COPY which overlaps itself.
+# CHECK-LABEL: name: copyprop5
+# CHECK: bb.0:
+# CHECK: COPY killed $q26_q27_q28_q29
+# CHECK: COPY killed $q28_q29_q30_q31
+name: copyprop5
+body: |
+  bb.0:
+    liveins: $q26_q27_q28_q29
+    $q28_q29_q30_q31 = COPY killed $q26_q27_q28_q29
+    $q26_q27_q28_q29 = COPY killed $q28_q29_q30_q31
+    BL @foo, csr_aarch64_aapcs, implicit killed $q26_q27_q28_q29
+    RET_ReallyLR
+...
+
+# Don't try to analyze any COPY which overlaps itself.
+# CHECK-LABEL: name: copyprop6
+# CHECK: bb.0:
+# CHECK: COPY killed $q26_q27_q28_q29
+# CHECK: $q30 = COPY $q28
+name: copyprop6
+body: |
+  bb.0:
+    liveins: $q26_q27_q28_q29
+    $q28_q29_q30_q31 = COPY killed $q26_q27_q28_q29
+    $q30 = COPY $q28
+    BL @foo, csr_aarch64_aapcs, implicit killed $q28_q29_q30_q31
+    RET_ReallyLR
+...
+