Page MenuHomePhabricator

PeepholeOptimizer: Remove redundant copies
ClosedPublic

Authored by arsenm on Sep 23 2015, 7:00 PM.

Details

Reviewers
qcolombet
hfinkel
Summary

If a virtual register is copied and another copy was already
seen, replace with the previous copy.

This pattern shows up from various operand restrictions
AMDGPU has which require inserting copies depending
on the register class of the operands.

Diff Detail

Event Timeline

arsenm updated this revision to Diff 35582.Sep 23 2015, 7:00 PM
arsenm retitled this revision from to PeepholeOptimizer: Remove redundant copies.
arsenm updated this object.
arsenm added a reviewer: qcolombet.
arsenm added a subscriber: llvm-commits.
hfinkel accepted this revision.Sep 24 2015, 2:51 AM
hfinkel added a reviewer: hfinkel.
hfinkel added a subscriber: hfinkel.

LGTM.

This revision is now accepted and ready to land.Sep 24 2015, 2:51 AM
qcolombet accepted this revision.Sep 25 2015, 9:25 AM
qcolombet edited edge metadata.

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

lib/CodeGen/PeepholeOptimizer.cpp
166

Please described the other arguments and how they are modified/use within the method.

1381

Assert that MI is a copy.

1389

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.

arsenm closed this revision.Sep 25 2015, 1:24 PM

r248611

lib/CodeGen/PeepholeOptimizer.cpp
1389

I'll look into that next. I saw some test improvements when I switched to inserting the mov that I don't see with this patch and this is probably why.