[PATCH] D15230: [BranchFolding] Merge MMOs during tail merge

[Geoff Berry via llvm-commits]

gberry added a comment.

A few comments below.


================
Comment at: lib/CodeGen/BranchFolding.cpp:760
@@ +759,3 @@
+
+  // Mostly, MI1's MMO count is 1 or zero. So we don't have to use
+  // SmallSet.
----------------
You might want to change this comment to refer to the threshold check done in the caller.

================
Comment at: lib/CodeGen/BranchFolding.cpp:784
@@ +783,3 @@
+  // MergeThreshold,
+  // Merge MMOs from memory operations in the common block. Otherwise, we remove
+  // them.
----------------
This comment formatting should be cleaned up.  Don't capitalize "Merge" on this line and move it up to the above line and re-wrap it.

================
Comment at: lib/CodeGen/BranchFolding.cpp:820
@@ +819,3 @@
+      unsigned I1MMOsCount = E1 - I1;
+      unsigned I2MMOsCount = E2 - I2;
+
----------------
It seems to me that adding MachineInstr::getNumMemOperands() and using it here would be cleaner.


http://reviews.llvm.org/D15230