[PATCH] D13123: PeepholeOptimizer: Remove redundant copies

[Quentin Colombet via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.

Hi Matt,

I’m fine with the patch modulo few nitpicks.
For a long term solution, I believe the right thing is to teach the copy rewriting logic to handle those cases. The rationale is that the proposed approach is, unlike the rewriting logic, limited to basic block scope and does not handle multi copies.

That being said, we can reconsider when we see motivating examples.

Cheers,
-Quentin


================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:166
@@ +165,3 @@
+    /// virtual register, replace this copy with the previous copy.
+    bool foldRedundantCopy(MachineInstr *MI,
+                           SmallSet<unsigned, 4> &CopiedFromRegs,
----------------
Please described the other arguments and how they are modified/use within the method.

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:1381
@@ +1380,3 @@
+  DenseMap<unsigned, MachineInstr *> &CopyMIs) {
+  unsigned SrcReg = MI->getOperand(1).getReg();
+  if (!TargetRegisterInfo::isVirtualRegister(SrcReg))
----------------
Assert that MI is a copy.

================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:1389
@@ +1388,3 @@
+
+  if (CopySrcRegs.insert(SrcReg).second) {
+    // First copy of this reg seen.
----------------
I don’t know if this is something you intend to support, but with this check, we only consider the first copy we see.
I.e., we do not consider thing like:
… = src
… = src.sub1

Anyway, may be worth adding a FIXME saying we will consider this when we see motivating examples.


http://reviews.llvm.org/D13123