Skip to content

[DA] Check for overflow in strong SIV test #166223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 6, 2025
+108 −31
Merged

[DA] Check for overflow in strong SIV test #166223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b021e4
[DA] Adding a test case failure due to overflow in strong SIV test
1997alireza Oct 28, 2025
4c2ba55
[DA] Check for overflow in strong SIV test
1997alireza Oct 22, 2025
cd52a57
Merge branch 'main' into strong-siv-overflow-2
1997alireza Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions llvm/lib/Analysis/DependenceAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1587,6 +1587,15 @@ static const SCEV *minusSCEVNoSignedOverflow(const SCEV *A, const SCEV *B,
return nullptr;
}

/// Returns \p A * \p B if it guaranteed not to signed wrap. Otherwise returns
/// nullptr. \p A and \p B must have the same integer type.
static const SCEV *mulSCEVNoSignedOverflow(const SCEV *A, const SCEV *B,
ScalarEvolution &SE) {
if (SE.willNotOverflow(Instruction::Mul, /*Signed=*/true, A, B))
return SE.getMulExpr(A, B);
return nullptr;
}

/// Returns the absolute value of \p A. In the context of dependence analysis,
/// we need an absolute value in a mathematical sense. If \p A is the signed
/// minimum value, we cannot represent it unless extending the original type.
Expand Down Expand Up @@ -1686,7 +1695,11 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
assert(0 < Level && Level <= CommonLevels && "level out of range");
Level--;

const SCEV *Delta = SE->getMinusSCEV(SrcConst, DstConst);
const SCEV *Delta = minusSCEVNoSignedOverflow(SrcConst, DstConst, *SE);
if (!Delta) {
Result.Consistent = false;
Copy link
Contributor

@kasuga-fj kasuga-fj Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Consistent need to be set to false?

Copy link
Contributor Author

@1997alireza 1997alireza Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, otherwise we will add the Consistent attribute to a few test cases that do not currently have this attribute. As you are also aware, the attribute Consistent seems to not be well defined. If it is true, we may need to try to remove it in other patches.

return false;
}
LLVM_DEBUG(dbgs() << "\t Delta = " << *Delta);
LLVM_DEBUG(dbgs() << ", " << *Delta->getType() << "\n");

Expand All @@ -1702,7 +1715,9 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
const SCEV *AbsCoeff = absSCEVNoSignedOverflow(Coeff, *SE);
if (!AbsDelta || !AbsCoeff)
return false;
const SCEV *Product = SE->getMulExpr(UpperBound, AbsCoeff);
const SCEV *Product = mulSCEVNoSignedOverflow(UpperBound, AbsCoeff, *SE);
if (!Product)
return false;
return isKnownPredicate(CmpInst::ICMP_SGT, AbsDelta, Product);
}();
if (IsDeltaLarge) {
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/DependenceAnalysis/SimpleSIVNoValidityCheck.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ define void @t3(i64 %n, i64 %m, i64 %lb, ptr %a) {
; CHECK-NEXT: Src: %2 = load i32, ptr %arrayidx6, align 4 --> Dst: %2 = load i32, ptr %arrayidx6, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: %2 = load i32, ptr %arrayidx6, align 4 --> Dst: store i32 %2, ptr %arrayidx8, align 4
; CHECK-NEXT: da analyze - consistent anti [1 -2]!
; CHECK-NEXT: da analyze - anti [1 *]!
; CHECK-NEXT: Src: store i32 %2, ptr %arrayidx8, align 4 --> Dst: store i32 %2, ptr %arrayidx8, align 4
; CHECK-NEXT: da analyze - none!
;
Expand Down
86 changes: 72 additions & 14 deletions llvm/test/Analysis/DependenceAnalysis/StrongSIV.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa 2>&1 \
; RUN: | FileCheck %s
; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-ALL
; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa -da-enable-dependence-test=strong-siv 2>&1 \
; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-STRONG-SIV

target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.6.0"
Expand Down Expand Up @@ -423,19 +425,33 @@ for.end: ; preds = %for.body
;; *B++ = A[i + 2*n];

define void @strong9(ptr %A, ptr %B, i64 %n) nounwind uwtable ssp {
; CHECK-LABEL: 'strong9'
; CHECK-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %conv, ptr %arrayidx, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-NEXT: da analyze - confused!
; CHECK-NEXT: Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-NEXT: da analyze - confused!
; CHECK-NEXT: Src: store i32 %0, ptr %B.addr.02, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-ALL-LABEL: 'strong9'
; CHECK-ALL-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %conv, ptr %arrayidx, align 4
; CHECK-ALL-NEXT: da analyze - none!
; CHECK-ALL-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
; CHECK-ALL-NEXT: da analyze - none!
; CHECK-ALL-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-ALL-NEXT: da analyze - confused!
; CHECK-ALL-NEXT: Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
; CHECK-ALL-NEXT: da analyze - none!
; CHECK-ALL-NEXT: Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-ALL-NEXT: da analyze - confused!
; CHECK-ALL-NEXT: Src: store i32 %0, ptr %B.addr.02, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-ALL-NEXT: da analyze - none!
;
; CHECK-STRONG-SIV-LABEL: 'strong9'
; CHECK-STRONG-SIV-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %conv, ptr %arrayidx, align 4
; CHECK-STRONG-SIV-NEXT: da analyze - none!
; CHECK-STRONG-SIV-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
; CHECK-STRONG-SIV-NEXT: da analyze - flow [*|<]!
; CHECK-STRONG-SIV-NEXT: Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-STRONG-SIV-NEXT: da analyze - confused!
; CHECK-STRONG-SIV-NEXT: Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
; CHECK-STRONG-SIV-NEXT: da analyze - none!
; CHECK-STRONG-SIV-NEXT: Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-STRONG-SIV-NEXT: da analyze - confused!
; CHECK-STRONG-SIV-NEXT: Src: store i32 %0, ptr %B.addr.02, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
; CHECK-STRONG-SIV-NEXT: da analyze - none!
;
entry:
%cmp1 = icmp eq i64 %n, 0
Expand Down Expand Up @@ -512,3 +528,45 @@ for.body: ; preds = %entry, %for.body
for.end: ; preds = %for.body
ret void
}


;; for (long unsigned i = 0; i < 9223372036854775806; i++)
;; for (long unsigned j = 0; j < 2147483640; j++)
;; if (i < 3000000000)
;; A[i] = 0;
;
; FIXME: DependenceAnalysis fails to detect the dependency between A[i] and
; itself, and the issue is not caused by the Strong SIV.
define void @strong11(ptr %A) nounwind uwtable ssp {
; CHECK-ALL-LABEL: 'strong11'
; CHECK-ALL-NEXT: Src: store i32 0, ptr %arrayidx, align 4 --> Dst: store i32 0, ptr %arrayidx, align 4
; CHECK-ALL-NEXT: da analyze - none!
;
; CHECK-STRONG-SIV-LABEL: 'strong11'
; CHECK-STRONG-SIV-NEXT: Src: store i32 0, ptr %arrayidx, align 4 --> Dst: store i32 0, ptr %arrayidx, align 4
; CHECK-STRONG-SIV-NEXT: da analyze - consistent output [0 S]!
;
entry:
br label %for.cond1.preheader

for.cond1.preheader: ; preds = %entry, %for.cond.cleanup3
%i.017 = phi i64 [ 0, %entry ], [ %inc8, %for.cond.cleanup3 ]
%cmp5 = icmp samesign ult i64 %i.017, 3000000000
%arrayidx = getelementptr inbounds nuw i32, ptr %A, i64 %i.017
br i1 %cmp5, label %for.body4.us, label %for.cond.cleanup3

for.body4.us: ; preds = %for.cond1.preheader, %for.body4.us
%j.016.us = phi i64 [ %inc.us, %for.body4.us ], [ 0, %for.cond1.preheader ]
store i32 0, ptr %arrayidx, align 4
%inc.us = add nuw nsw i64 %j.016.us, 1
%exitcond.not = icmp eq i64 %inc.us, 2147483640
br i1 %exitcond.not, label %for.cond.cleanup3, label %for.body4.us

for.cond.cleanup: ; preds = %for.cond.cleanup3
ret void

for.cond.cleanup3: ; preds = %for.body4.us, %for.cond1.preheader
%inc8 = add nuw nsw i64 %i.017, 1
%exitcond19.not = icmp eq i64 %inc8, 9223372036854775806
br i1 %exitcond19.not, label %for.cond.cleanup, label %for.cond1.preheader
}
32 changes: 18 additions & 14 deletions llvm/test/Analysis/DependenceAnalysis/strong-siv-overflow.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,24 @@
; A[2*i - 4] = 2;
; }
;
; FIXME: DependenceAnalysis currently detects no dependency between the two
; stores, but it does exist. For example, each store will access A[0] when i
; is 1 and 2 respectively.
; The root cause is that the product of the BTC and the coefficient
; ((1LL << 62) - 1 and 2) overflows in a signed sense.
Comment on lines -15 to -19
Copy link
Contributor

@kasuga-fj kasuga-fj Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you rewrite this comment?

Copy link
Contributor Author

@1997alireza 1997alireza Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because now the issue that has been mentioned in the comment regarding the overflow in the product of the BTC and the coefficient is resolved. By this patch, Strong SIV is generating the expected output consistent output [1]!. Now the problem is out of the Strong SIV scope that is leading the DA to generate an incorrect result.

Copy link
Contributor

@kasuga-fj kasuga-fj Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is not the same as @strongSIV11. It doesn't contain the access to A[i]. If you want to modify it, please follow the above pseudo code.

Copy link
Contributor Author

@1997alireza 1997alireza Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean. Please clarify your comment on this change. No it is not same as @strongSIV11 and it does not contain an access to A[i]. The result provided by StrongSIV consistent output [1]! is correct and is regarding the store to A[2*i - 4] versus A[2*i - 2].

Copy link
Contributor

@kasuga-fj kasuga-fj Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it is not same as @strongSIV11 and it does not contain an access to A[i]

But the comment is now saying FIXME: DependenceAnalysis fails to detect the dependency between A[i]....

Copy link
Contributor Author

@1997alireza 1997alireza Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing. Overlooked it. Now it is fixed.

Copy link
Contributor

@kasuga-fj kasuga-fj Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem to be fixed. Maybe forget to commit?

Copy link
Contributor Author

@1997alireza 1997alireza Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Committed now.

; FIXME: DependenceAnalysis fails to detect the dependency between the two
; stores, and the issue is not caused by the Strong SIV.
define void @strongsiv_const_ovfl(ptr %A) {
; CHECK-LABEL: 'strongsiv_const_ovfl'
; CHECK-NEXT: Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i8 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
; CHECK-NEXT: da analyze - none!
; CHECK-ALL-LABEL: 'strongsiv_const_ovfl'
; CHECK-ALL-NEXT: Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
; CHECK-ALL-NEXT: da analyze - none!
; CHECK-ALL-NEXT: Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
; CHECK-ALL-NEXT: da analyze - none!
; CHECK-ALL-NEXT: Src: store i8 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
; CHECK-ALL-NEXT: da analyze - none!
;
; CHECK-STRONG-SIV-LABEL: 'strongsiv_const_ovfl'
; CHECK-STRONG-SIV-NEXT: Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
; CHECK-STRONG-SIV-NEXT: da analyze - none!
; CHECK-STRONG-SIV-NEXT: Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
; CHECK-STRONG-SIV-NEXT: da analyze - consistent output [1]!
; CHECK-STRONG-SIV-NEXT: Src: store i8 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
; CHECK-STRONG-SIV-NEXT: da analyze - none!
;
entry:
br label %loop.header
Expand Down Expand Up @@ -64,5 +69,4 @@ exit:
ret void
}
;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
; CHECK-ALL: {{.*}}
; CHECK-STRONG-SIV: {{.*}}
; CHECK: {{.*}}