maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Revert "[ScalarEvolution] Fix overflow in computeBECount." 

This reverts commit 5b350183cd (and
also "[NFC][ScalarEvolution] Cleanup howManyLessThans.",
009436e9c1, to make it apply).

See https://reviews.llvm.org/D105216 for discussion on various
miscompilations caused by that commit.
This commit is contained in:
Martin Storsjö 2021-07-09 14:02:51 +03:00
parent 52aeacfbf5
commit e479777d3c
8 changed files with 50 additions and 248 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
llvm/include/llvm/Analysis/ScalarEvolution.h
View File

@ -1791,7 +1791,7 @@ private:
/// less-than comparison will execute. If not computable, return
/// CouldNotCompute.
///
/// \p Pred specifies the kind of less-than comparison.
/// \p isSigned specifies whether the less-than is signed.
///
/// \p ControlsExit is true when the LHS < RHS condition directly controls
/// the branch (loops exits only if condition is true). In this case, we can
@ -1800,12 +1800,12 @@ private:
/// If \p AllowPredicates is set, this call will try to use a minimal set of
/// SCEV predicates in order to return an exact answer.
ExitLimit howManyLessThans(const SCEV *LHS, const SCEV *RHS, const Loop *L,
ICmpInst::Predicate Pred, bool ControlsExit,
bool AllowPredicates);
bool isSigned, bool ControlsExit,
bool AllowPredicates = false);
ExitLimit howManyGreaterThans(const SCEV *LHS, const SCEV *RHS, const Loop *L,
bool isSigned, bool IsSubExpr,
bool AllowPredicates);
bool AllowPredicates = false);
/// Return a predecessor of BB (which may not be an immediate predecessor)
/// which has exactly one successor from which BB is reachable, or null if
@ -2023,38 +2023,11 @@ private:
createAddRecFromPHIWithCastsImpl(const SCEVUnknown *SymbolicPHI);
/// Compute the backedge taken count knowing the interval difference, and
/// the stride for an inequality.
///
/// Caller must ensure that non-negative N exists such that
/// (Start + Stride * N) >= End, and that computing "(Start + Stride * N)"
/// doesn't overflow. In other words:
/// 1. If IsSigned is true, Start <=s End. Otherwise, Start <=u End.
/// 2. If End is not equal to start and IsSigned is true, Stride >s 0. If
/// End is not equal to start and IsSigned is false, Stride >u 0.
/// 3. The index variable doesn't overflow.
///
/// If the preconditions hold, the backedge taken count is N.
///
/// IsSigned determines whether End, Start, and Stride are treated as
/// signed values, for the purpose of optimizing the form of the result.
///
/// This function tries to use an optimized form:
/// ((End - Start) + (Stride - 1)) /u Stride
///
/// If it can't prove the addition doesn't overflow in that form, it uses
/// getUDivCeilSCEV.
const SCEV *computeBECount(bool IsSigned, const SCEV *Start, const SCEV *End,
const SCEV *Stride);
/// Compute ceil(N / D). N and D are treated as unsigned values.
///
/// Since SCEV doesn't have native ceiling division, this generates a
/// SCEV expression of the following form:
///
/// umin(N, 1) + floor((N - umin(N, 1)) / D)
///
/// A denominator of zero or poison is handled the same way as getUDivExpr().
const SCEV *getUDivCeilSCEV(const SCEV *N, const SCEV *D);
/// the stride for an inequality. Result takes the form:
/// (Delta + (Stride - 1)) udiv Stride.
/// Caller must ensure that this expression either does not overflow or
/// that the result is undefined if it does.
const SCEV *computeBECount(const SCEV *Delta, const SCEV *Stride);
/// Compute the maximum backedge count based on the range of values
/// permitted by Start, End, and Stride. This is for loops of the form

144
llvm/lib/Analysis/ScalarEvolution.cpp
View File

@ -8091,8 +8091,9 @@ ScalarEvolution::computeExitLimitFromICmp(const Loop *L,
}
case ICmpInst::ICMP_SLT:
case ICmpInst::ICMP_ULT: { // while (X < Y)
ExitLimit EL =
howManyLessThans(LHS, RHS, L, Pred, ControlsExit, AllowPredicates);
bool IsSigned = Pred == ICmpInst::ICMP_SLT;
ExitLimit EL = howManyLessThans(LHS, RHS, L, IsSigned, ControlsExit,
AllowPredicates);
if (EL.hasAnyInfo()) return EL;
break;
}
@ -11496,108 +11497,11 @@ bool ScalarEvolution::canIVOverflowOnGT(const SCEV *RHS, const SCEV *Stride,
return (std::move(MinValue) + MaxStrideMinusOne).ugt(MinRHS);
}
const SCEV *ScalarEvolution::computeBECount(bool IsSigned, const SCEV *Start,
const SCEV *End,
const SCEV *Stride) {
// The basic formula here is ceil((End - Start) / Stride). Since SCEV
// doesn't natively have division that rounds up, we need to convert to
// floor division.
//
// MayOverflow is whether adding (End - Start) + (Stride - 1)
// can overflow if Stride is positive. It's a precondition of the
// function that "End - Start" doesn't overflow. We handle the case where
// Stride isn't positive later.
//
// In practice, the arithmetic almost never overflows, but we have to prove
// it. We have a variety of ways to come up with a proof.
const SCEV *One = getOne(Stride->getType());
bool MayOverflow = [&] {
if (auto *StrideC = dyn_cast<SCEVConstant>(Stride)) {
if (StrideC->getAPInt().isPowerOf2()) {
// Suppose Stride is a power of two, and Start/End are unsigned
// integers. Let UMAX be the largest representable unsigned
// integer.
//
// By the preconditions of this function (see comment in header), we
// know "(Start + Stride * N)" >= End, and this doesn't overflow.
// As a formula:
//
// End <= (Start + Stride * N) <= UMAX
//
// Subtracting Start from all the terms:
//
// End - Start <= Stride * N <= UMAX - Start
//
// Since Start is unsigned, UMAX - Start <= UMAX. Therefore:
//
// End - Start <= Stride * N <= UMAX
//
// Stride * N is a multiple of Stride. Therefore,
//
// End - Start <= Stride * N <= UMAX - (UMAX mod Stride)
//
// Since Stride is a power of two, UMAX + 1 is divisible by Stride.
// Therefore, UMAX mod Stride == Stride - 1. So we can write:
//
// End - Start <= Stride * N <= UMAX - Stride - 1
//
// Dropping the middle term:
//
// End - Start <= UMAX - Stride - 1
//
// Adding Stride - 1 to both sides:
//
// (End - Start) + (Stride - 1) <= UMAX
//
// In other words, the addition doesn't have unsigned overflow.
//
// A similar proof works if we treat Start/End as signed values.
// Just rewrite steps before "End - Start <= Stride * N <= UMAX" to
// use signed max instead of unsigned max. Note that we're trying
// to prove a lack of unsigned overflow in either case.
return false;
}
}
if (Start == Stride || Start == getMinusSCEV(Stride, One)) {
// If Start is equal to Stride, (End - Start) + (Stride - 1) == End - 1.
// If !IsSigned, 0 <u Stride == Start <=u End; so 0 <u End - 1 <u End.
// If IsSigned, 0 <s Stride == Start <=s End; so 0 <s End - 1 <s End.
//
// If Start is equal to Stride - 1, (End - Start) + Stride - 1 == End.
return false;
}
if (IsSigned && isKnownNonNegative(Start)) {
// IsSigned implies "Start <=s End <=s INT_MAX".
// "isKnownNonNegative(Start)" implies "Start >=s 0".
// Therefore, "0 <=s End - Start <=s INT_MAX - Start <= INT_MAX".
// IsSigned also implies "0 <=s Stride - 1 <s INT_MAX". Therefore,
// "(End - Start) + (Stride - 1) <u INT_MAX * 2 <u UINT_MAX".
return false;
}
return true;
}();
// Force the stride to at least one, so we don't divide by zero. The stride
// can be zero if Delta is zero. We don't actually care what value we use
// for Stride in this case, as long as it isn't zero.
Stride = getUMaxExpr(Stride, One);
const SCEV *Delta = getMinusSCEV(End, Start);
if (!MayOverflow) {
// floor((D + (S - 1)) / S)
// We prefer this formulation if it's legal because it's fewer operations.
return getUDivExpr(getAddExpr(Delta, getMinusSCEV(Stride, One)), Stride);
}
return getUDivCeilSCEV(Delta, Stride);
}
const SCEV *ScalarEvolution::getUDivCeilSCEV(const SCEV *N, const SCEV *D) {
// umin(N, 1) + floor((N - umin(N, 1)) / D)
// This is equivalent to "1 + floor((N - 1) / D)" for N != 0. The umin
// expression fixes the case of N=0.
const SCEV *MinNOne = getUMinExpr(N, getOne(N->getType()));
const SCEV *NMinusOne = getMinusSCEV(N, MinNOne);
return getAddExpr(MinNOne, getUDivExpr(NMinusOne, D));
const SCEV *ScalarEvolution::computeBECount(const SCEV *Delta,
const SCEV *Step) {
const SCEV *One = getOne(Step->getType());
Delta = getAddExpr(Delta, getMinusSCEV(Step, One));
return getUDivExpr(Delta, Step);
}
const SCEV *ScalarEvolution::computeMaxBECountForLT(const SCEV *Start,
@ -11636,21 +11540,18 @@ const SCEV *ScalarEvolution::computeMaxBECountForLT(const SCEV *Start,
APInt MaxEnd = IsSigned ? APIntOps::smin(getSignedRangeMax(End), Limit)
: APIntOps::umin(getUnsignedRangeMax(End), Limit);
MaxBECount = getUDivCeilSCEV(getConstant(MaxEnd - MinStart) /* Delta */,
getConstant(StrideForMaxBECount) /* Step */);
MaxBECount = computeBECount(getConstant(MaxEnd - MinStart) /* Delta */,
getConstant(StrideForMaxBECount) /* Step */);
return MaxBECount;
}
ScalarEvolution::ExitLimit
ScalarEvolution::howManyLessThans(const SCEV *LHS, const SCEV *RHS,
const Loop *L, ICmpInst::Predicate Pred,
const Loop *L, bool IsSigned,
bool ControlsExit, bool AllowPredicates) {
SmallPtrSet<const SCEVPredicate *, 4> Predicates;
assert(ICmpInst::isLT(Pred) && "Unexpected pred");
bool IsSigned = ICmpInst::isSigned(Pred);
const SCEVAddRecExpr *IV = dyn_cast<SCEVAddRecExpr>(LHS);
bool PredicatedIV = false;
@ -11668,6 +11569,7 @@ ScalarEvolution::howManyLessThans(const SCEV *LHS, const SCEV *RHS,
auto WrapType = IsSigned ? SCEV::FlagNSW : SCEV::FlagNUW;
bool NoWrap = ControlsExit && IV->getNoWrapFlags(WrapType);
ICmpInst::Predicate Cond = IsSigned ? ICmpInst::ICMP_SLT : ICmpInst::ICMP_ULT;
const SCEV *Stride = IV->getStepRecurrence(*this);
@ -11780,6 +11682,7 @@ ScalarEvolution::howManyLessThans(const SCEV *LHS, const SCEV *RHS,
return RHS;
}
const SCEV *End = RHS;
// When the RHS is not invariant, we do not know the end bound of the loop and
// cannot calculate the ExactBECount needed by ExitLimit. However, we can
// calculate the MaxBECount, given the start, stride and max value for the end
@ -11796,7 +11699,7 @@ ScalarEvolution::howManyLessThans(const SCEV *LHS, const SCEV *RHS,
// is the LHS value of the less-than comparison the first time it is evaluated
// and End is the RHS.
const SCEV *BECountIfBackedgeTaken =
computeBECount(IsSigned, Start, RHS, Stride);
computeBECount(getMinusSCEV(End, Start), Stride);
// If the loop entry is guarded by the result of the backedge test of the
// first loop iteration, then we know the backedge will be taken at least
// once and so the backedge taken count is as above. If not then we use the
@ -11805,19 +11708,17 @@ ScalarEvolution::howManyLessThans(const SCEV *LHS, const SCEV *RHS,
// result is as above, and if not max(End,Start) is Start so we get a backedge
// count of zero.
const SCEV *BECount;
if (isLoopEntryGuardedByCond(L, Pred, getMinusSCEV(OrigStart, Stride),
OrigRHS))
if (isLoopEntryGuardedByCond(L, Cond, getMinusSCEV(OrigStart, Stride), OrigRHS))
BECount = BECountIfBackedgeTaken;
else {
const SCEV *End;
// If we know that RHS >= Start in the context of loop, then we know that
// max(RHS, Start) = RHS at this point.
if (isLoopEntryGuardedByCond(L, ICmpInst::getInversePredicate(Pred),
OrigRHS, OrigStart))
if (isLoopEntryGuardedByCond(
L, IsSigned ? ICmpInst::ICMP_SGE : ICmpInst::ICMP_UGE, OrigRHS, OrigStart))
End = RHS;
else
End = IsSigned ? getSMaxExpr(RHS, Start) : getUMaxExpr(RHS, Start);
BECount = computeBECount(IsSigned, Start, End, Stride);
BECount = computeBECount(getMinusSCEV(End, Start), Stride);
}
const SCEV *MaxBECount;
@ -11903,7 +11804,7 @@ ScalarEvolution::howManyGreaterThans(const SCEV *LHS, const SCEV *RHS,
return End;
}
const SCEV *BECount = computeBECount(IsSigned, End, Start, Stride);
const SCEV *BECount = computeBECount(getMinusSCEV(Start, End), Stride);
APInt MaxStart = IsSigned ? getSignedRangeMax(Start)
: getUnsignedRangeMax(Start);
@ -11924,8 +11825,11 @@ ScalarEvolution::howManyGreaterThans(const SCEV *LHS, const SCEV *RHS,
const SCEV *MaxBECount = isa<SCEVConstant>(BECount)
? BECount
: getUDivCeilSCEV(getConstant(MaxStart - MinEnd),
getConstant(MinStride));
: computeBECount(getConstant(MaxStart - MinEnd),
getConstant(MinStride));
if (isa<SCEVCouldNotCompute>(MaxBECount))
MaxBECount = BECount;
return ExitLimit(BECount, MaxBECount, false, Predicates);
}

7
llvm/test/Analysis/ScalarEvolution/2008-11-18-Stride1.ll
View File

@ -1,9 +1,14 @@
; RUN: opt < %s -analyze -enable-new-pm=0 -scalar-evolution | FileCheck %s
; RUN: opt < %s -disable-output "-passes=print<scalar-evolution>" 2>&1 | FileCheck %s
; CHECK: Loop %bb: backedge-taken count is (((-7 + (-1 * (1 umin (-7 + %x)))<nuw><nsw> + %x) /u 3) + (1 umin (-7 + %x)))
; CHECK: Loop %bb: backedge-taken count is ((-5 + %x) /u 3)
; CHECK: Loop %bb: max backedge-taken count is 1431655764
; ScalarEvolution can't compute a trip count because it doesn't know if
; dividing by the stride will have a remainder. This could theoretically
; be teaching it how to use a more elaborate trip count computation.
define i32 @f(i32 %x) nounwind readnone {
entry:
%0 = icmp ugt i32 %x, 4 ; <i1> [#uses=1]

6
llvm/test/Analysis/ScalarEvolution/2008-11-18-Stride2.ll
View File

@ -1,10 +1,12 @@
; RUN: opt < %s -analyze -enable-new-pm=0 -scalar-evolution 2>&1 | FileCheck %s
; RUN: opt < %s -disable-output "-passes=print<scalar-evolution>" 2>&1 2>&1 | FileCheck %s
; CHECK: Loop %bb: backedge-taken count is (((997 + (-1 * (1 umin (997 + (-1 * %x))))<nuw><nsw> + (-1 * %x)) /u 3) + (1 umin (997 + (-1 * %x))))
; CHECK: Loop %bb: backedge-taken count is ((999 + (-1 * %x)) /u 3)
; CHECK: Loop %bb: max backedge-taken count is 334
; This is a tricky testcase for unsigned wrap detection.
; This is a tricky testcase for unsigned wrap detection which ScalarEvolution
; doesn't yet know how to do.
define i32 @f(i32 %x) nounwind readnone {
entry:

14
llvm/test/Analysis/ScalarEvolution/lt-overflow.ll
View File

@ -27,8 +27,6 @@ target triple = "x86_64-unknown-linux-gnu"
; CHECK: Loop %for.body: Unpredictable backedge-taken count.
; CHECK: Determining loop execution counts for: @test_other_exit
; CHECK: Loop %for.body: <multiple exits> Unpredictable backedge-taken count.
; CHECK: Determining loop execution counts for: @test_gt
; CHECK: Loop %for.body: Unpredictable backedge-taken count.
define void @test(i32 %N) mustprogress {
entry:
@ -186,16 +184,4 @@ for.cond.cleanup:
ret void
}
define void @test_gt(i32 %S, i32 %N) mustprogress {
entry:
br label %for.body
for.body:
%iv = phi i32 [ %iv.next, %for.body ], [ %S, %entry ]
%iv.next = add i32 %iv, -2
%cmp = icmp ugt i32 %iv.next, %N
br i1 %cmp, label %for.body, label %for.cond.cleanup
for.cond.cleanup:
ret void
}

23
llvm/test/Analysis/ScalarEvolution/max-trip-count.ll
View File

@ -432,26 +432,3 @@ loop:
loop.exit:
ret void
}
; sgt with negative stride
define void @changing_end_bound7(i32 %start, i32* %n_addr, i32* %addr) {
; CHECK-LABEL: Determining loop execution counts for: @changing_end_bound7
; CHECK: Loop %loop: Unpredictable backedge-taken count.
; CHECK: Loop %loop: Unpredictable max backedge-taken count.
entry:
br label %loop
loop:
%iv = phi i32 [ %start, %entry ], [ %iv.next, %loop ]
%acc = phi i32 [ 0, %entry ], [ %acc.next, %loop ]
%val = load atomic i32, i32* %addr unordered, align 4
fence acquire
%acc.next = add i32 %acc, %val
%iv.next = add i32 %iv, -1
%n = load atomic i32, i32* %n_addr unordered, align 4
%cmp = icmp sgt i32 %iv.next, %n
br i1 %cmp, label %loop, label %loop.exit
loop.exit:
ret void
}

40
llvm/test/Analysis/ScalarEvolution/trip-count-unknown-stride.ll
View File

@ -4,8 +4,8 @@
; ScalarEvolution should be able to compute trip count of the loop by proving
; that this is not an infinite loop with side effects.
; CHECK-LABEL: Determining loop execution counts for: @foo1
; CHECK: backedge-taken count is ((-1 + (-1 * %s) + (1 umax %s) + %n) /u (1 umax %s))
; CHECK: Determining loop execution counts for: @foo1
; CHECK: backedge-taken count is ((-1 + %n) /u %s)
; We should have a conservative estimate for the max backedge taken count for
; loops with unknown stride.
@ -34,8 +34,8 @@ for.end: ; preds = %for.body, %entry
; Check that we are able to compute trip count of a loop without an entry guard.
; CHECK-LABEL: Determining loop execution counts for: @foo2
; CHECK: backedge-taken count is ((-1 + (-1 * %s) + (1 umax %s) + (%n smax %s)) /u (1 umax %s))
; CHECK: Determining loop execution counts for: @foo2
; CHECK: backedge-taken count is ((-1 + (%n smax %s)) /u %s)
; We should have a conservative estimate for the max backedge taken count for
; loops with unknown stride.
@ -61,7 +61,7 @@ for.end: ; preds = %for.body, %entry
; Check that without mustprogress we don't make assumptions about infinite
; loops being UB.
; CHECK-LABEL: Determining loop execution counts for: @foo3
; CHECK: Determining loop execution counts for: @foo3
; CHECK: Loop %for.body: Unpredictable backedge-taken count.
; CHECK: Loop %for.body: Unpredictable max backedge-taken count.
@ -84,8 +84,8 @@ for.end: ; preds = %for.body, %entry
}
; Same as foo2, but with mustprogress on loop, not function
; CHECK-LABEL: Determining loop execution counts for: @foo4
; CHECK: backedge-taken count is ((-1 + (-1 * %s) + (1 umax %s) + (%n smax %s)) /u (1 umax %s))
; CHECK: Determining loop execution counts for: @foo4
; CHECK: backedge-taken count is ((-1 + (%n smax %s)) /u %s)
; CHECK: max backedge-taken count is -1
define void @foo4(i32* nocapture %A, i32 %n, i32 %s) {
@ -106,31 +106,5 @@ for.end: ; preds = %for.body, %entry
ret void
}
; A more complex case with pre-increment compare instead of post-increment.
; CHECK-LABEL: Determining loop execution counts for: @foo5
; CHECK: Loop %for.body: backedge-taken count is ((((-1 * (1 umin ((-1 * %start) + (%n smax %start))))<nuw><nsw> + (-1 * %start) + (%n smax %start)) /u (1 umax %s)) + (1 umin ((-1 * %start) + (%n smax %start))))
; We should have a conservative estimate for the max backedge taken count for
; loops with unknown stride.
; CHECK: max backedge-taken count is -1
define void @foo5(i32* nocapture %A, i32 %n, i32 %s, i32 %start) mustprogress {
entry:
br label %for.body
for.body: ; preds = %entry, %for.body
%i.05 = phi i32 [ %add, %for.body ], [ %start, %entry ]
%arrayidx = getelementptr inbounds i32, i32* %A, i32 %i.05
%0 = load i32, i32* %arrayidx, align 4
%inc = add nsw i32 %0, 1
store i32 %inc, i32* %arrayidx, align 4
%add = add nsw i32 %i.05, %s
%cmp = icmp slt i32 %i.05, %n
br i1 %cmp, label %for.body, label %for.end
for.end: ; preds = %for.body, %entry
ret void
}
!8 = distinct !{!8, !9}
!9 = !{!"llvm.loop.mustprogress"}

19
llvm/test/Analysis/ScalarEvolution/trip-count13.ll
View File

@ -80,22 +80,3 @@ loop:
leave:
ret void
}
define void @s_2(i8 %start) {
entry:
%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 sgt i8 %iv, %rhs
br i1 %iv.cmp, label %loop, label %leave
; CHECK-LABEL: Determining loop execution counts for: @s_2
; CHECK-NEXT: Loop %loop: backedge-taken count is ((-1 * ((-100 + %start) smin %start)) + %start)
; CHECK-NEXT: Loop %loop: max backedge-taken count is -1
leave:
ret void
}