MachineCopyPropagation: Do not consider undef operands as clobbers

[MatzeB]

Summary

This was originally introduced in r278321 to work around correctness
problems in the ExecutionDepsFix pass; Probably also to keep the
performance benefits of breaking the false dependencies which of course
also affect undef operands.

ExecutionDepsFix has been improved here recently (see for example
r278321) so we should not need this exception any longer.

[qcolombet]

Great if the ExecutionDepsFixer has been fixed!

LGTM modulo having the original test case a bit more constrained.

test/CodeGen/X86/copy-propagation.ll

16 ↗

(On Diff #87061)

Could you do a CHECK-NEXT of that move or a CHECK-NOT of xor

llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp

for (const MachineOperand &MO : MI->operands()) {
       // for elimination.
       for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
         Reg2MIMap::iterator CI = CopyMap.find(*AI);
         if (CI != CopyMap.end()) {
           DEBUG(dbgs() << "MCP: Copy is used - not dead: "; CI->second->dump());
           MaybeDeadCopies.remove(CI->second);
         }
       }
-      // Treat undef use like defs for copy propagation but not for
-      // dead copy. We would need to do a liveness check to be sure the copy
-      // is dead for undef uses.
-      // The backends are allowed to do whatever they want with undef value
-      // and we cannot be sure this register will not be rewritten to break
-      // some false dependencies for the hardware for instance.
-      if (MO.isUndef())
-        Defs.push_back(Reg);
     }
 
     // The instruction has a register mask operand which means that it clobbers
     // a large set of registers.  Treat clobbered registers the same way as
     // defined registers.
     if (RegMask) {
       // Erase any MaybeDeadCopies whose destination register is clobbered.
       for (SmallSetVector<MachineInstr *, 8>::iterator DI =

llvm/trunk/test/CodeGen/X86/copy-propagation.ll

 ; RUN: llc %s -mattr=+avx -o - | FileCheck %s
-; PR21743.
+; Originally from http://llvm.org/PR21743.
 
 target triple = "x86_64-pc-win32-elf"
 
-; Check that copy propagation conservatively assumes that undef register
-; can be rewritten by the backend to break false dependencies for the
-; hardware.
-; In this function we are in this situation:
-; reg1 = copy reg2
-; = inst reg2<undef>
-; reg2 = copy reg1
-; Copy propagation used to remove the last copy.
-; This is incorrect because the undef flag on reg2 in inst, allows next
-; passes to put whatever trashed value in reg2 that may help.
-; In practice we end up with this code:
-; reg1 = copy reg2
-; reg2 = 0
-; = inst reg2<undef>
-; reg2 = copy reg1
-; Therefore, removing the last copy is wrong.
+; Copy propagation may remove COPYs if the result is only used by undef
+; operands.
 ;
 ; CHECK-LABEL: foo:
 ; CHECK: movl $339752784, %e[[INDIRECT_CALL1:[a-z]+]]
 ; CHECK: callq *%r[[INDIRECT_CALL1]]
 ; Copy the result in a temporary.
-; Note: Technically the regalloc could have been smarter and this move not required,
-; which would have hidden the bug.
+; Note: Technically the regalloc could have been smarter and this move not
+; required, which would have hidden the bug.
 ; CHECK: vmovapd  %xmm0, [[TMP:%xmm[0-9]+]]
-; Crush xmm0.
-; CHECK-NEXT: vxorps %xmm0, %xmm0, %xmm0
+; CHECK-NOT: vxorps  %xmm0, %xmm0, %xmm0
+; CHECK-NEXT: vcvtsi2sdq      %rsi, %xmm0, %xmm6
 ; CHECK: movl $339772768, %e[[INDIRECT_CALL2:[a-z]+]]
+; CHECK-NOT: vmovapd %xmm7, %xmm0
+; CHECK-NEXT: vmovapd %xmm6, %xmm1
 ; Set TMP in the first argument of the second call.
-; CHECK-NEXT: vmovapd [[TMP]], %xmm0
-; CHECK: callq *%r[[INDIRECT_CALL2]]
+; CHECK_NEXT: callq *%r[[INDIRECT_CALL2]]
 ; CHECK: retq
 define double @foo(i64 %arg) {
 top:
   %tmp = call double inttoptr (i64 339752784 to double (double, double)*)(double 1.000000e+00, double 0.000000e+00)
   tail call void asm sideeffect "", "x,~{xmm1},~{xmm2},~{xmm3},~{xmm4},~{xmm5},~{xmm6},~{xmm7},~{xmm8},~{xmm9},~{xmm10},~{xmm11},~{xmm12},~{xmm13},~{xmm14},~{xmm15},~{dirflag},~{fpsr},~{flags}"(double %tmp)
   %tmp1 = sitofp i64 %arg to double
   call void inttoptr (i64 339772768 to void (double, double)*)(double %tmp, double %tmp1)
   %tmp3 = fadd double %tmp1, %tmp
   ret double %tmp3
 }