From: Sanjoy Das Date: Sat, 18 Jun 2016 04:38:31 +0000 (+0000) Subject: [SCEV] Fix incorrect trip count computation X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bac5341e94334904dc93745fe627c5b288a7a898;p=llvm [SCEV] Fix incorrect trip count computation The way we elide max expressions when computing trip counts is incorrect -- it breaks cases like this: ``` static int wrapping_add(int a, int b) { return (int)((unsigned)a + (unsigned)b); } void test() { volatile int end_buf = 2147483548; // INT_MIN - 100 int end = end_buf; unsigned counter = 0; for (int start = wrapping_add(end, 200); start < end; start++) counter++; print(counter); } ``` Note: the `NoWrap` variable that was being tested has little to do with the values flowing into the max expression; it is a property of the induction variable. test/Transforms/LoopUnroll/nsw-tripcount.ll was added to solely test functionality I'm reverting in this change, so I've deleted the test fully. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@273079 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/ScalarEvolution.cpp b/lib/Analysis/ScalarEvolution.cpp index 250903100a0..c2e1825c027 100644 --- a/lib/Analysis/ScalarEvolution.cpp +++ b/lib/Analysis/ScalarEvolution.cpp @@ -8668,18 +8668,8 @@ ScalarEvolution::howManyLessThans(const SCEV *LHS, const SCEV *RHS, : ICmpInst::ICMP_ULT; const SCEV *Start = IV->getStart(); const SCEV *End = RHS; - if (!isLoopEntryGuardedByCond(L, Cond, getMinusSCEV(Start, Stride), RHS)) { - const SCEV *Diff = getMinusSCEV(RHS, Start); - // If we have NoWrap set, then we can assume that the increment won't - // overflow, in which case if RHS - Start is a constant, we don't need to - // do a max operation since we can just figure it out statically - if (NoWrap && isa(Diff)) { - if (cast(Diff)->getAPInt().isNegative()) - End = Start; - } else - End = IsSigned ? getSMaxExpr(RHS, Start) - : getUMaxExpr(RHS, Start); - } + if (!isLoopEntryGuardedByCond(L, Cond, getMinusSCEV(Start, Stride), RHS)) + End = IsSigned ? getSMaxExpr(RHS, Start) : getUMaxExpr(RHS, Start); const SCEV *BECount = computeBECount(getMinusSCEV(End, Start), Stride, false); @@ -8754,18 +8744,8 @@ ScalarEvolution::howManyGreaterThans(const SCEV *LHS, const SCEV *RHS, const SCEV *Start = IV->getStart(); const SCEV *End = RHS; - if (!isLoopEntryGuardedByCond(L, Cond, getAddExpr(Start, Stride), RHS)) { - const SCEV *Diff = getMinusSCEV(RHS, Start); - // If we have NoWrap set, then we can assume that the increment won't - // overflow, in which case if RHS - Start is a constant, we don't need to - // do a max operation since we can just figure it out statically - if (NoWrap && isa(Diff)) { - if (!cast(Diff)->getAPInt().isNegative()) - End = Start; - } else - End = IsSigned ? getSMinExpr(RHS, Start) - : getUMinExpr(RHS, Start); - } + if (!isLoopEntryGuardedByCond(L, Cond, getAddExpr(Start, Stride), RHS)) + End = IsSigned ? getSMinExpr(RHS, Start) : getUMinExpr(RHS, Start); const SCEV *BECount = computeBECount(getMinusSCEV(Start, End), Stride, false); diff --git a/test/Analysis/ScalarEvolution/nsw.ll b/test/Analysis/ScalarEvolution/nsw.ll index 49050c53d06..a3752919d33 100644 --- a/test/Analysis/ScalarEvolution/nsw.ll +++ b/test/Analysis/ScalarEvolution/nsw.ll @@ -126,7 +126,7 @@ exit: } ; CHECK-LABEL: PR12375 -; CHECK: --> {(4 + %arg),+,4}<%bb1>{{ U: [^ ]+ S: [^ ]+}}{{ *}}Exits: (8 + %arg) +; CHECK: --> {(4 + %arg),+,4}<%bb1>{{ U: [^ ]+ S: [^ ]+}}{{ *}}Exits: (4 + (4 * ((-1 + (-1 * %arg) + ((4 + %arg) umax (8 + %arg))) /u 4)) + %arg) define i32 @PR12375(i32* readnone %arg) { bb: %tmp = getelementptr inbounds i32, i32* %arg, i64 2 @@ -163,7 +163,7 @@ bb5: ; preds = %bb2 declare void @f(i32) ; CHECK-LABEL: nswnowrap -; CHECK: --> {(1 + %v),+,1}<%for.body>{{ U: [^ ]+ S: [^ ]+}}{{ *}}Exits: (2 + %v) +; CHECK: --> {(1 + %v),+,1}<%for.body>{{ U: [^ ]+ S: [^ ]+}}{{ *}}Exits: (1 + ((1 + %v) smax %v)) define void @nswnowrap(i32 %v, i32* %buf) { entry: %add = add nsw i32 %v, 1 diff --git a/test/Analysis/ScalarEvolution/trip-count13.ll b/test/Analysis/ScalarEvolution/trip-count13.ll new file mode 100644 index 00000000000..37ef2fd500a --- /dev/null +++ b/test/Analysis/ScalarEvolution/trip-count13.ll @@ -0,0 +1,81 @@ +; RUN: opt -S -analyze -scalar-evolution < %s | FileCheck %s + +define void @u_0(i8 %rhs) { +; E.g.: %rhs = 255, %start = 99, backedge taken 156 times +entry: + %start = add i8 %rhs, 100 + br label %loop + +loop: + %iv = phi i8 [ %start, %entry ], [ %iv.inc, %loop ] + %iv.inc = add nuw i8 %iv, 1 ;; Note: this never unsigned-wraps + %iv.cmp = icmp ult i8 %iv, %rhs + br i1 %iv.cmp, label %loop, label %leave + +; CHECK-LABEL: Determining loop execution counts for: @u_0 +; CHECK-NEXT: Loop %loop: backedge-taken count is (-100 + (-1 * %rhs) + ((100 + %rhs) umax %rhs)) +; CHECK-NEXT: Loop %loop: max backedge-taken count is -1 + +leave: + ret void +} + +define void @u_1(i8 %start) { +entry: +; E.g.: %start = 99, %rhs = 255, backedge taken 156 times + %rhs = add i8 %start, -100 + br label %loop + +loop: + %iv = phi i8 [ %start, %entry ], [ %iv.inc, %loop ] + %iv.inc = add nuw i8 %iv, 1 ;; Note: this never unsigned-wraps + %iv.cmp = icmp ult i8 %iv, %rhs + br i1 %iv.cmp, label %loop, label %leave + +; CHECK-LABEL: Determining loop execution counts for: @u_1 +; CHECK-NEXT: Loop %loop: backedge-taken count is ((-1 * %start) + ((-100 + %start) umax %start)) +; CHECK-NEXT: Loop %loop: max backedge-taken count is -1 + +leave: + ret void +} + +define void @s_0(i8 %rhs) { +entry: +; E.g.: %rhs = 127, %start = -29, backedge taken 156 times + %start = add i8 %rhs, 100 + br label %loop + +loop: + %iv = phi i8 [ %start, %entry ], [ %iv.inc, %loop ] + %iv.inc = add nsw i8 %iv, 1 ;; Note: this never signed-wraps + %iv.cmp = icmp slt i8 %iv, %rhs + br i1 %iv.cmp, label %loop, label %leave + +; CHECK-LABEL: Determining loop execution counts for: @s_0 +; CHECK-NEXT: Loop %loop: backedge-taken count is (-100 + (-1 * %rhs) + ((100 + %rhs) smax %rhs)) +; CHECK-NEXT: Loop %loop: max backedge-taken count is -1 + +leave: + ret void +} + +define void @s_1(i8 %start) { +entry: +; E.g.: start = -29, %rhs = 127, %backedge taken 156 times + %rhs = add i8 %start, -100 + br label %loop + +loop: + %iv = phi i8 [ %start, %entry ], [ %iv.inc, %loop ] + %iv.inc = add nsw i8 %iv, 1 + %iv.cmp = icmp slt i8 %iv, %rhs + br i1 %iv.cmp, label %loop, label %leave + +; CHECK-LABEL: Determining loop execution counts for: @s_1 +; CHECK-NEXT: Loop %loop: backedge-taken count is ((-1 * %start) + ((-100 + %start) smax %start)) +; CHECK-NEXT: Loop %loop: max backedge-taken count is -1 + +leave: + ret void +} diff --git a/test/Transforms/LoopUnroll/nsw-tripcount.ll b/test/Transforms/LoopUnroll/nsw-tripcount.ll deleted file mode 100644 index 98cab32a42a..00000000000 --- a/test/Transforms/LoopUnroll/nsw-tripcount.ll +++ /dev/null @@ -1,32 +0,0 @@ -; RUN: opt -loop-unroll -S %s | FileCheck %s - -; extern void f(int); -; void test1(int v) { -; for (int i=v; i<=v+1; ++i) -; f(i); -; } -; -; We can use the nsw information to see that the tripcount will be 2, so the -; loop should be unrolled as this is always beneficial - -declare void @f(i32) - -; CHECK-LABEL: @test1 -define void @test1(i32 %v) { -entry: - %add = add nsw i32 %v, 1 - br label %for.body - -for.body: - %i.04 = phi i32 [ %v, %entry ], [ %inc, %for.body ] - tail call void @f(i32 %i.04) - %inc = add nsw i32 %i.04, 1 - %cmp = icmp slt i32 %i.04, %add - br i1 %cmp, label %for.body, label %for.end - -; CHECK: call void @f -; CHECK-NOT: br i1 -; CHECK: call void @f -for.end: - ret void -}