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

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 15 14:56:23 PDT 2016


MatzeB accepted this revision.
MatzeB added a comment.

If BL is defined, it does not mean that the other lanes are dead before the instruction. So there is still a bug in the anti dependency breaker.

On the other hand this patch is obviously good and there is no reason not to do it, so LGTM.

Have you considered writing a .mir test ("llc -run-pass postrapseudos ...")? That should give you a much more stable test.


================
Comment at: lib/CodeGen/ExpandPostRAPseudos.cpp:72-77
@@ -71,8 +71,8 @@
 
   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));
   }
 }
----------------
If we are on it anyway we could clean this code up:
```
  for (const MachineOperand &MO : MI->implicit_operands()) {
    if (MO.isReg())
      CopyMI->addOperand(MO);
  }
```



https://reviews.llvm.org/D22425





More information about the llvm-commits mailing list