[PATCH] ScalarEvolution incorrectly assumes that the start of certain add recurrences don't overflow

[Sanjoy Das]

Hi atrick, majnemer,

I'm not super-clear on how the original code worked (but AFAICT scev does the wrong thing on the test case); so please consider this more of an RFC.

http://reviews.llvm.org/D7331

Files:
  lib/Analysis/ScalarEvolution.cpp
  test/Analysis/ScalarEvolution/bad-scev-prestart-nowrap.ll

Index: lib/Analysis/ScalarEvolution.cpp
===================================================================
--- lib/Analysis/ScalarEvolution.cpp
+++ lib/Analysis/ScalarEvolution.cpp
@@ -1356,18 +1356,20 @@
   if (DiffOps.size() == SA->getNumOperands())
     return nullptr;
 
-  // This is a postinc AR. Check for overflow on the preinc recurrence using the
-  // same three conditions that getSignExtendedExpr checks.
+  // This is a postinc AR. Rule out overflow on the preinc recurrence if
+  // possible.
 
-  // 1. NSW flags on the step increment.
   const SCEV *PreStart = SE->getAddExpr(DiffOps, SA->getNoWrapFlags());
   const SCEVAddRecExpr *PreAR = dyn_cast<SCEVAddRecExpr>(
     SE->getAddRecExpr(PreStart, Step, L, SCEV::FlagAnyWrap));
 
-  if (PreAR && PreAR->getNoWrapFlags(SCEV::FlagNSW))
-    return PreStart;
+  // ScalarEvolution may have concluded that PreAR does not overflow due to a
+  // pre-existing add recurrance that contains an `add nsw` at the IR level.
+  // There may be a path out of the loop without executing that `add nsw`, so we
+  // cannot assume the increment of PreStart to not overflow even if PreAR does
+  // not overflow.
 
-  // 2. Direct overflow check on the step operation's expression.
+  // 1. Direct overflow check on the step operation's expression.
   unsigned BitWidth = SE->getTypeSizeInBits(AR->getType());
   Type *WideTy = IntegerType::get(SE->getContext(), BitWidth * 2);
   const SCEV *OperandExtendedStart =
@@ -1382,7 +1384,7 @@
     return PreStart;
   }
 
-  // 3. Loop precondition.
+  // 2. Loop precondition.
   ICmpInst::Predicate Pred;
   const SCEV *OverflowLimit = getOverflowLimitForStep(Step, &Pred, SE);
 
Index: test/Analysis/ScalarEvolution/bad-scev-prestart-nowrap.ll
===================================================================
--- /dev/null
+++ test/Analysis/ScalarEvolution/bad-scev-prestart-nowrap.ll
@@ -0,0 +1,50 @@
+; RUN: opt -analyze -scalar-evolution < %s | FileCheck %s
+
+; An example run where SCEV(%postinc)->getStart() may overflow:
+;
+; %start = INT_SMAX
+; %low.limit = INT_SMIN
+; %high.limit = < not used >
+;
+; >> entry:
+;  %postinc.start = INT_SMIN
+;
+; >> loop:
+;  %idx = %start 
+;  %postinc = INT_SMIN
+;  %postinc.inc = INT_SMIN + 1
+;  %postinc.sext = sext(INT_SMIN) = i64 INT32_SMIN
+;  %break.early = INT_SMIN `slt` INT_SMIN = false
+;  br i1 false, ___,  %early.exit
+;
+; >> early.exit:
+;  ret i64 INT32_SMIN
+
+
+define i64 @foo(i32 %start, i32 %low.limit, i32 %high.limit) {
+; CHECK-LABEL: Classifying expressions for: @foo
+ entry:
+  %postinc.start = add i32 %start, 1
+  br label %loop
+
+ loop:
+  %idx = phi i32 [ %start, %entry ], [ %idx.inc, %continue ]
+  %postinc = phi i32 [ %postinc.start, %entry ], [ %postinc.inc, %continue ]
+  %postinc.inc = add nsw i32 %postinc, 1
+  %postinc.sext = sext i32 %postinc to i64
+; CHECK:  %postinc.sext = sext i32 %postinc to i64
+; CHECK-NEXT:  -->  {(sext i32 (1 + %start) to i64),+,1}<nsw><%loop>
+  %break.early = icmp slt i32 %postinc, %low.limit
+  br i1 %break.early, label %continue, label %early.exit
+
+ continue:
+  %idx.inc = add nsw i32 %idx, 1
+  %cmp = icmp slt i32 %idx.inc, %high.limit
+  br i1 %cmp, label %loop, label %exit
+
+ exit:
+  ret i64 0
+
+ early.exit:
+  ret i64 %postinc.sext
+}

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7331.19133.patch
Type: text/x-patch
Size: 3280 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150202/21d348f1/attachment.bin>