[PATCH] D22425: ExpandPostRAPseudos should transfer implicit uses, not only implicit defs

[Michael Kuperstein via llvm-commits]

mkuper created this revision.
mkuper added reviewers: qcolombet, MatzeB.
mkuper added subscribers: llvm-commits, echristo.
Herald added a subscriber: aemerson.

This fixes PR28560.

ExpandPostRAPseudos would transform:

```
%BL<def> = COPY %DL<kill>, %EBX<imp-use,kill>, %EBX<imp-def>
```
Into:

```
%BL<def> = MOV8rr %DL<kill>, %EBX<imp-def>
```

That caused problems for CriticalAntiDepBreaker which - I believe, correctly - assumes that if an instruction defs but doesn't use a register, that register was dead immediately before the instruction. However, in our case, the high part of EBX is very much alive.

Two notes about the tests:
1) The new instruction in the arm test isn't really new, it's a scheduling change. The vmov used to be immediately before the vld1, instead of after. Unfortunately, I don't understand ARM enough (= at all) to judge whether this is significant.
2) The regalloc for the x86 test looks like it could be better - and I'm not entirely sure we need the mov at all. But that's a separate issue.

https://reviews.llvm.org/D22425

Files:
  lib/CodeGen/ExpandPostRAPseudos.cpp
  test/CodeGen/ARM/twoaddrinstr.ll
  test/CodeGen/X86/pr28560.ll

Index: test/CodeGen/X86/pr28560.ll
===================================================================
--- test/CodeGen/X86/pr28560.ll
+++ test/CodeGen/X86/pr28560.ll
@@ -0,0 +1,13 @@
+; RUN: llc -mtriple=i686-pc-linux -print-after=postrapseudos < %s 2>&1 | FileCheck %s
+
+; CHECK: MOV8rr %{{[A-D]}}L, %E[[R:[A-D]]]X<imp-use>, %E[[R]]X<imp-def>
+define i32 @foo(i32 %i, i32 %k, i8* %p) {
+  %f = icmp ne i32 %i, %k
+  %s = zext i1 %f to i8
+  %ret = zext i1 %f to i32
+  br label %next
+next:
+  %d = add i8 %s, 5
+  store i8 %d, i8* %p
+  ret i32 %ret
+}
Index: test/CodeGen/ARM/twoaddrinstr.ll
===================================================================
--- test/CodeGen/ARM/twoaddrinstr.ll
+++ test/CodeGen/ARM/twoaddrinstr.ll
@@ -5,6 +5,7 @@
 ; This was orriginally a crasher trying to schedule the instructions.
 ; CHECK-LABEL:      PR13378:
 ; CHECK:        vld1.32
+; CHECK-NEXT:   vmov.i32
 ; CHECK-NEXT:   vst1.32
 ; CHECK-NEXT:   vst1.32
 ; CHECK-NEXT:   vmov.f32
Index: lib/CodeGen/ExpandPostRAPseudos.cpp
===================================================================
--- lib/CodeGen/ExpandPostRAPseudos.cpp
+++ lib/CodeGen/ExpandPostRAPseudos.cpp
@@ -51,7 +51,7 @@
   bool LowerSubregToReg(MachineInstr *MI);
   bool LowerCopy(MachineInstr *MI);
 
-  void TransferImplicitDefs(MachineInstr *MI);
+  void TransferImplicitOperands(MachineInstr *MI);
 };
 } // end anonymous namespace
 
@@ -61,19 +61,19 @@
 INITIALIZE_PASS(ExpandPostRA, "postrapseudos",
                 "Post-RA pseudo instruction expansion pass", false, false)
 
-/// TransferImplicitDefs - MI is a pseudo-instruction, and the lowered
-/// replacement instructions immediately precede it.  Copy any implicit-def
+/// TransferImplicitOperands - MI is a pseudo-instruction, and the lowered
+/// replacement instructions immediately precede it.  Copy any implicit
 /// operands from MI to the replacement instruction.
 void
-ExpandPostRA::TransferImplicitDefs(MachineInstr *MI) {
+ExpandPostRA::TransferImplicitOperands(MachineInstr *MI) {
   MachineBasicBlock::iterator CopyMI = MI;
   --CopyMI;
 
   for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
     MachineOperand &MO = MI->getOperand(i);
-    if (!MO.isReg() || !MO.isImplicit() || MO.isUse())
+    if (!MO.isReg() || !MO.isImplicit())
       continue;
-    CopyMI->addOperand(MachineOperand::CreateReg(MO.getReg(), true, true));
+    CopyMI->addOperand(MachineOperand::CreateReg(MO.getReg(), MO.isDef(), true));
   }
 }
 
@@ -167,7 +167,7 @@
                    DstMO.getReg(), SrcMO.getReg(), SrcMO.isKill());
 
   if (MI->getNumOperands() > 2)
-    TransferImplicitDefs(MI);
+    TransferImplicitOperands(MI);
   DEBUG({
     MachineBasicBlock::iterator dMI = MI;
     dbgs() << "replaced by: " << *(--dMI);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22425.64195.patch
Type: text/x-patch
Size: 2773 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160715/41095f35/attachment.bin>