MachineCopyPropagation has special logic for removing COPY instructions. It will...

This actually totally breaks and causes the machine verifier to cry in several cases, one of which being:

%RAX<def> = COPY %RCX<kill>
%ECX<def> = COPY %EAX<kill>, %RAX<imp-use,kill>

These subregister copies are together identified as noops, so are both removed. However, the second one as it has an imp-use gets converted into a kill:

%ECX<def> = KILL %EAX<kill>, %RAX<imp-use,kill>

As the original COPY has been removed, the verifier goes into tears at the use of undefined EAX and RAX.

There are several hacky solutions to this hacky problem (which is all to do with imp-use/def weirdnesses), but the least hacky I've come up with is to *always* remove COPYs by converting to KILLs. KILLs are no-ops to the code generator so the generated code doesn't change (which is why they were partially used in the first place), but using them also keeps the def/use and imp-def/imp-use chains alive:

%RAX<def> = KILL %RCX<kill>
%ECX<def> = KILL %EAX<kill>, %RAX<imp-use,kill>

The patch passes all test cases including the ones that check the removal of MOVs in this circumstance, along with an extra test I added to check subregister behaviour (which made the machine verifier fall over before my patch).

The patch also adds some DEBUG() statements because the file hadn't got any.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@199797 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/MachineCopyPropagation.cpp b/lib/CodeGen/MachineCopyPropagation.cpp
index 4f48e2cd9720a96d3ada07f5fec611ba3174e22f..e41d3eee216a7c54901cf627bf1265213733f390 100644 (file)
--- a/lib/CodeGen/MachineCopyPropagation.cpp
+++ b/lib/CodeGen/MachineCopyPropagation.cpp
@@ -127,13 +127,10 @@ static bool isNopCopy(MachineInstr *CopyMI, unsigned Def, unsigned Src,
 }
 
 // Remove MI from the function because it has been determined it is dead.
-// Turn it into a noop KILL instruction if it has super-register liveness
-// adjustments.
+// Turn it into a noop KILL instruction as opposed to removing it to
+// maintain imp-use/imp-def chains.
 void MachineCopyPropagation::removeCopy(MachineInstr *MI) {
-  if (MI->getNumOperands() == 2)
-    MI->eraseFromParent();
-  else
-    MI->setDesc(TII->get(TargetOpcode::KILL));
+  MI->setDesc(TII->get(TargetOpcode::KILL));
 }
 
 bool MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
@@ -142,6 +139,8 @@ bool MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
   DenseMap<unsigned, MachineInstr*> CopyMap;         // Def -> copies map
   SourceMap SrcMap; // Src -> Def map
 
+  DEBUG(dbgs() << "MCP: CopyPropagateBlock " << MBB.getName() << "\n");
+
   bool Changed = false;
   for (MachineBasicBlock::iterator I = MBB.begin(), E = MBB.end(); I != E; ) {
     MachineInstr *MI = &*I;
@@ -176,6 +175,8 @@ bool MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
           // CALL
           // %RAX<def> = COPY %RSP
 
+          DEBUG(dbgs() << "MCP: copy is a NOP, removing: "; MI->dump());
+
           // Clear any kills of Def between CopyMI and MI. This extends the
           // live range.
           for (MachineBasicBlock::iterator I = CopyMI, E = MI; I != E; ++I)
@@ -191,10 +192,14 @@ bool MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
       // If Src is defined by a previous copy, it cannot be eliminated.
       for (MCRegAliasIterator AI(Src, TRI, true); AI.isValid(); ++AI) {
         CI = CopyMap.find(*AI);
-        if (CI != CopyMap.end())
+        if (CI != CopyMap.end()) {
+          DEBUG(dbgs() << "MCP: Copy is no longer dead: "; CI->second->dump());
           MaybeDeadCopies.remove(CI->second);
+        }
       }
 
+      DEBUG(dbgs() << "MCP: Copy is a deletion candidate: "; MI->dump());
+
       // Copy is now a candidate for deletion.
       MaybeDeadCopies.insert(MI);
 
@@ -255,8 +260,10 @@ bool MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
       // for elimination.
       for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
         DenseMap<unsigned, MachineInstr*>::iterator CI = CopyMap.find(*AI);
-        if (CI != CopyMap.end())
+        if (CI != CopyMap.end()) {
+          DEBUG(dbgs() << "MCP: Copy is used - not dead: "; CI->second->dump());
           MaybeDeadCopies.remove(CI->second);
+        }
       }
     }
 
@@ -273,6 +280,8 @@ bool MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
         unsigned Reg = (*DI)->getOperand(0).getReg();
         if (MRI->isReserved(Reg) || !MaskMO.clobbersPhysReg(Reg))
           continue;
+        DEBUG(dbgs() << "MCP: Removing copy due to regmask clobbering: ";
+              (*DI)->dump());
         removeCopy(*DI);
         Changed = true;
         ++NumDeletes;

diff --git a/test/CodeGen/X86/machine-cp.ll b/test/CodeGen/X86/machine-cp.ll
index f04e111714ae9c243d55af7f8ec6d502b44dc1fc..0006b6ea7133ffbfe9a44b2c7f627be6e91874bf 100644 (file)
--- a/test/CodeGen/X86/machine-cp.ll
+++ b/test/CodeGen/X86/machine-cp.ll
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple=x86_64-apple-macosx -mcpu=nocona < %s | FileCheck %s
+; RUN: llc -mtriple=x86_64-apple-macosx -mcpu=nocona -verify-machineinstrs < %s | FileCheck %s
 
 ; After tail duplication, two copies in an early exit BB can be cancelled out.
 ; rdar://10640363
@@ -34,3 +34,27 @@ entry:
   %tmp8 = shufflevector <8 x i16> %T0, <8 x i16> %T1, <8 x i32> < i32 undef, i32 undef, i32 7, i32 2, i32 8, i32 undef, i32 undef , i32 undef >
   ret <8 x i16> %tmp8
 }
+
+define i32 @t3(i64 %a, i64 %b) nounwind  {
+entry:
+; CHECK-LABEL: t3:
+; CHECK: je [[LABEL:.*BB.*]]
+  %cmp1 = icmp eq i64 %b, 0
+  br i1 %cmp1, label %while.end, label %while.body
+
+; CHECK: [[LABEL]]:
+; CHECK-NOT: mov
+; CHECK: ret
+
+while.body:                                       ; preds = %entry, %while.body
+  %a.addr.03 = phi i64 [ %b.addr.02, %while.body ], [ %a, %entry ]
+  %b.addr.02 = phi i64 [ %rem, %while.body ], [ %b, %entry ]
+  %rem = srem i64 %a.addr.03, %b.addr.02
+  %cmp = icmp eq i64 %rem, 0
+  br i1 %cmp, label %while.end, label %while.body
+
+while.end:                                        ; preds = %while.body, %entry
+  %a.addr.0.lcssa = phi i64 [ %a, %entry ], [ %b.addr.02, %while.body ]
+  %t = trunc i64 %a.addr.0.lcssa to i32
+  ret i32 %t
+}