Skip to content

Commit 3098435

Browse files
authored
[DA] Fix absolute value calculation (llvm#164967)
This patch fixes the computation of the absolute value for SCEV. Previously, it was calculated as `AbsX = SE->isKnownNonNegative(X) ? X : -X`, which would incorrectly assume that `!isKnownNonNegative` implies `isKnownNegative`. This assumption does not hold in general, for example, when `X` is a `SCEVUnknown` and it can be an arbitrary value. To compute the absolute value properly, we should use ScalarEvolution::getAbsExpr instead. Fix llvm#149977
1 parent a1ae900 commit 3098435

File tree

2 files changed

+87
-12
lines changed

2 files changed

+87
-12
lines changed

llvm/lib/Analysis/DependenceAnalysis.cpp

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1582,6 +1582,23 @@ static const SCEV *minusSCEVNoSignedOverflow(const SCEV *A, const SCEV *B,
15821582
return nullptr;
15831583
}
15841584

1585+
/// Returns the absolute value of \p A. In the context of dependence analysis,
1586+
/// we need an absolute value in a mathematical sense. If \p A is the signed
1587+
/// minimum value, we cannot represent it unless extending the original type.
1588+
/// Thus if we cannot prove that \p A is not the signed minimum value, returns
1589+
/// nullptr.
1590+
static const SCEV *absSCEVNoSignedOverflow(const SCEV *A, ScalarEvolution &SE) {
1591+
IntegerType *Ty = cast<IntegerType>(A->getType());
1592+
if (!Ty)
1593+
return nullptr;
1594+
1595+
const SCEV *SMin =
1596+
SE.getConstant(APInt::getSignedMinValue(Ty->getBitWidth()));
1597+
if (!SE.isKnownPredicate(CmpInst::ICMP_NE, A, SMin))
1598+
return nullptr;
1599+
return SE.getAbsExpr(A, /*IsNSW=*/true);
1600+
}
1601+
15851602
/// Returns true iff \p Test is enabled.
15861603
static bool isDependenceTestEnabled(DependenceTestType Test) {
15871604
if (EnableDependenceTest == DependenceTestType::All)
@@ -1669,21 +1686,25 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
16691686
LLVM_DEBUG(dbgs() << ", " << *Delta->getType() << "\n");
16701687

16711688
// check that |Delta| < iteration count
1672-
if (const SCEV *UpperBound =
1673-
collectUpperBound(CurSrcLoop, Delta->getType())) {
1689+
bool IsDeltaLarge = [&] {
1690+
const SCEV *UpperBound = collectUpperBound(CurSrcLoop, Delta->getType());
1691+
if (!UpperBound)
1692+
return false;
1693+
16741694
LLVM_DEBUG(dbgs() << "\t UpperBound = " << *UpperBound);
16751695
LLVM_DEBUG(dbgs() << ", " << *UpperBound->getType() << "\n");
1676-
const SCEV *AbsDelta =
1677-
SE->isKnownNonNegative(Delta) ? Delta : SE->getNegativeSCEV(Delta);
1678-
const SCEV *AbsCoeff =
1679-
SE->isKnownNonNegative(Coeff) ? Coeff : SE->getNegativeSCEV(Coeff);
1696+
const SCEV *AbsDelta = absSCEVNoSignedOverflow(Delta, *SE);
1697+
const SCEV *AbsCoeff = absSCEVNoSignedOverflow(Coeff, *SE);
1698+
if (!AbsDelta || !AbsCoeff)
1699+
return false;
16801700
const SCEV *Product = SE->getMulExpr(UpperBound, AbsCoeff);
1681-
if (isKnownPredicate(CmpInst::ICMP_SGT, AbsDelta, Product)) {
1682-
// Distance greater than trip count - no dependence
1683-
++StrongSIVindependence;
1684-
++StrongSIVsuccesses;
1685-
return true;
1686-
}
1701+
return isKnownPredicate(CmpInst::ICMP_SGT, AbsDelta, Product);
1702+
}();
1703+
if (IsDeltaLarge) {
1704+
// Distance greater than trip count - no dependence
1705+
++StrongSIVindependence;
1706+
++StrongSIVsuccesses;
1707+
return true;
16871708
}
16881709

16891710
// Can we compute distance?
@@ -2259,6 +2280,9 @@ bool DependenceInfo::weakZeroSrcSIVtest(
22592280
const SCEVConstant *ConstCoeff = dyn_cast<SCEVConstant>(DstCoeff);
22602281
if (!ConstCoeff)
22612282
return false;
2283+
2284+
// Since ConstCoeff is constant, !isKnownNegative means it's non-negative.
2285+
// TODO: Bail out if it's a signed minimum value.
22622286
const SCEV *AbsCoeff = SE->isKnownNegative(ConstCoeff)
22632287
? SE->getNegativeSCEV(ConstCoeff)
22642288
: ConstCoeff;
@@ -2369,6 +2393,9 @@ bool DependenceInfo::weakZeroDstSIVtest(
23692393
const SCEVConstant *ConstCoeff = dyn_cast<SCEVConstant>(SrcCoeff);
23702394
if (!ConstCoeff)
23712395
return false;
2396+
2397+
// Since ConstCoeff is constant, !isKnownNegative means it's non-negative.
2398+
// TODO: Bail out if it's a signed minimum value.
23722399
const SCEV *AbsCoeff = SE->isKnownNegative(ConstCoeff)
23732400
? SE->getNegativeSCEV(ConstCoeff)
23742401
: ConstCoeff;
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 6
2+
; RUN: opt < %s -disable-output "-passes=print<da>" 2>&1 | FileCheck %s
3+
4+
; for (i = 0; i < 3; i++) {
5+
; a[-k * i] = 1;
6+
; a[-k * i + (2 * k + 1)] = 2;
7+
; }
8+
;
9+
; When k = -1, dependency exists between the two stores. Accesses will be:
10+
;
11+
; - a[-k * i] : a[ 0], a[-1], a[-2]
12+
; - a[-k * i + (2 * k + 1)] : a[-1], a[-2], a[-3]
13+
;
14+
; We cannot determine the sign of `k` and `2*k + 1` at compile time,
15+
;
16+
define void @unknown_sign(ptr %a, i64 %k) {
17+
; CHECK-LABEL: 'unknown_sign'
18+
; CHECK-NEXT: Src: store i8 1, ptr %idx.0, align 1 --> Dst: store i8 1, ptr %idx.0, align 1
19+
; CHECK-NEXT: da analyze - none!
20+
; CHECK-NEXT: Src: store i8 1, ptr %idx.0, align 1 --> Dst: store i8 2, ptr %idx.1, align 1
21+
; CHECK-NEXT: da analyze - output [<>]!
22+
; CHECK-NEXT: Src: store i8 2, ptr %idx.1, align 1 --> Dst: store i8 2, ptr %idx.1, align 1
23+
; CHECK-NEXT: da analyze - none!
24+
;
25+
entry:
26+
%k.neg = sub nsw i64 0, %k
27+
%kk = mul nsw i64 %k, 2
28+
%subscript.1.init = add i64 1, %kk
29+
br label %loop
30+
31+
loop:
32+
%i = phi i64 [ 0, %entry ], [ %i.next, %loop ]
33+
%subscript.0 = phi i64 [ 0, %entry ], [ %subscript.0.next, %loop ]
34+
%subscript.1 = phi i64 [ %subscript.1.init, %entry ], [ %subscript.1.next, %loop ]
35+
%idx.0 = getelementptr i8, ptr %a, i64 %subscript.0
36+
%idx.1 = getelementptr i8, ptr %a, i64 %subscript.1
37+
store i8 1, ptr %idx.0
38+
store i8 2, ptr %idx.1
39+
%i.next = add i64 %i, 1
40+
%subscript.0.next = add nsw i64 %subscript.0, %k.neg
41+
%subscript.1.next = add nsw i64 %subscript.1, %k.neg
42+
%cond.exit = icmp eq i64 %i.next, 3
43+
br i1 %cond.exit, label %exit, label %loop
44+
45+
exit:
46+
ret void
47+
}
48+

0 commit comments

Comments
 (0)