[PATCH] D28585: [RegisterCoalescing] Remove partial redundent copy

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 13 12:48:09 PST 2017


wmi marked 5 inline comments as done.
wmi added a comment.

Thanks for the review.

> I'd also like to see a .mir test instead of the .ll one.

A mir test is Added.

> What happens in this case: Will we loop endlessly because B=A is removed and re-added to BB1?

It is guaranteed by the requirement that BB1 has only one successor. In this way, we can only move the copy from a hotter to a colder place, so no endless loop will happen. I add this correctness precondition in the comment.

Thanks,
Wei.



================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:903
+bool RegisterCoalescer::removePartialRedundency(const CoalescerPair &CP,
+                                                MachineInstr *CopyMI) {
+  assert(!CP.isPhys());
----------------
MatzeB wrote:
> Use `MachineInstr &CopyMI` to indicate it cannot be nullptr.
Fixed.


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:908
+
+  MachineBasicBlock *MBB = CopyMI->getParent();
+  if (MBB->isEHPad())
----------------
MatzeB wrote:
> Use `MachineBasicBlock &MBB`.
Fixed.


Repository:
  rL LLVM

https://reviews.llvm.org/D28585





More information about the llvm-commits mailing list