[llvm] r258016 - [IndVars] Fix PR25576

[Sanjoy Das via llvm-commits]

Author: sanjoy
Date: Sun Jan 17 12:12:52 2016
New Revision: 258016

URL: http://llvm.org/viewvc/llvm-project?rev=258016&view=rev
Log:
[IndVars] Fix PR25576

`LCSSASafePhiForRAUW` as computed was incorrect -- in cases like
these (this exact example does not actually trigger the bug):

define i32 @f(i32 %n, i1* %c) {
entry:
  br label %outer.loop

outer.loop:
  br label %inner.loop

inner.loop:
  %iv = phi i32 [ 0, %outer.loop ], [ %iv.inc, %inner.loop ]
  %iv.inc = add nuw nsw i32 %iv, 1
  %tc = udiv i32 %n, 13
  %be.cond = icmp ult i32 %iv, %tc
  br i1 %be.cond, label %inner.loop, label %inner.exit

inner.exit:
  %iv.lcssa = phi i32 [ %iv, %inner.loop ]
  %outer.be.cond = load volatile i1, i1* %c
  br i1 %outer.be.cond, label %outer.loop, label %leave

leave:
  %iv.lcssa.lcssa = phi i32 [ %iv.lcssa, %inner.exit ]
  ret i32 %iv.lcssa.lcssa
}

`LCSSASafePhiForRAUW` is true for `%iv.lcssa` when re-rewriting the exit
value of `%iv` for `%inner.loop` to `%tc` (this can happen due to
`SCEVExpander::findExistingExpansion`), but the RAUW breaks LCSSA.

To fix this, instead of computing `SafePhi` with special logic, decide
the safety of RAUW directly via `replacementPreservesLCSSAForm`.

Added:
    llvm/trunk/test/Transforms/IndVarSimplify/pr25576.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp?rev=258016&r1=258015&r2=258016&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Sun Jan 17 12:12:52 2016
@@ -504,10 +504,9 @@ struct RewritePhi {
   unsigned Ith;  // Ith incoming value.
   Value *Val;    // Exit value after expansion.
   bool HighCost; // High Cost when expansion.
-  bool SafePhi;  // LCSSASafePhiForRAUW.
 
-  RewritePhi(PHINode *P, unsigned I, Value *V, bool H, bool S)
-      : PN(P), Ith(I), Val(V), HighCost(H), SafePhi(S) {}
+  RewritePhi(PHINode *P, unsigned I, Value *V, bool H)
+      : PN(P), Ith(I), Val(V), HighCost(H) {}
 };
 }
 
@@ -560,21 +559,6 @@ void IndVarSimplify::rewriteLoopExitValu
 
     unsigned NumPreds = PN->getNumIncomingValues();
 
-    // We would like to be able to RAUW single-incoming value PHI nodes. We
-    // have to be certain this is safe even when this is an LCSSA PHI node.
-    // While the computed exit value is no longer varying in *this* loop, the
-    // exit block may be an exit block for an outer containing loop as well,
-    // the exit value may be varying in the outer loop, and thus it may still
-    // require an LCSSA PHI node. The safe case is when this is
-    // single-predecessor PHI node (LCSSA) and the exit block containing it is
-    // part of the enclosing loop, or this is the outer most loop of the nest.
-    // In either case the exit value could (at most) be varying in the same
-    // loop body as the phi node itself. Thus if it is in turn used outside of
-    // an enclosing loop it will only be via a separate LCSSA node.
-    bool LCSSASafePhiForRAUW =
-        NumPreds == 1 &&
-        (!L->getParentLoop() || L->getParentLoop() == LI->getLoopFor(ExitBB));
-
     // Iterate over all of the PHI nodes.
     BasicBlock::iterator BBI = ExitBB->begin();
     while ((PN = dyn_cast<PHINode>(BBI++))) {
@@ -669,8 +653,7 @@ void IndVarSimplify::rewriteLoopExitValu
         }
 
         // Collect all the candidate PHINodes to be rewritten.
-        RewritePhiSet.emplace_back(PN, i, ExitVal, HighCost,
-                                   LCSSASafePhiForRAUW);
+        RewritePhiSet.emplace_back(PN, i, ExitVal, HighCost);
       }
     }
   }
@@ -699,9 +682,9 @@ void IndVarSimplify::rewriteLoopExitValu
     if (isInstructionTriviallyDead(Inst, TLI))
       DeadInsts.push_back(Inst);
 
-    // If we determined that this PHI is safe to replace even if an LCSSA
-    // PHI, do so.
-    if (Phi.SafePhi) {
+    // Replace PN with ExitVal if that is legal and does not break LCSSA.
+    if (PN->getNumIncomingValues() == 1 &&
+        LI->replacementPreservesLCSSAForm(PN, ExitVal)) {
       PN->replaceAllUsesWith(ExitVal);
       PN->eraseFromParent();
     }

Added: llvm/trunk/test/Transforms/IndVarSimplify/pr25576.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/pr25576.ll?rev=258016&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/IndVarSimplify/pr25576.ll (added)
+++ llvm/trunk/test/Transforms/IndVarSimplify/pr25576.ll Sun Jan 17 12:12:52 2016
@@ -0,0 +1,31 @@
+; RUN: opt -S -indvars < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @fn1() {
+; CHECK-LABEL: @fn1(
+entry:
+  br label %for.cond.loopexit
+
+for.cond.loopexit:                                ; preds = %for.inc7, %for.cond.loopexit, %entry
+  %c.1.lcssa = phi i32 [ %inc8, %for.inc7 ], [ 0, %for.cond.loopexit ], [ 0, %entry ]
+  br i1 undef, label %for.cond.loopexit, label %for.cond4.preheader
+
+for.cond4.preheader:                              ; preds = %for.inc7, %for.cond.loopexit
+  %c.17 = phi i32 [ %inc8, %for.inc7 ], [ 0, %for.cond.loopexit ]
+  br label %for.body6
+
+for.body6:                                        ; preds = %for.body6, %for.cond4.preheader
+  %inc14 = phi i32 [ 0, %for.cond4.preheader ], [ %inc, %for.body6 ]
+  %idxprom = zext i32 %inc14 to i64
+  %inc = add i32 %inc14, 1
+  %cmp5 = icmp ult i32 %inc, 2
+  br i1 %cmp5, label %for.body6, label %for.inc7
+
+for.inc7:                                         ; preds = %for.body6
+  %inc.lcssa = phi i32 [ %inc, %for.body6 ]
+  %inc8 = add i32 %c.17, 1
+  %cmp = icmp ult i32 %inc8, %inc.lcssa
+  br i1 %cmp, label %for.cond4.preheader, label %for.cond.loopexit
+}