Skip to content

Conversation

@kasuga-fj
Copy link
Contributor

@kasuga-fj kasuga-fj commented Oct 20, 2025

This patch adds test cases that demonstrate missing dependencies in DA caused by the lack of overflow handling. These issues will be addressed by properly inserting overflow checks and bailing out when one is detected.

It covers the following dependence test functions:

  • Strong SIV
  • Weak-Crossing SIV
  • Weak-Zero SIV
  • Symbolic RDIV
  • GCD MIV

It does NOT cover:

  • Exact SIV
  • Exact RDIV
  • Banerjee MIV

Copy link
Contributor Author

kasuga-fj commented Oct 20, 2025

@kasuga-fj
Copy link
Contributor Author

kasuga-fj commented Oct 20, 2025

@sjoerdmeijer These are the cases I was able to find where intermediate computations cause overflow, leading to incorrect results being reported. Note that

  • I didn't check all functions, especially those that based on greatest common divisor algorithms (exactSIV, exactRDIV and gcdMIV).
    • It is more difficult to reason about the absence of overflow in them. I personally think we should conservatively insert overflow checks in these functions, even if we cannot find any test cases that trigger an overflow.
  • I also didn't investigate functions that I believe should be removed (e.g., banerjeeMIV).
  • These issues are not related to monotonicity checks. As far as I can tell, all of these cases are monotonic.
  • There are other correctness issues due to other reasons. I'll share them separately.

Base automatically changed from users/kasuga-fj/da-option-to-select-routine to main October 21, 2025 09:59
@kasuga-fj kasuga-fj force-pushed the users/kasuga-fj/da-add-tests-for-overflow branch 2 times, most recently from 443fde7 to 026d35b Compare October 21, 2025 11:42
@kasuga-fj kasuga-fj changed the title [DA] Add tests where dependencies are missed due to overflow [DA] Add tests where dependencies are missed due to overflow (NFC) Oct 21, 2025
Copy link
Contributor Author

@kasuga-fj kasuga-fj left a comment

Choose a reason for hiding this comment

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

Addressing the issues of SIV/RDIV would be a bit tricky, since they handle SCEVUnknown as well, not only constant values. I think the easiest way to fix them is to insert pessimistic overflow checks at the expense of analysis precision...

Comment on lines 6 to 27
; offset0 = 4;
; offset1 = 0;
; for (i = 0; i < 100; i++) {
; A[offset0] = 1;
; A[offset1] = 2;
; offset0 += 3*m;
; offset1 += 3;
; }
;
; FIXME: DependenceAnalysis currently detects no dependency between the two
; stores, but it does exist. E.g., consider `m` is 12297829382473034411, which
; is a modular multiplicative inverse of 3 under modulo 2^64. Then `offset0` is
; effectively `i + 4`, so accesses will be as follows:
;
; - A[offset0] : A[4], A[5], A[6], ...
; - A[offset1] : A[0], A[3], A[6], ...
;
; The root cause is that DA assumes `3*m` begin a multiple of 3 in mathematical
; sense, which isn't necessarily true due to overflow.
;
define void @gcdmiv_coef_ovfl(ptr %A, i64 %m) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit special; the coefficient have already wrapped. This should be fixed by #164408 .

Copy link
Member

@Meinersbur Meinersbur Oct 28, 2025

Choose a reason for hiding this comment

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

3*m is loop-invariant, and even computed before the loop. Ideally, DA shouldn't care about overflow, it us just "some invariant value" with this same result if only %step was passed as argument directly instead of %m.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I strongly agree with that, and I feel like the situation may be more serious. For example, given two accesses A[i + b 2*%n] and A[i + %n], StrongSIV regards the distance between them (i.e., Delta in strongSIVtest) as 2*%n - %n = %n. I think this is only valid if 2*%n doesn't overflow.

I think similar issues would exist in other parts of DA as well (StrongSIV and SymbolicRDIV are suspicious).

@kasuga-fj kasuga-fj marked this pull request as ready for review October 21, 2025 13:49
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Oct 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

Changes

This patch adds test cases that demonstrate missing dependencies in DA caused by the lack of overflow handling. These issues will be addressed by properly inserting overflow checks and bailing out when one is detected.

It covers the following dependence test functions:

  • Strong SIV
  • Weak-Crossing SIV
  • Weak-Zero SIV
  • Symbolic RDIV
  • GCD MIV

It does NOT cover:

  • Exact SIV
  • Exact RDIV
  • Banerjee MIV

Patch is 21.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164246.diff

5 Files Affected:

  • (added) llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll (+63)
  • (added) llvm/test/Analysis/DependenceAnalysis/strong-siv-overflow.ll (+72)
  • (added) llvm/test/Analysis/DependenceAnalysis/symbolic-rdiv-overflow.ll (+128)
  • (added) llvm/test/Analysis/DependenceAnalysis/weak-crossing-siv-overflow.ll (+122)
  • (added) llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-overflow.ll (+121)
diff --git a/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll b/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
new file mode 100644
index 0000000000000..724b347b56f3a
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
@@ -0,0 +1,63 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -disable-output "-passes=print<da>" 2>&1 | FileCheck %s
+; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=gcd-miv 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-GCD-MIV
+
+; offset0 = 4;
+; offset1 = 0;
+; for (i = 0; i < 100; i++) {
+;   A[offset0] = 1;
+;   A[offset1] = 2;
+;   offset0 += 3*m;
+;   offset1 += 3;
+; }
+;
+; FIXME: DependenceAnalysis currently detects no dependency between the two
+; stores, but it does exist. E.g., consider `m` is 12297829382473034411, which
+; is a modular multiplicative inverse of 3 under modulo 2^64. Then `offset0` is
+; effectively `i + 4`, so accesses will be as follows:
+;
+;   - A[offset0] : A[4], A[5], A[6], ...
+;   - A[offset1] : A[0], A[3], A[6], ...
+;
+; The root cause is that DA assumes `3*m` begin a multiple of 3 in mathematical
+; sense, which isn't necessarily true due to overflow.
+;
+define void @gcdmiv_coef_ovfl(ptr %A, i64 %m) {
+; CHECK-LABEL: 'gcdmiv_coef_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-GCD-MIV-LABEL: 'gcdmiv_coef_ovfl'
+; CHECK-GCD-MIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
+; CHECK-GCD-MIV-NEXT:    da analyze - consistent output [*]!
+; CHECK-GCD-MIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
+; CHECK-GCD-MIV-NEXT:    da analyze - none!
+; CHECK-GCD-MIV-NEXT:  Src: store i8 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
+; CHECK-GCD-MIV-NEXT:    da analyze - consistent output [*]!
+;
+entry:
+  %step = mul i64 3, %m
+  br label %loop
+
+loop:
+  %i = phi i64 [ 0, %entry ], [ %i.inc, %loop ]
+  %offset.0 = phi i64 [ 4, %entry ] , [ %offset.0.next, %loop ]
+  %offset.1 = phi i64 [ 0, %entry ] , [ %offset.1.next, %loop ]
+  %gep.0 = getelementptr inbounds i8, ptr %A, i64 %offset.0
+  %gep.1 = getelementptr inbounds i8, ptr %A, i64 %offset.1
+  store i8 1, ptr %gep.0
+  store i8 2, ptr %gep.1
+  %i.inc = add nuw nsw i64 %i, 1
+  %offset.0.next = add nsw i64 %offset.0, %step
+  %offset.1.next = add nsw i64 %offset.1, 3
+  %ec = icmp eq i64 %i.inc, 100
+  br i1 %ec, label %exit, label %loop
+
+exit:
+  ret void
+}
diff --git a/llvm/test/Analysis/DependenceAnalysis/strong-siv-overflow.ll b/llvm/test/Analysis/DependenceAnalysis/strong-siv-overflow.ll
new file mode 100644
index 0000000000000..559f4858612e5
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/strong-siv-overflow.ll
@@ -0,0 +1,72 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -disable-output "-passes=print<da>" 2>&1 | FileCheck %s
+; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=strong-siv 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-STRONG-SIV
+
+; offset0 = -2;
+; offset1 = -4;
+; for (i = 0; i < (1LL << 62); i++, offset0 += 2, offset1 += 2) {
+;   if (0 <= offset0)
+;     A[offset0] = 1;
+;   if (0 <= offset1)
+;     A[offset1] = 2;
+; }
+;
+; FIXME: DependenceAnalysis currently detects no dependency between the two
+; stores, but it does exist.
+; The root cause is that the product of the BTC and the coefficient triggers an
+; overflow.
+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-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 - none!
+; 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
+
+loop.header:
+  %i = phi i64 [ 0, %entry ], [ %i.inc, %loop.latch ]
+  %offset.0 = phi i64 [ -2, %entry ], [ %offset.0.next, %loop.latch ]
+  %offset.1 = phi i64 [ -4, %entry ], [ %offset.1.next, %loop.latch ]
+  %ec = icmp eq i64 %i, 4611686018427387904
+  br i1 %ec, label %exit, label %loop.body
+
+loop.body:
+  %cond.0 = icmp sge i64 %offset.0, 0
+  %cond.1 = icmp sge i64 %offset.1, 0
+  br i1 %cond.0, label %if.then.0, label %loop.middle
+
+if.then.0:
+  %gep.0 = getelementptr inbounds i8, ptr %A, i64 %offset.0
+  store i8 1, ptr %gep.0
+  br label %loop.middle
+
+loop.middle:
+  br i1 %cond.1, label %if.then.1, label %loop.latch
+
+if.then.1:
+  %gep.1 = getelementptr inbounds i8, ptr %A, i64 %offset.1
+  store i8 2, ptr %gep.1
+  br label %loop.latch
+
+loop.latch:
+  %i.inc = add nuw nsw i64 %i, 1
+  %offset.0.next = add nsw i64 %offset.0, 2
+  %offset.1.next = add nsw i64 %offset.1, 2
+  br label %loop.header
+
+exit:
+  ret void
+}
diff --git a/llvm/test/Analysis/DependenceAnalysis/symbolic-rdiv-overflow.ll b/llvm/test/Analysis/DependenceAnalysis/symbolic-rdiv-overflow.ll
new file mode 100644
index 0000000000000..f22553f9931a2
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/symbolic-rdiv-overflow.ll
@@ -0,0 +1,128 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -disable-output "-passes=print<da>" 2>&1 | FileCheck %s
+; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=symbolic-rdiv 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-SYMBOLIC-RDIV
+
+; offset = -2;
+; for (i = 0; i < (1LL << 62); i++, offset += 2) {
+;   if (0 <= offset0)
+;     A[offset0] = 1;
+;   A[i] = 2;
+; }
+;
+; FIXME: DependenceAnalysis currently detects no dependency between the two
+; stores, but it does exist.
+; The root cause is that the product of the BTC and the coefficient triggers an
+; overflow.
+define void @symbolicrdiv_prod_ovfl(ptr %A) {
+; CHECK-LABEL: 'symbolicrdiv_prod_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-SYMBOLIC-RDIV-LABEL: 'symbolicrdiv_prod_ovfl'
+; CHECK-SYMBOLIC-RDIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
+; CHECK-SYMBOLIC-RDIV-NEXT:    da analyze - none!
+; CHECK-SYMBOLIC-RDIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
+; CHECK-SYMBOLIC-RDIV-NEXT:    da analyze - none!
+; CHECK-SYMBOLIC-RDIV-NEXT:  Src: store i8 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
+; CHECK-SYMBOLIC-RDIV-NEXT:    da analyze - consistent output [*]!
+;
+entry:
+  br label %loop.header
+
+loop.header:
+  %i = phi i64 [ 0, %entry ], [ %i.inc, %loop.latch ]
+  %offset = phi i64 [ -2, %entry ], [ %offset.next, %loop.latch ]
+  %ec = icmp eq i64 %i, 4611686018427387904
+  br i1 %ec, label %exit, label %loop.body
+
+loop.body:
+  %cond = icmp sge i64 %offset, 0
+  br i1 %cond, label %if.then, label %loop.latch
+
+if.then:
+  %gep.0 = getelementptr inbounds i8, ptr %A, i64 %offset
+  store i8 1, ptr %gep.0
+  br label %loop.latch
+
+loop.latch:
+  %gep.1 = getelementptr inbounds i8, ptr %A, i64 %i
+  store i8 2, ptr %gep.1
+  %i.inc = add nuw nsw i64 %i, 1
+  %offset.next = add nsw i64 %offset, 2
+  br label %loop.header
+
+exit:
+  ret void
+}
+
+; offset0 = -4611686018427387904  // -2^62
+; offset1 =  4611686018427387904  // 2^62
+; for (i = 0; i < (1LL << 62) - 100; i++) {
+;   if (0 <= offset0)
+;     A[offset0] = 1;
+;   if (0 <= offset1)
+;     A[offset1] = 2;
+;   offset0 += 2;
+;   offset1 -= 1;
+; }
+;
+; FIXME: DependenceAnalysis currently detects no dependency between the two
+; stores, but it does exist.
+; The root cause is that the calculation of the differenct between the two
+; constants (-2^62 and 2^62) triggers an overflow.
+define void @symbolicrdiv_delta_ovfl(ptr %A) {
+; CHECK-LABEL: 'symbolicrdiv_delta_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-SYMBOLIC-RDIV-LABEL: 'symbolicrdiv_delta_ovfl'
+; CHECK-SYMBOLIC-RDIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
+; CHECK-SYMBOLIC-RDIV-NEXT:    da analyze - consistent output [*]!
+; CHECK-SYMBOLIC-RDIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
+; CHECK-SYMBOLIC-RDIV-NEXT:    da analyze - none!
+; CHECK-SYMBOLIC-RDIV-NEXT:  Src: store i8 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
+; CHECK-SYMBOLIC-RDIV-NEXT:    da analyze - consistent output [*]!
+;
+entry:
+  br label %loop.header
+
+loop.header:
+  %i = phi i64 [ 0, %entry ], [ %i.inc, %loop.latch ]
+  %offset.0 = phi i64 [ -4611686018427387904, %entry ], [ %offset.0.next, %loop.latch ]
+  %offset.1 = phi i64 [ 4611686018427387904, %entry ], [ %offset.1.next, %loop.latch ]
+  %cond.0 = icmp sge i64 %offset.0, 0
+  %cond.1 = icmp sge i64 %offset.1, 0
+  br i1 %cond.0, label %if.then.0, label %loop.middle
+
+if.then.0:
+  %gep.0 = getelementptr inbounds i8, ptr %A, i64 %offset.0
+  store i8 1, ptr %gep.0
+  br label %loop.middle
+
+loop.middle:
+  br i1 %cond.1, label %if.then.1, label %loop.latch
+
+if.then.1:
+  %gep.1 = getelementptr inbounds i8, ptr %A, i64 %offset.1
+  store i8 2, ptr %gep.1
+  br label %loop.latch
+
+loop.latch:
+  %i.inc = add nuw nsw i64 %i, 1
+  %offset.0.next = add nsw i64 %offset.0, 2
+  %offset.1.next = sub nsw i64 %offset.1, 1
+  %ec = icmp eq i64 %i.inc, 4611686018427387804 ; 2^62 - 100
+  br i1 %ec, label %exit, label %loop.header
+
+exit:
+  ret void
+}
diff --git a/llvm/test/Analysis/DependenceAnalysis/weak-crossing-siv-overflow.ll b/llvm/test/Analysis/DependenceAnalysis/weak-crossing-siv-overflow.ll
new file mode 100644
index 0000000000000..59412d8381d68
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/weak-crossing-siv-overflow.ll
@@ -0,0 +1,122 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -disable-output "-passes=print<da>" 2>&1 | FileCheck %s
+; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=weak-crossing-siv 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-WEAK-CROSSING-SIV
+
+; max_i = INT64_MAX/3  // 3074457345618258602
+; for (long long i = 0; i <= max_i; i++) {
+;   A[-3*i + INT64_MAX] = 0;
+;   if (i)
+;     A[3*i - 2] = 1;
+; }
+;
+; FIXME: DependencyAnalsysis currently detects no dependency between
+; `A[-3*i + INT64_MAX]` and `A[3*i - 2]`, but it does exist. For example,
+;
+;  memory location  | -3*i + INT64_MAX | 3*i - 2
+; ------------------|------------------|-----------
+;  A[1]             | i = max_i        | i = 1
+;  A[INT64_MAX - 3] | i = 1            | i = max_i
+;
+; The root cause is that the calculation of the differenct between the two
+; constants (INT64_MAX and -2) triggers an overflow.
+
+define void @weakcorssing_delta_ovfl(ptr %A) {
+; CHECK-LABEL: 'weakcorssing_delta_ovfl'
+; CHECK-NEXT:  Src: store i8 0, ptr %idx.0, align 1 --> Dst: store i8 0, ptr %idx.0, align 1
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i8 0, ptr %idx.0, align 1 --> Dst: store i8 1, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i8 1, ptr %idx.1, align 1 --> Dst: store i8 1, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - none!
+;
+; CHECK-WEAK-CROSSING-SIV-LABEL: 'weakcorssing_delta_ovfl'
+; CHECK-WEAK-CROSSING-SIV-NEXT:  Src: store i8 0, ptr %idx.0, align 1 --> Dst: store i8 0, ptr %idx.0, align 1
+; CHECK-WEAK-CROSSING-SIV-NEXT:    da analyze - consistent output [*]!
+; CHECK-WEAK-CROSSING-SIV-NEXT:  Src: store i8 0, ptr %idx.0, align 1 --> Dst: store i8 1, ptr %idx.1, align 1
+; CHECK-WEAK-CROSSING-SIV-NEXT:    da analyze - none!
+; CHECK-WEAK-CROSSING-SIV-NEXT:  Src: store i8 1, ptr %idx.1, align 1 --> Dst: store i8 1, ptr %idx.1, align 1
+; CHECK-WEAK-CROSSING-SIV-NEXT:    da analyze - consistent output [*]!
+;
+entry:
+  br label %loop.header
+
+loop.header:
+  %i = phi i64 [ 0, %entry ], [ %i.inc, %loop.latch ]
+  %subscript.0 = phi i64 [ 9223372036854775807, %entry ], [ %subscript.0.next, %loop.latch ]
+  %subscript.1 = phi i64 [ -2, %entry ], [ %subscript.1.next, %loop.latch ]
+  %idx.0 = getelementptr inbounds i8, ptr %A, i64 %subscript.0
+  store i8 0, ptr %idx.0
+  %cond.store = icmp ne i64 %i, 0
+  br i1 %cond.store, label %if.store, label %loop.latch
+
+if.store:
+  %idx.1 = getelementptr inbounds i8, ptr %A, i64 %subscript.1
+  store i8 1, ptr %idx.1
+  br label %loop.latch
+
+loop.latch:
+  %i.inc = add nuw nsw i64 %i, 1
+  %subscript.0.next = add nsw i64 %subscript.0, -3
+  %subscript.1.next = add nsw i64 %subscript.1, 3
+  %ec = icmp sgt i64 %i.inc, 3074457345618258602
+  br i1 %ec, label %exit, label %loop.header
+
+exit:
+  ret void
+}
+
+; max_i = INT64_MAX/3  // 3074457345618258602
+; for (long long i = 0; i <= max_i; i++) {
+;   A[-3*i + INT64_MAX] = 0;
+;   A[3*i + 1] = 1;
+; }
+;
+; FIXME: DependencyAnalsysis currently detects no dependency between
+; `A[-3*i + INT64_MAX]` and `A[3*i - 2]`, but it does exist. For example,
+;
+;  memory location  | -3*i + INT64_MAX | 3*i + 1
+; ------------------|------------------|--------------
+;  A[1]             | i = max_i        | i = 0
+;  A[INT64_MAX - 3] | i = 1            | i = max_i - 1
+;
+; The root cause is that the product of the BTC, the coefficient, and 2
+; triggers an overflow.
+;
+define void @weakcorssing_prod_ovfl(ptr %A) {
+; CHECK-LABEL: 'weakcorssing_prod_ovfl'
+; CHECK-NEXT:  Src: store i8 0, ptr %idx.0, align 1 --> Dst: store i8 0, ptr %idx.0, align 1
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i8 0, ptr %idx.0, align 1 --> Dst: store i8 1, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i8 1, ptr %idx.1, align 1 --> Dst: store i8 1, ptr %idx.1, align 1
+; CHECK-NEXT:    da analyze - none!
+;
+; CHECK-WEAK-CROSSING-SIV-LABEL: 'weakcorssing_prod_ovfl'
+; CHECK-WEAK-CROSSING-SIV-NEXT:  Src: store i8 0, ptr %idx.0, align 1 --> Dst: store i8 0, ptr %idx.0, align 1
+; CHECK-WEAK-CROSSING-SIV-NEXT:    da analyze - consistent output [*]!
+; CHECK-WEAK-CROSSING-SIV-NEXT:  Src: store i8 0, ptr %idx.0, align 1 --> Dst: store i8 1, ptr %idx.1, align 1
+; CHECK-WEAK-CROSSING-SIV-NEXT:    da analyze - none!
+; CHECK-WEAK-CROSSING-SIV-NEXT:  Src: store i8 1, ptr %idx.1, align 1 --> Dst: store i8 1, ptr %idx.1, align 1
+; CHECK-WEAK-CROSSING-SIV-NEXT:    da analyze - consistent output [*]!
+;
+entry:
+  br label %loop
+
+loop:
+  %i = phi i64 [ 0, %entry ], [ %i.inc, %loop ]
+  %subscript.0 = phi i64 [ 9223372036854775807, %entry ], [ %subscript.0.next, %loop ]
+  %subscript.1 = phi i64 [ 1, %entry ], [ %subscript.1.next, %loop ]
+  %idx.0 = getelementptr inbounds i8, ptr %A, i64 %subscript.0
+  %idx.1 = getelementptr inbounds i8, ptr %A, i64 %subscript.1
+  store i8 0, ptr %idx.0
+  store i8 1, ptr %idx.1
+  %i.inc = add nuw nsw i64 %i, 1
+  %subscript.0.next = add nsw i64 %subscript.0, -3
+  %subscript.1.next = add nsw i64 %subscript.1, 3
+  %ec = icmp sgt i64 %i.inc, 3074457345618258602
+  br i1 %ec, label %exit, label %loop
+
+exit:
+  ret void
+}
diff --git a/llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-overflow.ll b/llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-overflow.ll
new file mode 100644
index 0000000000000..0e5deb610bd61
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-overflow.ll
@@ -0,0 +1,121 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -disable-output "-passes=print<da>" 2>&1 | FileCheck %s
+; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=weak-zero-siv 2>&1 \
+; RUN:     | FileCheck %s --check-prefix=CHECK-WEAK-ZERO-SIV
+
+; offset = -2;
+; for (i = 0; i < (1LL << 62); i++, offset += 2) {
+;   if (0 <= offset)
+;     A[offset] = 1;
+;   A[2] = 2;
+; }
+;
+; FIXME: DependenceAnalysis currently detects no dependency between the two
+; stores, but it does exist. The root cause is that the product of the BTC and
+; the coefficient triggers an overflow.
+;
+define void @weakzero_dst_siv_prod_ovfl(ptr %A) {
+; CHECK-LABEL: 'weakzero_dst_siv_prod_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 - consistent output [S]!
+;
+; CHECK-WEAK-ZERO-SIV-LABEL: 'weakzero_dst_siv_prod_ovfl'
+; CHECK-WEAK-ZERO-SIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
+; CHECK-WEAK-ZERO-SIV-NEXT:    da analyze - consistent output [*]!
+; CHECK-WEAK-ZERO-SIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
+; CHECK-WEAK-ZERO-SIV-NEXT:    da analyze - none!
+; CHECK-WEAK-ZERO-SIV-NEXT:  Src: store i8 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
+; CHECK-WEAK-ZERO-SIV-NEXT:    da analyze - consistent output [S]!
+;
+entry:
+  br label %loop.header
+
+loop.header:
+  %i = phi i64 [ 0, %entry ], [ %i.inc, %loop.latch ]
+  %offset = phi i64 [ -2, %entry ], [ %offset.next, %loop.latch ]
+  %ec = icmp eq i64 %i, 4611686018427387904
+  br i1 %ec, label %exit, label %loop.body
+
+loop.body:
+  %cond = icmp sge i64 %offset, 0
+  br i1 %cond, label %if.then, label %loop.latch
+
+if.then:
+  %gep.0 = getelementptr inbounds i8, ptr %A, i64 %offset
+  store i8 1, ptr %gep.0
+  br label %loop.latch
+
+loop.latch:
+  %gep.1 = getelementptr inbounds i8, ptr %A, i64 2
+  store i8 2, ptr %gep.1
+  %i.inc = add nuw nsw i64 %i, 1
+  %offset.next = add nsw i64 %offset, 2
+  br label %loop.header
+
+exit:
+  ret void
+}
+
+; offset = -1;
+; for (i = 0; i < n; i++, offset += 2) {
+;   if (0 <= offset)
+;     A[offset] = 1;
+;   A[INT64_MAX] = 2;
+; }
+;
+; FIXME: DependenceAnalysis currently detects no dependency between the two
+; stores, but it does exist. When `%n` is 2^62, the value of `%offset` will be
+; the same as INT64_MAX at the last iteration.
+; The root cause is that the calculation of the differenct between the two
+; constants (INT64_MAX and -1) triggers an overflow.
+;
+define void @weakzero_dst_siv_delta_ovfl(ptr %A, i64 %n) {
+; CHECK-LABEL: 'weakzero_dst_siv_delta_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...
[truncated]

@kasuga-fj kasuga-fj force-pushed the users/kasuga-fj/da-add-tests-for-overflow branch from 026d35b to 8f5537b Compare October 28, 2025 07:57
@kasuga-fj kasuga-fj force-pushed the users/kasuga-fj/da-add-tests-for-overflow branch from 8f5537b to 7444b19 Compare October 28, 2025 08:20
Comment on lines 6 to 27
; offset0 = 4;
; offset1 = 0;
; for (i = 0; i < 100; i++) {
; A[offset0] = 1;
; A[offset1] = 2;
; offset0 += 3*m;
; offset1 += 3;
; }
;
; FIXME: DependenceAnalysis currently detects no dependency between the two
; stores, but it does exist. E.g., consider `m` is 12297829382473034411, which
; is a modular multiplicative inverse of 3 under modulo 2^64. Then `offset0` is
; effectively `i + 4`, so accesses will be as follows:
;
; - A[offset0] : A[4], A[5], A[6], ...
; - A[offset1] : A[0], A[3], A[6], ...
;
; The root cause is that DA assumes `3*m` begin a multiple of 3 in mathematical
; sense, which isn't necessarily true due to overflow.
;
define void @gcdmiv_coef_ovfl(ptr %A, i64 %m) {
Copy link
Member

@Meinersbur Meinersbur Oct 28, 2025

Choose a reason for hiding this comment

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

3*m is loop-invariant, and even computed before the loop. Ideally, DA shouldn't care about overflow, it us just "some invariant value" with this same result if only %step was passed as argument directly instead of %m.

Comment on lines 16 to 17
; FIXME: DependenceAnalysis currently detects no dependency between the two
; stores, but it does exist.
Copy link
Member

Choose a reason for hiding this comment

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

Do you know at which values of i, offset0 and offset1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the dependency in this case is trivial, but using offset0 and offset1 makes it unclear. I've rewritten the pseudo code and add brief comments to clarify the dependency.

Comment on lines 14 to 15
; FIXME: DependenceAnalysis currently detects no dependency between the two
; stores, but it does exist.
Copy link
Member

Choose a reason for hiding this comment

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

Do you know at which values of i and offset0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revised the pseudo code and added brief comment.

Comment on lines 17 to 20
; memory location | -3*i + INT64_MAX | 3*i - 2
; ------------------|------------------|-----------
; A[1] | i = max_i | i = 1
; A[INT64_MAX - 3] | i = 1 | i = max_i
Copy link
Member

@Meinersbur Meinersbur Oct 28, 2025

Choose a reason for hiding this comment

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

Suggested change
; memory location | -3*i + INT64_MAX | 3*i - 2
; ------------------|------------------|-----------
; A[1] | i = max_i | i = 1
; A[INT64_MAX - 3] | i = 1 | i = max_i
; memory access | i == 1 | i == max_i
; ---------------------|------------------|-----------
; A[-3*i + INT64_MAX] | A[INT64_MAX - 3] | A[1]
; A[3*i - 2] | A[1] | A[INT64_MAX - 3]

It is not clear that the columns represent the memory access at line 9, respectively at line 11.
I find a table with location addresses per values of i for each memory access more intiuitive than a table per address and when it is accessed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, replace the table. Also added similar one to other non-trivial cases.

; max_i = INT64_MAX/3 // 3074457345618258602
; for (long long i = 0; i <= max_i; i++) {
; A[-3*i + INT64_MAX] = 0;
; if (i)
Copy link
Member

Choose a reason for hiding this comment

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

Does the if (i) matter for DA?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The purpose of this condition (and similar ones in other cases) is to avoid UB caused by the combination of the size limitation of allocated objects and inbounds. However, I'm not sure if the presence of these branches affects DA, or simply dropping inbounds would be sufficient.

Copy link
Member

Choose a reason for hiding this comment

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

I've seen it in the other cases to avoid the subscripts becoming negative. In this case, A[-3*0 + INT64_MAX] == A[INT64_MAX] still seems to be valid, at least no less than other values of i. E.g. with &A == nullptr (valid in Linux kernel code), the resulting address is still within the half-address space bound.

However, I see the point, we can/should leave it to be on the safe side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is if (i) A[3*i - 2], which prevents access A[3*0 - 2] == A[-2]. Like the other guards, this one also excludes negative subscripts.

Comment on lines 15 to 16
; stores, but it does exist. The root cause is that the product of the BTC and
; the coefficient triggers an overflow.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
; stores, but it does exist. The root cause is that the product of the BTC and
; the coefficient triggers an overflow.
; stores, but it does exist. The root cause is that the product of the BTC and
; the coefficient overflows.

in the signed integer space? Is the BTC 1LL << 62 - 1 and coefficient the access stride, i.e. 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Added comment to clarify that.

Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

Comment on lines +78 to +81
; memory access | i == 2^61 | i == 2^61 + 2^59 | i == 2^61 + 2^60
; -------------------------|-----------|------------------|-------------------
; A[2*i - 2^62] (offset0) | | A[2^60] | A[2^61]
; A[-i + 2^62] (offset1) | A[2^61] | | A[2^60]
Copy link
Member

Choose a reason for hiding this comment

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

👍

; max_i = INT64_MAX/3 // 3074457345618258602
; for (long long i = 0; i <= max_i; i++) {
; A[-3*i + INT64_MAX] = 0;
; if (i)
Copy link
Member

Choose a reason for hiding this comment

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

I've seen it in the other cases to avoid the subscripts becoming negative. In this case, A[-3*0 + INT64_MAX] == A[INT64_MAX] still seems to be valid, at least no less than other values of i. E.g. with &A == nullptr (valid in Linux kernel code), the resulting address is still within the half-address space bound.

However, I see the point, we can/should leave it to be on the safe side.

@kasuga-fj kasuga-fj enabled auto-merge (squash) October 30, 2025 10:09
@kasuga-fj kasuga-fj merged commit 0e2b890 into main Oct 30, 2025
9 of 10 checks passed
@kasuga-fj kasuga-fj deleted the users/kasuga-fj/da-add-tests-for-overflow branch October 30, 2025 10:49
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
…lvm#164246)

This patch adds test cases that demonstrate missing dependencies in DA
caused by the lack of overflow handling. These issues will be addressed
by properly inserting overflow checks and bailing out when one is
detected.

It covers the following dependence test functions:

- Strong SIV
- Weak-Crossing SIV
- Weak-Zero SIV
- Symbolic RDIV
- GCD MIV

It does NOT cover:

- Exact SIV
- Exact RDIV
- Banerjee MIV
@sjoerdmeijer
Copy link
Collaborator

@sjoerdmeijer These are the cases I was able to find where intermediate computations cause overflow, leading to incorrect results being reported. Note that

  • I didn't check all functions, especially those that based on greatest common divisor algorithms (exactSIV, exactRDIV and gcdMIV).

    • It is more difficult to reason about the absence of overflow in them. I personally think we should conservatively insert overflow checks in these functions, even if we cannot find any test cases that trigger an overflow.
  • I also didn't investigate functions that I believe should be removed (e.g., banerjeeMIV).

  • These issues are not related to monotonicity checks. As far as I can tell, all of these cases are monotonic.

  • There are other correctness issues due to other reasons. I'll share them separately.

Just wanted to leave this message: thanks for the ping. I am attending the llvm dev conf, and am traveling. But I will be back in the office next week, and will be able to help with DA. Next week I am going to pick up a few things.

luciechoi pushed a commit to luciechoi/llvm-project that referenced this pull request Nov 1, 2025
…lvm#164246)

This patch adds test cases that demonstrate missing dependencies in DA
caused by the lack of overflow handling. These issues will be addressed
by properly inserting overflow checks and bailing out when one is
detected.

It covers the following dependence test functions:

- Strong SIV
- Weak-Crossing SIV
- Weak-Zero SIV
- Symbolic RDIV
- GCD MIV

It does NOT cover:

- Exact SIV
- Exact RDIV
- Banerjee MIV
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
…lvm#164246)

This patch adds test cases that demonstrate missing dependencies in DA
caused by the lack of overflow handling. These issues will be addressed
by properly inserting overflow checks and bailing out when one is
detected.

It covers the following dependence test functions:

- Strong SIV
- Weak-Crossing SIV
- Weak-Zero SIV
- Symbolic RDIV
- GCD MIV

It does NOT cover:

- Exact SIV
- Exact RDIV
- Banerjee MIV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants