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

[Justin Bogner via Phabricator via llvm-commits]

bogner marked 2 inline comments as done.
bogner added inline comments.


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:263
 
+  for (auto I = PrevCopy->getIterator(), E = Copy.getIterator(); I != E; ++I) {
+    const MachineOperand *RegMask = nullptr;
----------------
MatzeB wrote:
> 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()))```?
Done and done. I'm not entirely convinced the range-for is any more readable in this case, but it isn't worse either.


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:575-577
+        // Make sure we invalidate any entries in the copy maps before erasing
+        // the instruction.
+        Tracker.clobberRegister(Reg, *TRI);
----------------
This was missing in the version of the patch I uploaded before, which led to use-after-frees.


https://reviews.llvm.org/D52370