[PATCH] D17889: [MachineCopyPropagation] Expose more dead copies across instructions with regmasks

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 15 11:29:43 PDT 2016

qcolombet added a comment.

Hi Jun,

Thanks for the additional test case.

The iteration in the loop still looks problematic to me. See my inlined comment.


Comment at: lib/CodeGen/MachineCopyPropagation.cpp:288-302
@@ -287,10 +287,17 @@
       // Erase any MaybeDeadCopies whose destination register is clobbered.
-      for (MachineInstr *MaybeDead : MaybeDeadCopies) {
+      for (SmallSetVector<MachineInstr *, 8>::iterator DI =
+               MaybeDeadCopies.begin();
+           DI != MaybeDeadCopies.end();) {
+        MachineInstr *MaybeDead = *DI;
         unsigned Reg = MaybeDead->getOperand(0).getReg();
-        if (!RegMask->clobbersPhysReg(Reg))
+        if (!RegMask->clobbersPhysReg(Reg)) {
+          ++DI;
+        }
         DEBUG(dbgs() << "MCP: Removing copy due to regmask clobbering: ";
+        MaybeDeadCopies.remove(MaybeDead);
         Changed = true;
junbuml wrote:
> Modified the way I iterate the loop; do not increase the iterator when removing an element. 
This is not clear to me how the iterator makes progress when we remove an element now.
Could you explain what I am missing, please?


More information about the llvm-commits mailing list