[PATCH] D67794: [MachineCopyPropagation] Extend MCP to do trivial copy backward propagation

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet requested changes to this revision.
qcolombet added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:291
+    MachineInstr &Copy, MachineInstr &SrcMI) {
+  MachineOperand &SrcOp = SrcMI.getOperand(0);
+  if (!(SrcOp.isReg() && SrcOp.isDef() &&
----------------
Hard coding 0, seems artificial to me.
Could we find which operand defines Copy.getOperand(1).getReg() instead?


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:295
+        !SrcOp.isTied() && !SrcOp.isImplicit() &&
+        !MRI->isReserved(SrcOp.getReg())))
+    return false;
----------------
I find this condition hard to read.
Could you break it into smaller conditions?

E.g.,
```
if (!SrcOp.isRenamable())
  return false;
if (SrcOp.isImplicit())
  return false;
```

and so on.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:328
+    }
+  }
+  if (!SrcMI || !isSafeBackwardCopyPropagation(Copy, *SrcMI))
----------------
Could you build this information as we iterate into the basic block?
I am afraid the compile time cost of this loop to be fairly large otherwise, since we are going to traverse all the instructions between two instructions again and again.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67794/new/

https://reviews.llvm.org/D67794