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

[Chandler Carruth]

REPOSITORY
  rL LLVM

================
Comment at: lib/Transforms/Utils/LCSSA.cpp:97
@@ -96,2 +96,3 @@
   SmallVector<PHINode *, 16> AddedPHIs;
+  SmallPtrSet<PHINode *, 8> PostProcessPHIs;
 
----------------
Rather than a set pointing into AddedPHIs, just keep a list? We don't put PHI nodes into AddedPHIs more than once, so there is no need for set logic, just a separate list for the ones needing post-processing?

================
Comment at: lib/Transforms/Utils/LCSSA.cpp:136-137
@@ +135,4 @@
+
+    // LoopSimplify might skip to simplify some loops (e.g. when indirect
+    // branches are involved). In such situations, it might happen that an exit
+    // for Loop L1 is the header of a disjoint Loop L2. Thus, when we create
----------------
I would say "LoopSimplify might fail to simplify some loops (...)."

================
Comment at: lib/Transforms/Utils/LCSSA.cpp:139
@@ +138,3 @@
+    // for Loop L1 is the header of a disjoint Loop L2. Thus, when we create
+    // PHIs in one of such exits we are also inserting PHIs in L2 header. This
+    // could break LCSSA form for L2 because these inserted PHIs can also have
----------------
"PHIs in such an exit block, we are also inserting PHIs into L2's header." reads a bit better to me.

================
Comment at: lib/Transforms/Utils/LCSSA.cpp:141
@@ +140,3 @@
+    // could break LCSSA form for L2 because these inserted PHIs can also have
+    // uses in L2 exits. Remember all PHIs in such situation as to revisit than
+    // later on.  FIXME: Remove this if indirectbr support into LoopSimplify
----------------
"can also have uses outside of L2."? It doesn't matter where the uses are, the key is that we're inserting a loop value in the form of a PHI node in the header.

================
Comment at: lib/Transforms/Utils/LCSSA.cpp:182-192
@@ -164,2 +181,13 @@
+
+  // Post process PHI instructions that were inserted into another disjoint loop
+  // and update their exits properly.
+  for (auto *I : PostProcessPHIs) {
+    BasicBlock *PHIBB = I->getParent();
+    Loop *OtherLoop = LI->getLoopFor(PHIBB);
+    SmallVector<BasicBlock *, 8> EBs;
+    OtherLoop->getExitBlocks(EBs);
+    if (EBs.empty())
+      continue;
+    processInstruction(*OtherLoop, *I, DT, EBs, PredCache, LI);
   }
 
----------------
Add a comment that we specifically recurse here?

Note that I can probably blow out the stack here with a suitably horrible crafted IR: simply set up a chain of loops such as this that is arbitrarily long. It's pretty contrived, but at least leave a FIXME that we should really convert this entire thing to a worklist approach where we process a vector of instructions...

http://reviews.llvm.org/D6624

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