[SCEV] Fix an assertion failure in the max backedge taken count
authorSanjoy Das <sanjoy@playingwithpointers.com>
Wed, 25 Oct 2017 21:41:00 +0000 (21:41 +0000)
committerSanjoy Das <sanjoy@playingwithpointers.com>
Wed, 25 Oct 2017 21:41:00 +0000 (21:41 +0000)
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
llvm/test/Analysis/ScalarEvolution/max-be-count-not-constant.ll [new file with mode: 0644]

index f6c45de9ed62ab93405664780ac531c8c56850bf..d48e8a57562cc0ade39a3cc849a36745be6cbdbb 100644 (file)
@@ -9703,17 +9703,16 @@ const SCEV *ScalarEvolution::computeMaxBECountForLT(const SCEV *Start,
   APInt MinStart =
       IsSigned ? getSignedRangeMin(Start) : getUnsignedRangeMin(Start);
 
-  APInt StrideForMaxBECount;
-
-  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);
+  APInt StrideForMaxBECount =
+      IsSigned ? getSignedRangeMin(Stride) : getUnsignedRangeMin(Stride);
+
+  // 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 (file)
index 0000000..b593fc2
--- /dev/null
@@ -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
+}