-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[DA] Add tests where dependencies are missed due to overflow (NFC) #164246
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.
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
Changes from 2 commits
16601d9
7444b19
ae260d3
314ad42
013fdde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| ; 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 \ | ||
| ; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-ALL | ||
| ; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=gcd-miv 2>&1 \ | ||
| ; RUN: | FileCheck %s --check-prefixes=CHECK,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) { | ||
|
Comment on lines
7
to
27
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 .
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I think similar issues would exist in other parts of DA as well (StrongSIV and SymbolicRDIV are suspicious). |
||
| ; CHECK-ALL-LABEL: 'gcdmiv_coef_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-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 | ||
| } | ||
| ;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line: | ||
| ; CHECK: {{.*}} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| ; 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 \ | ||
| ; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-ALL | ||
| ; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=strong-siv 2>&1 \ | ||
| ; RUN: | FileCheck %s --check-prefixes=CHECK,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! | ||
| ; | ||
| 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 | ||
| } | ||
| ;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line: | ||
| ; CHECK-ALL: {{.*}} | ||
| ; CHECK-STRONG-SIV: {{.*}} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| ; 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 \ | ||
| ; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-ALL | ||
| ; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=symbolic-rdiv 2>&1 \ | ||
| ; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-SYMBOLIC-RDIV | ||
|
|
||
| ; offset = -2; | ||
| ; for (i = 0; i < (1LL << 62); i++, offset += 2) { | ||
| ; if (0 <= offset0) | ||
| ; A[offset0] = 1; | ||
kasuga-fj marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ; 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-ALL-LABEL: 'symbolicrdiv_prod_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-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-ALL-LABEL: 'symbolicrdiv_delta_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-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 | ||
| } | ||
| ;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line: | ||
| ; CHECK: {{.*}} | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,125 @@ | ||||||||||||||||||
| ; 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 \ | ||||||||||||||||||
| ; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-ALL | ||||||||||||||||||
| ; RUN: opt < %s -disable-output "-passes=print<da>" -da-enable-dependence-test=weak-crossing-siv 2>&1 \ | ||||||||||||||||||
| ; RUN: | FileCheck %s --check-prefixes=CHECK,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) | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, However, I see the point, we can/should leave it to be on the safe side.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is |
||||||||||||||||||
| ; A[3*i - 2] = 1; | ||||||||||||||||||
| ; } | ||||||||||||||||||
| ; | ||||||||||||||||||
| ; FIXME: DependencyAnalsysis currently detects no dependency between | ||||||||||||||||||
kasuga-fj marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||
| ; `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 | ||||||||||||||||||
|
||||||||||||||||||
| ; 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.
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.