[PATCH] D52370: [MachineCopyPropagation] Rework how we manage RegMask clobbers

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:263
 
+  for (auto I = PrevCopy->getIterator(), E = Copy.getIterator(); I != E; ++I) {
+    const MachineOperand *RegMask = nullptr;
----------------
Can you add a comment here along the lines of "we haven't checked regmasks yet, do this now".

Can this be expressed as range based for? (something like ```for (const MachineInstr &MI : make_range(PrevCopy->getIterator(), Copy.getIterator()))```?


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:266-273
+      if (MO.isRegMask()) {
+        RegMask = &MO;
+        break;
+      }
+    if (!RegMask)
+      continue;
+    if (RegMask->clobbersPhysReg(Src) || RegMask->clobbersPhysReg(Def))
----------------
why not just check the regmask operand inside the loop? Should make the code simpler and get away with the artifical restriction to a single regmask operand per instruction (not that there is any sensible usecase for multiple but anyway...)


Repository:
  rL LLVM

https://reviews.llvm.org/D52370