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

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

Looks sensible to me. Some points:

- I'd also like to see a .mir test instead of the .ll one.
- What happens in this case: Will we loop endlessly because B=A is removed and re-added to BB1?

BB0:

  A = B;
  CondJmp BBX, BB1

BB1:

  B = A;
  CondJmp BBY, BB1



================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:902
+/// We may hoist B = A from BB0/BB2 to BB1.
+bool RegisterCoalescer::removePartialRedundency(const CoalescerPair &CP,
+                                                MachineInstr *CopyMI) {
----------------
Redundancy


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


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


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:935
+  MachineBasicBlock *CopyLeftBB = nullptr;
+  for (auto Pred : MBB->predecessors()) {
+    VNInfo *PVal = IntA.getVNInfoBefore(LIS->getMBBEndIdx(Pred));
----------------
I'd prefer to spell out the types instead of `auto` as that is friendlier to people reading the code.
Same with the inner VNI loop.


Repository:
  rL LLVM

https://reviews.llvm.org/D28585