From 8499ebf2e980ec96aa244838df5bc7602477d560 Mon Sep 17 00:00:00 2001
From: Sanjoy Das <sanjoy@playingwithpointers.com>
Date: Wed, 25 Oct 2017 21:41:00 +0000
Subject: [PATCH] [SCEV] Fix an assertion failure in the max backedge taken
 count

Max backedge taken count is always expected to be a constant; and this is
usually true by construction -- it is a SCEV expression with constant inputs.
However, if the max backedge expression ends up being computed to be a udiv with
a constant zero denominator[0], SCEV does not fold the result to a constant
since there is no constant it can fold it to (SCEV has no representation for
"infinity" or "undef").

However, in computeMaxBECountForLT we already know the denominator is positive,
and thus at least 1; and we can use this fact to avoid dividing by zero.

[0]: We can end up with a constant zero denominator if the signed range of the
stride is more precise than the unsigned range.

llvm-svn: 316615
---
 llvm/lib/Analysis/ScalarEvolution.cpp         | 19 +++++++-------
 .../max-be-count-not-constant.ll              | 26 +++++++++++++++++++
 2 files changed, 35 insertions(+), 10 deletions(-)
 create mode 100644 llvm/test/Analysis/ScalarEvolution/max-be-count-not-constant.ll

diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index f6c45de9ed62..d48e8a57562c 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -9703,17 +9703,16 @@ const SCEV *ScalarEvolution::computeMaxBECountForLT(const SCEV *Start,
   APInt MinStart =
       IsSigned ? getSignedRangeMin(Start) : getUnsignedRangeMin(Start);
 
-  APInt StrideForMaxBECount;
+  APInt StrideForMaxBECount =
+      IsSigned ? getSignedRangeMin(Stride) : getUnsignedRangeMin(Stride);
 
-  bool PositiveStride = isKnownPositive(Stride);
-  if (PositiveStride)
-    StrideForMaxBECount =
-        IsSigned ? getSignedRangeMin(Stride) : getUnsignedRangeMin(Stride);
-  else
-    // Using a stride of 1 is safe when computing max backedge taken count for a
-    // loop with unknown stride, since the precondition for this function is
-    // that it is positive.
-    StrideForMaxBECount = APInt(BitWidth, 1, IsSigned);
+  // We already know that the stride is positive, so we paper over conservatism
+  // in our range computation by forcing StrideForMaxBECount to be at least one.
+  // In theory this is unnecessary, but we expect MaxBECount to be a
+  // SCEVConstant, and (udiv <constant> 0) is not constant folded by SCEV (there
+  // is nothing to constant fold it to).
+  APInt One(BitWidth, 1, IsSigned);
+  StrideForMaxBECount = APIntOps::smax(One, StrideForMaxBECount);
 
   APInt MaxValue = IsSigned ? APInt::getSignedMaxValue(BitWidth)
                             : APInt::getMaxValue(BitWidth);
diff --git a/llvm/test/Analysis/ScalarEvolution/max-be-count-not-constant.ll b/llvm/test/Analysis/ScalarEvolution/max-be-count-not-constant.ll
new file mode 100644
index 000000000000..b593fc269a7b
--- /dev/null
+++ b/llvm/test/Analysis/ScalarEvolution/max-be-count-not-constant.ll
@@ -0,0 +1,26 @@
+; RUN: opt < %s -analyze -scalar-evolution | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Previously in this case the max backedge count would be computed as 1/0, which
+; is correct but undesirable.  It would also not fold as a constant, tripping
+; asserts in SCEV.
+
+define void @pluto(i32 %arg) {
+; CHECK-LABEL: Classifying expressions for: @pluto
+; CHECK: Loop %bb2: max backedge-taken count is 2
+bb:
+  %tmp = ashr i32 %arg, 31
+  %tmp1 = add nsw i32 %tmp, 2
+  br label %bb2
+
+bb2:                                              ; preds = %bb2, %bb
+  %tmp3 = phi i32 [ 0, %bb ], [ %tmp4, %bb2 ]
+  %tmp4 = add nuw nsw i32 %tmp1, %tmp3
+  %tmp5 = icmp ult i32 %tmp4, 2
+  br i1 %tmp5, label %bb2, label %bb6
+
+bb6:                                              ; preds = %bb2
+  ret void
+}