shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[DA] Fix the check between Subscript and Size after delinearization (#151326)

Browse Source 
Delinearization provides two values: the size of the array, and the
subscript of the access. DA checks their validity (`0 <= subscript <
size`), with some special handling. In particular, to ensure `subscript
< size`, calculate the maximum value of `subscript - size` and check if
it is negative. There was an issue in its process: when `subscript -
size` is expressed as an affine format like `init + step * i`, the value
in the last iteration (`start + step * (num_iterations - 1)`) was
assumed to be the maximum value. This assumption is incorrect in the
following cases:

- When `step` is negative
- When the AddRec wraps

This patch introduces extra checks to ensure the sign of `step` and
verify the existence of nsw/nuw flags.

Also, `isKnownNonNegative(S - smax(1, Size))` was used as a regular
check, which is incorrect when `Size` is negative. This patch also
replace it with `isKnownNonNegative(S - Size)`, although it's still
unclear whether using `isKnownNonNegative` is appropriate in the first
place.

Fix #150604
This commit is contained in:
Ryotaro Kasuga 2025-08-08 10:58:13 +09:00 committed by GitHub
parent 1458eb206f
commit 05dd957cda
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 114 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
llvm/lib/Analysis/DependenceAnalysis.cpp
View File

@ -1074,7 +1074,7 @@ bool DependenceInfo::isKnownPredicate(ICmpInst::Predicate Pred, const SCEV *X,
/// Compare to see if S is less than Size, using
///
/// isKnownNegative(S - max(Size, 1))
/// isKnownNegative(S - Size)
///
/// with some extra checking if S is an AddRec and we can prove less-than using
/// the loop bounds.
@ -1090,21 +1090,34 @@ bool DependenceInfo::isKnownLessThan(const SCEV *S, const SCEV *Size) const {
Size = SE->getTruncateOrZeroExtend(Size, MaxType);
// Special check for addrecs using BE taken count
const SCEV *Bound = SE->getMinusSCEV(S, Size);
if (const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(Bound)) {
if (AddRec->isAffine()) {
if (const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(S))
if (AddRec->isAffine() && AddRec->hasNoSignedWrap()) {
const SCEV *BECount = SE->getBackedgeTakenCount(AddRec->getLoop());
if (!isa<SCEVCouldNotCompute>(BECount)) {
const SCEV *Limit = AddRec->evaluateAtIteration(BECount, *SE);
if (SE->isKnownNegative(Limit))
return true;
}
const SCEV *Start = AddRec->getStart();
const SCEV *Step = AddRec->getStepRecurrence(*SE);
const SCEV *End = AddRec->evaluateAtIteration(BECount, *SE);
const SCEV *Diff0 = SE->getMinusSCEV(Start, Size);
const SCEV *Diff1 = SE->getMinusSCEV(End, Size);
// If the value of Step is non-negative and the AddRec is non-wrap, it
// reaches its maximum at the last iteration. So it's enouth to check
// whether End - Size is negative.
if (SE->isKnownNonNegative(Step) && SE->isKnownNegative(Diff1))
return true;
// If the value of Step is non-positive and the AddRec is non-wrap, the
// initial value is its maximum.
if (SE->isKnownNonPositive(Step) && SE->isKnownNegative(Diff0))
return true;
// Even if we don't know the sign of Step, either Start or End must be
// the maximum value of the AddRec since it is non-wrap.
if (SE->isKnownNegative(Diff0) && SE->isKnownNegative(Diff1))
return true;
}
}
// Check using normal isKnownNegative
const SCEV *LimitedBound =
SE->getMinusSCEV(S, SE->getSMaxExpr(Size, SE->getOne(Size->getType())));
const SCEV *LimitedBound = SE->getMinusSCEV(S, Size);
return SE->isKnownNegative(LimitedBound);
}

5
llvm/test/Analysis/DDG/basic-loopnest.ll
View File

@ -1,5 +1,8 @@
; RUN: opt < %s -disable-output "-passes=print<ddg>" 2>&1 | FileCheck %s
; XFAIL: *
; At the moment, DependenceAnalysis cannot infer `n` to be positive.
; CHECK-LABEL: 'DDG' for loop 'test1.for.cond1.preheader':
@ -378,4 +381,4 @@ for.inc12: ; preds = %for.body4, %test2.f
for.end14: ; preds = %for.inc12, %entry
ret void
}
}

4
llvm/test/Analysis/DependenceAnalysis/Coupled.ll
View File

@ -719,12 +719,14 @@ for.end: ; preds = %for.body
;; for(int j = 0; j < M; j+=1)
;; A[M*N + M*i + j] = 2;
; FIXME: Currently failing to infer %M being positive.
define void @couple_weakzerosiv(ptr noalias nocapture %A, i64 %N, i64 %M) {
; CHECK-LABEL: 'couple_weakzerosiv'
; CHECK-NEXT: Src: store i32 1, ptr %arrayidx.us, align 4 --> Dst: store i32 1, ptr %arrayidx.us, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i32 1, ptr %arrayidx.us, align 4 --> Dst: store i32 2, ptr %arrayidx9.us, align 4
; CHECK-NEXT: da analyze - output [p>]!
; CHECK-NEXT: da analyze - output [*|<]!
; CHECK-NEXT: Src: store i32 2, ptr %arrayidx9.us, align 4 --> Dst: store i32 2, ptr %arrayidx9.us, align 4
; CHECK-NEXT: da analyze - none!
;

85
llvm/test/Analysis/DependenceAnalysis/DADelin.ll
View File

@ -594,14 +594,15 @@ for.end12: ; preds = %for.inc10, %entry
}
; FIXME? It seems that we cannot prove that %N is non-negative...
define void @nonnegative(ptr nocapture %A, i32 %N) {
; CHECK-LABEL: 'nonnegative'
; CHECK-NEXT: Src: store i32 1, ptr %arrayidx, align 4 --> Dst: store i32 1, ptr %arrayidx, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: da analyze - output [* *]!
; CHECK-NEXT: Src: store i32 1, ptr %arrayidx, align 4 --> Dst: store i32 2, ptr %arrayidx, align 4
; CHECK-NEXT: da analyze - consistent output [0 0|<]!
; CHECK-NEXT: da analyze - output [* *|<]!
; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 4 --> Dst: store i32 2, ptr %arrayidx, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: da analyze - output [* *]!
;
entry:
%cmp44 = icmp eq i32 %N, 0
@ -630,3 +631,81 @@ for.latch:
exit:
ret void
}
; i = 0;
; do {
; a[k * i] = 42;
; a[k * (i + 1)] = 42;
; i++;
; } while (i != k);
;
; The dependency direction between the two stores depends on the sign of k.
; Note that the loop guard is omitted intentionally.
; FIXME: Each store has loop-carried dependencies on itself if k is zero.
;
define void @coeff_may_negative(ptr %a, i32 %k) {
; CHECK-LABEL: 'coeff_may_negative'
; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.0, align 1
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
; CHECK-NEXT: da analyze - output [*|<]!
; CHECK-NEXT: Src: store i8 42, ptr %idx.1, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
; CHECK-NEXT: da analyze - none!
;
entry:
br label %loop
loop:
%i = phi i32 [ 0, %entry ], [ %i.next, %loop ]
%i.next = add i32 %i, 1
%subscript.0 = mul i32 %i, %k
%subscript.1 = mul i32 %i.next, %k
%idx.0 = getelementptr i8, ptr %a, i32 %subscript.0
%idx.1 = getelementptr i8, ptr %a, i32 %subscript.1
store i8 42, ptr %idx.0
store i8 42, ptr %idx.1
%cond.exit = icmp eq i32 %i.next, %k
br i1 %cond.exit, label %exit, label %loop
exit:
ret void
}
; i = 0;
; do {
; a[k * i] = 42;
; a[k * (i + 1)] = 42;
; i++;
; } while (i != k);
;
; Note that the loop guard is omitted intentionally.
; FIXME: In principle, we can infer that the value of k is non-negative from
; the nsw flag.
;
define void @coeff_positive(ptr %a, i32 %k) {
; CHECK-LABEL: 'coeff_positive'
; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.0, align 1
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
; CHECK-NEXT: da analyze - output [*|<]!
; CHECK-NEXT: Src: store i8 42, ptr %idx.1, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
; CHECK-NEXT: da analyze - none!
;
entry:
br label %loop
loop:
%i = phi i32 [ 0, %entry ], [ %i.next, %loop ]
%i.next = add nsw i32 %i, 1
%subscript.0 = mul i32 %i, %k
%subscript.1 = mul i32 %i.next, %k
%idx.0 = getelementptr i8, ptr %a, i32 %subscript.0
%idx.1 = getelementptr i8, ptr %a, i32 %subscript.1
store i8 42, ptr %idx.0
store i8 42, ptr %idx.1
%cond.exit = icmp eq i32 %i.next, %k
br i1 %cond.exit, label %exit, label %loop
exit:
ret void
}