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

Junmo Park via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 20 20:28:01 PST 2015


flyingforyou marked 3 inline comments as done.

================
Comment at: lib/CodeGen/BranchFolding.cpp:756
@@ +755,3 @@
+  // Mostly, MI1's MMO count is 1 or zero. So we don't have to use
+  // SmallSet.
+  for (; I1 != E1; ++I1) {
----------------
When I wrote the code with N^2 search, there is a reason that most of cases, MI only has a one or zero MMO.
So, This routine is faster than using SmallSet.

In order to support that most of MI has one or zero MMO, I tested some benchmarks app and got a data.

When I investigated 12354 MI which is used by mergeMMOs function,
MI(MMO is 0) == 1595, MI(MMO is 1) == 10514. Sum of two cases is 12109 and it's 98%.

So, I don't think Smallset is useful in this case. 
But there is no explain why I use N^2 search, so I added the comment.

I want you agree with this, please.

================
Comment at: lib/CodeGen/BranchFolding.cpp:758
@@ +757,3 @@
+  for (; I1 != E1; ++I1) {
+    bool IsDupMMO = false;
+    for (I2 = MI2->memoperands_begin(); I2 != E2; ++I2) {
----------------
Thanks. 
You're right.  I2 = MI2->memoperands_begin() need to be set. But E2 = MI2->memoperands_end() is only needed when MI2's MMO is changed.


http://reviews.llvm.org/D14797





More information about the llvm-commits mailing list