[PATCH] D17474: MachineCopyPropagation: Keep scanning through instructions with regmasks

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 22 10:44:50 PST 2016


qcolombet added inline comments.

================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:99
@@ +98,3 @@
+      // Continue if the instruction explicitely preserves the register
+      // (calls often do)
+      bool RegIsPreserved = false;
----------------
Period.

================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:105
@@ +104,3 @@
+          RegIsPreserved = true;
+          break;
+        }
----------------
Although I doubt it happens in practice, MIs might have several RegMask operands.
(After scanning through the uses of getRegMask(), it looks to me that this pass is the only one that would do wrong thing with several regmasks. It happened before this patch, e.g., line 238, and thus we can fix as a follow-up patch.)
Can’t we just return false if Reg is clobbered instead of checking if it is preserved?

================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:278
@@ -264,3 +277,3 @@
     // prune the available copies, but treat it like a basic block boundary for
     // now.
     if (RegMask) {
----------------
Update this comment, the end is no more relevant.

================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:307
@@ +306,3 @@
+          CopyMap.erase(I);
+      }
+      for (SourceMap::iterator I = SrcMap.begin(), E = SrcMap.end(), Next;
----------------
We could factorize the two previous loops into a helper function.


Repository:
  rL LLVM

http://reviews.llvm.org/D17474





More information about the llvm-commits mailing list