[PATCH] [LCSSA] Handle PHI insertion in disjoint loops

Chandler Carruth chandlerc at gmail.com
Mon Dec 22 11:28:59 PST 2014


One totally nit-picky comment, and one substantial comment. Feel free to commit once addressed.


================
Comment at: lib/Transforms/Utils/LCSSA.cpp:144-146
@@ -134,1 +143,5 @@
+    // LoopSimplify gets improved.
+    Loop *OtherLoop = LI->getLoopFor(ExitBB);
+    if (OtherLoop && !L.contains(OtherLoop))
+      PostProcessPHIs.push_back(PN);
   }
----------------
I would write this as:

  if (auto OtherLoop = LI->getLoopFor(ExitBB))
    if (!L.contains(OtherLoop))
      PostProcessPHIs.push_back(PN)

================
Comment at: lib/Transforms/Utils/LCSSA.cpp:177-181
@@ +176,7 @@
+      continue;
+    for (unsigned j = 0, ee = PostProcessPHIs.size(); j != ee; ++j)
+      if (PostProcessPHIs[j] == AddedPHIs[i]) {
+        PostProcessPHIs.erase(PostProcessPHIs.begin()+j);
+        break;
+      }
+    AddedPHIs[i]->eraseFromParent();
----------------
Ow. This is quadratic in all cases AFAICT. Either we move all remaining elements in PostProcessPHIs down a slot, or we skip over an element in every trip throuh AddedPHIs (AddedPHIs is a superset and ordered the same).

Instead of all this, how about we do the post processing first, and then this loop? You can add a similar guard to skip post processing of phis with empty use lists. That avoids all the complexity here.

http://reviews.llvm.org/D6624

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list