[PATCH] [PR18861] Fix for LoopUnroll pass was breaking LCSSA form while completely removing loop

[Dinesh Dwivedi]

Hi chandlerc,

I have already sent mail to llvm-commits [http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140303/207346.html].

If LoopUnroll pass decides to completely unroll a loop, before deleting it [deleteLoopFromQueue()] from the loop list, it updates parent loop for all blocks in the loop to be unrolled [updateUnloop()->updateBlockParents()]. Candidate for parent loop is based on finding nearest parent loop among the block successors [getNearestLoop()], and returns null if a block does not have any successor [e.g. exit blocks having return statement, not sure if this may happen in other cases]. This causes the block to come out of loop nest. Now if this block is using some variable assigned inside loop nest, then it breaks LCSSA form.

This is what I have come up to fix LCSSA while detaching exit block from loop nest.

I have assumed few things.
1. As block was part of loop getting unrolled, it will have predecessor which is part of loop nest. 
2. If any value used as instruction operand in the block whose parent is out side the loop getting unrolled, we have to add PHI node from all predecessor of the block for it to maintain LCSSA.

I will appreciate any comment or pointer to right direction.

http://llvm-reviews.chandlerc.com/D2976

Files:
  lib/Analysis/LoopInfo.cpp
  test/Transforms/LoopUnroll/pr18861.ll

Index: lib/Analysis/LoopInfo.cpp
===================================================================
--- lib/Analysis/LoopInfo.cpp
+++ lib/Analysis/LoopInfo.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Metadata.h"
+#include "llvm/IR/PredIteratorCache.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include <algorithm>
@@ -439,6 +440,8 @@
   UnloopUpdater(Loop *UL, LoopInfo *LInfo) :
     Unloop(UL), LI(LInfo), DFS(UL), FoundIB(false) {}
 
+  void fixLCSSAForDetachedBlock(BasicBlock *BB);
+
   void updateBlockParents();
 
   void removeBlocksFromAncestors();
@@ -450,6 +453,34 @@
 };
 } // end anonymous namespace
 
+/// fixLCSSAForDetachedBlocks - Fix LCSSA if basic block was removed from the
+/// loop getting unrolled and now not inside any loop.
+void UnloopUpdater::fixLCSSAForDetachedBlock(BasicBlock *BB) {
+  assert(!LI->getLoopFor(BB) && "block part of other loop");
+  for (BasicBlock::iterator I = BB->begin(), ie = BB->end(); I != ie; ++I) {
+    Instruction *Inst = dyn_cast<Instruction>(&*I);
+    for (User::op_iterator Op = Inst->op_begin(), Oe = Inst->op_end(); Op != Oe;
+         ++Op)
+      if (Instruction *Oi = dyn_cast<Instruction>(*Op))
+        if (!Unloop->contains(LI->getLoopFor(Oi->getParent()))) {
+          PredIteratorCache PIC;
+          if (PIC.GetNumPreds(BB)) {
+            PHINode *OiPN =
+                PHINode::Create(Oi->getType(), PIC.GetNumPreds(BB),
+                                Oi->getName() + ".lcssa", BB->getFirstNonPHI());
+            for (BasicBlock **PI = PIC.GetPreds(BB); *PI; ++PI)
+              OiPN->addIncoming(Oi, *PI);
+
+            for (Value::use_iterator U = Oi->use_begin(), Ue = Oi->use_end();
+                 U != Ue; ++U)
+              if (Instruction *Ui = dyn_cast<Instruction>(*U))
+                if (OiPN != Ui && Ui->getParent() == BB)
+                  Ui->replaceUsesOfWith(Oi, OiPN);
+          }
+        }
+  }
+}
+
 /// updateBlockParents - Update the parent loop for all blocks that are directly
 /// contained within the original "unloop".
 void UnloopUpdater::updateBlockParents() {
@@ -468,6 +499,9 @@
         assert((NL != Unloop && (!NL || NL->contains(Unloop))) &&
                "uninitialized successor");
         LI->changeLoopFor(*POI, NL);
+
+        if (!NL)
+          fixLCSSAForDetachedBlock(*POI);
       }
       else {
         // Or the current block is part of a subloop, in which case its parent
@@ -494,6 +528,10 @@
         assert(NL != Unloop && (!NL || NL->contains(Unloop)) &&
                "uninitialized successor");
         LI->changeLoopFor(*POI, NL);
+
+        if (!NL)
+          fixLCSSAForDetachedBlock(*POI);
+
         Changed = true;
       }
     }
Index: test/Transforms/LoopUnroll/pr18861.ll
===================================================================
--- /dev/null
+++ test/Transforms/LoopUnroll/pr18861.ll
@@ -0,0 +1,43 @@
+; RUN: opt < %s -loop-unroll -indvars -disable-output
+
+ at b = external global i32, align 4
+
+; Function Attrs: nounwind uwtable
+define void @fn1() #0 {
+entry:
+  br label %for.cond1thread-pre-split
+
+for.cond1thread-pre-split:                        ; preds = %for.inc8, %entry
+  %storemerge1 = phi i32 [ 0, %entry ], [ %inc9, %for.inc8 ]
+  br i1 undef, label %for.inc8, label %for.cond2.preheader.lr.ph
+
+for.cond2.preheader.lr.ph:                        ; preds = %for.cond1thread-pre-split
+  br label %for.cond2.preheader
+
+for.cond2.preheader:                              ; preds = %for.inc5, %for.cond2.preheader.lr.ph
+  br label %for.cond2
+
+for.cond2:                                        ; preds = %for.body3, %for.cond2.preheader
+  %storemerge = phi i32 [ %add, %for.body3 ], [ 0, %for.cond2.preheader ]
+  %cmp = icmp slt i32 %storemerge, 1
+  br i1 %cmp, label %for.body3, label %for.inc5
+
+for.body3:                                        ; preds = %for.cond2
+  %tobool4 = icmp eq i32 %storemerge, 0
+  %add = add nsw i32 %storemerge, 1
+  br i1 %tobool4, label %for.cond2, label %if.then
+
+if.then:                                          ; preds = %for.body3
+  store i32 %storemerge1, i32* @b, align 4
+  ret void
+
+for.inc5:                                         ; preds = %for.cond2
+  br i1 undef, label %for.cond1.for.inc8_crit_edge, label %for.cond2.preheader
+
+for.cond1.for.inc8_crit_edge:                     ; preds = %for.inc5
+  br label %for.inc8
+
+for.inc8:                                         ; preds = %for.cond1.for.inc8_crit_edge, %for.cond1thread-pre-split
+  %inc9 = add nsw i32 %storemerge1, 1
+  br label %for.cond1thread-pre-split
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2976.1.patch
Type: text/x-patch
Size: 4675 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140305/6f51cd58/attachment.bin>