[PATCH] D52374: [MachineCopyPropagation] Reimplement CopyTracker in terms of register units

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:133-136
+  bool hasAvailableCopies() {
+    // TODO: This is conservatively correct, but is it useful?
+    return !Copies.empty();
   }
----------------
This feels odd now, as in the new code you say `Avail = false` instead of removing elements from the `Copies` set... Would it make sense to maintain a counter of entries with `Avail == true` for this?


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:148-155
+  MachineInstr *findCopy(unsigned Reg, const TargetRegisterInfo &TRI,
+                         bool MustBeAvailable = false) {
+    MCRegUnitIterator RUI(Reg, &TRI);
+    MachineInstr *MI = findCopyForUnit(*RUI, TRI, MustBeAvailable);
+    if (!MI || !TRI.isSubRegisterEq(MI->getOperand(0).getReg(), Reg))
+      return nullptr;
+    return MI;
----------------
Is it correct to only check the first register unit here? Isn't it possible that only some of the units gots clobbered in the meantime, while the first one still makes it seem like the copy is available?


Repository:
  rL LLVM

https://reviews.llvm.org/D52374