[llvm] r230856 - TRE: Just erase dead BBs and tweak the iteration loop not to increment the deleted BB iterator.

[Benjamin Kramer]

Author: d0k
Date: Sat Feb 28 10:47:27 2015
New Revision: 230856

URL: http://llvm.org/viewvc/llvm-project?rev=230856&view=rev
Log:
TRE: Just erase dead BBs and tweak the iteration loop not to increment the deleted BB iterator.

Leaving empty blocks around just opens up a can of bugs like PR22704. Deleting
them early also slightly simplifies code.

Thanks to Sanjay for the IR test case.

Modified:
    llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
    llvm/trunk/test/Transforms/TailCallElim/inf-recursion.ll

Modified: llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp?rev=230856&r1=230855&r2=230856&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp Sat Feb 28 10:47:27 2015
@@ -403,28 +403,19 @@ bool TailCallElim::runTRE(Function &F) {
   // alloca' is changed from being a static alloca to being a dynamic alloca.
   // Until this is resolved, disable this transformation if that would ever
   // happen.  This bug is PR962.
-  SmallVector<BasicBlock*, 8> BBToErase;
-  for (Function::iterator BB = F.begin(), E = F.end(); BB != E; ++BB) {
+  for (Function::iterator BBI = F.begin(), E = F.end(); BBI != E; /*in loop*/) {
+    BasicBlock *BB = BBI++; // FoldReturnAndProcessPred may delete BB.
     if (ReturnInst *Ret = dyn_cast<ReturnInst>(BB->getTerminator())) {
       bool Change = ProcessReturningBlock(Ret, OldEntry, TailCallsAreMarkedTail,
                                           ArgumentPHIs, !CanTRETailMarkedCall);
-      if (!Change && BB->getFirstNonPHIOrDbg() == Ret) {
+      if (!Change && BB->getFirstNonPHIOrDbg() == Ret)
         Change = FoldReturnAndProcessPred(BB, Ret, OldEntry,
                                           TailCallsAreMarkedTail, ArgumentPHIs,
                                           !CanTRETailMarkedCall);
-        // FoldReturnAndProcessPred may have emptied some BB. Remember to
-        // erase them.
-        if (Change && BB->empty())
-          BBToErase.push_back(BB);
-
-      }
       MadeChange |= Change;
     }
   }
 
-  for (auto BB: BBToErase)
-    BB->eraseFromParent();
-
   // If we eliminated any tail recursions, it's possible that we inserted some
   // silly PHI nodes which just merge an initial value (the incoming operand)
   // with themselves.  Check to see if we did and clean up our mess if so.  This
@@ -831,14 +822,11 @@ bool TailCallElim::FoldReturnAndProcessP
       ReturnInst *RI = FoldReturnIntoUncondBranch(Ret, BB, Pred);
 
       // Cleanup: if all predecessors of BB have been eliminated by
-      // FoldReturnIntoUncondBranch, we would like to delete it, but we
-      // can not just nuke it as it is being used as an iterator by our caller.
-      // Just empty it, and the caller will erase it when it is safe to do so.
-      // It is important to empty it, because the ret instruction in there is
-      // still using a value which EliminateRecursiveTailCall will attempt
-      // to remove.
+      // FoldReturnIntoUncondBranch, delete it.  It is important to empty it,
+      // because the ret instruction in there is still using a value which
+      // EliminateRecursiveTailCall will attempt to remove.
       if (!BB->hasAddressTaken() && pred_begin(BB) == pred_end(BB))
-        BB->getInstList().clear();
+        BB->eraseFromParent();
 
       EliminateRecursiveTailCall(CI, RI, OldEntry, TailCallsAreMarkedTail,
                                  ArgumentPHIs,

Modified: llvm/trunk/test/Transforms/TailCallElim/inf-recursion.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/TailCallElim/inf-recursion.ll?rev=230856&r1=230855&r2=230856&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/TailCallElim/inf-recursion.ll (original)
+++ llvm/trunk/test/Transforms/TailCallElim/inf-recursion.ll Sat Feb 28 10:47:27 2015
@@ -31,3 +31,24 @@ define float @fabsf(float %f) {
 }
 
 declare x86_fp80 @fabsl(x86_fp80 %f)
+
+; Don't crash while transforming a function with infinite recursion.
+define i32 @PR22704(i1 %bool) {
+entry:
+  br i1 %bool, label %t, label %f
+
+t:
+  %call1 = call i32 @PR22704(i1 1)
+  br label %return
+
+f:
+  %call = call i32 @PR22704(i1 1)
+  br label %return
+
+return:
+  ret i32 0
+
+; CHECK-LABEL: @PR22704(
+; CHECK:       %bool.tr = phi i1 [ %bool, %entry ], [ true, %t ], [ true, %f ]
+; CHECK:       br i1 %bool.tr, label %t, label %f
+}