Skip to content

Commit dbbacb9

Browse files
kasuga-fjJaddyen
kasuga-fj
authored and
Jaddyen
committed
[DA] Add check for base pointer invariance (llvm#148241)
As specified in llvm#53942, DA assumes base pointer invariance in its process. Some cases were fixed by llvm#116628. However, that PR only addressed the parts related to AliasAnalysis, so the original issue persists in later stages, especially when the AliasAnalysis results in `MustAlias`. This patch insert an explicit loop-invariant checks for the base pointer and skips analysis when it is not loop-invariant. Fix the cases added in llvm#148240.
1 parent 4ecbf61 commit dbbacb9

File tree

8 files changed

+96
-24
lines changed

8 files changed

+96
-24
lines changed

‎llvm/lib/Analysis/DependenceAnalysis.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3383,6 +3383,10 @@ bool DependenceInfo::tryDelinearize(Instruction *Src, Instruction *Dst,
33833383
SrcSubscripts, DstSubscripts))
33843384
return false;
33853385

3386+
assert(isLoopInvariant(SrcBase, SrcLoop) &&
3387+
isLoopInvariant(DstBase, DstLoop) &&
3388+
"Expected SrcBase and DstBase to be loop invariant");
3389+
33863390
int Size = SrcSubscripts.size();
33873391
LLVM_DEBUG({
33883392
dbgs() << "\nSrcSubscripts: ";
@@ -3666,6 +3670,19 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
36663670
SCEVUnionPredicate(Assume, *SE));
36673671
}
36683672

3673+
// Even if the base pointers are the same, they may not be loop-invariant. It
3674+
// could lead to incorrect results, as we're analyzing loop-carried
3675+
// dependencies. Src and Dst can be in different loops, so we need to check
3676+
// the base pointer is invariant in both loops.
3677+
Loop *SrcLoop = LI->getLoopFor(Src->getParent());
3678+
Loop *DstLoop = LI->getLoopFor(Dst->getParent());
3679+
if (!isLoopInvariant(SrcBase, SrcLoop) ||
3680+
!isLoopInvariant(DstBase, DstLoop)) {
3681+
LLVM_DEBUG(dbgs() << "The base pointer is not loop invariant.\n");
3682+
return std::make_unique<Dependence>(Src, Dst,
3683+
SCEVUnionPredicate(Assume, *SE));
3684+
}
3685+
36693686
uint64_t EltSize = SrcLoc.Size.toRaw();
36703687
const SCEV *SrcEv = SE->getMinusSCEV(SrcSCEV, SrcBase);
36713688
const SCEV *DstEv = SE->getMinusSCEV(DstSCEV, DstBase);

‎llvm/test/Analysis/DependenceAnalysis/Banerjee.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ define void @banerjee1(ptr %A, ptr %B, i64 %m, i64 %n) nounwind uwtable ssp {
113113
; CHECK-NEXT: Src: %2 = load i64, ptr %arrayidx6, align 8 --> Dst: store i64 %2, ptr %B.addr.12, align 8
114114
; CHECK-NEXT: da analyze - confused!
115115
; CHECK-NEXT: Src: store i64 %2, ptr %B.addr.12, align 8 --> Dst: store i64 %2, ptr %B.addr.12, align 8
116-
; CHECK-NEXT: da analyze - output [* *]!
116+
; CHECK-NEXT: da analyze - confused!
117117
;
118118
; NORMALIZE-LABEL: 'banerjee1'
119119
; NORMALIZE-NEXT: Src: store i64 0, ptr %arrayidx, align 8 --> Dst: store i64 0, ptr %arrayidx, align 8
@@ -127,7 +127,7 @@ define void @banerjee1(ptr %A, ptr %B, i64 %m, i64 %n) nounwind uwtable ssp {
127127
; NORMALIZE-NEXT: Src: %2 = load i64, ptr %arrayidx6, align 8 --> Dst: store i64 %2, ptr %B.addr.12, align 8
128128
; NORMALIZE-NEXT: da analyze - confused!
129129
; NORMALIZE-NEXT: Src: store i64 %2, ptr %B.addr.12, align 8 --> Dst: store i64 %2, ptr %B.addr.12, align 8
130-
; NORMALIZE-NEXT: da analyze - output [* *]!
130+
; NORMALIZE-NEXT: da analyze - confused!
131131
;
132132
; DELIN-LABEL: 'banerjee1'
133133
; DELIN-NEXT: Src: store i64 0, ptr %arrayidx, align 8 --> Dst: store i64 0, ptr %arrayidx, align 8
@@ -141,7 +141,7 @@ define void @banerjee1(ptr %A, ptr %B, i64 %m, i64 %n) nounwind uwtable ssp {
141141
; DELIN-NEXT: Src: %2 = load i64, ptr %arrayidx6, align 8 --> Dst: store i64 %2, ptr %B.addr.12, align 8
142142
; DELIN-NEXT: da analyze - confused!
143143
; DELIN-NEXT: Src: store i64 %2, ptr %B.addr.12, align 8 --> Dst: store i64 %2, ptr %B.addr.12, align 8
144-
; DELIN-NEXT: da analyze - output [* *]!
144+
; DELIN-NEXT: da analyze - confused!
145145
;
146146
entry:
147147
%cmp4 = icmp sgt i64 %n, 0

‎llvm/test/Analysis/DependenceAnalysis/FlipFlopBaseAddress.ll

Lines changed: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
define float @bug41488_test1(float %f) {
99
; CHECK-LABEL: 'bug41488_test1'
1010
; CHECK-NEXT: Src: %0 = load float, ptr %p, align 4 --> Dst: %0 = load float, ptr %p, align 4
11-
; CHECK-NEXT: da analyze - input [*]!
11+
; CHECK-NEXT: da analyze - confused!
1212
; CHECK-NEXT: Src: %0 = load float, ptr %p, align 4 --> Dst: store float %f, ptr %q, align 4
1313
; CHECK-NEXT: da analyze - confused!
1414
; CHECK-NEXT: Src: store float %f, ptr %q, align 4 --> Dst: store float %f, ptr %q, align 4
15-
; CHECK-NEXT: da analyze - output [*]!
15+
; CHECK-NEXT: da analyze - confused!
1616
;
1717
entry:
1818
%g = alloca float, align 4
@@ -34,11 +34,11 @@ for.cond.cleanup:
3434
define void @bug41488_test2(i32 %n) {
3535
; CHECK-LABEL: 'bug41488_test2'
3636
; CHECK-NEXT: Src: %0 = load float, ptr %p, align 4 --> Dst: %0 = load float, ptr %p, align 4
37-
; CHECK-NEXT: da analyze - input [*]!
37+
; CHECK-NEXT: da analyze - confused!
3838
; CHECK-NEXT: Src: %0 = load float, ptr %p, align 4 --> Dst: store float 0.000000e+00, ptr %q, align 4
3939
; CHECK-NEXT: da analyze - confused!
4040
; CHECK-NEXT: Src: store float 0.000000e+00, ptr %q, align 4 --> Dst: store float 0.000000e+00, ptr %q, align 4
41-
; CHECK-NEXT: da analyze - output [*]!
41+
; CHECK-NEXT: da analyze - confused!
4242
;
4343
entry:
4444
%g = alloca float, align 4
@@ -68,7 +68,7 @@ define void @bug53942_foo(i32 noundef %n, ptr noalias nocapture noundef writeonl
6868
; CHECK-NEXT: Src: %.pre = load double, ptr %B, align 8 --> Dst: store double %.pre, ptr %arrayidx2, align 8
6969
; CHECK-NEXT: da analyze - confused!
7070
; CHECK-NEXT: Src: store double %.pre, ptr %arrayidx2, align 8 --> Dst: store double %.pre, ptr %arrayidx2, align 8
71-
; CHECK-NEXT: da analyze - output [*]!
71+
; CHECK-NEXT: da analyze - confused!
7272
;
7373
entry:
7474
%cmp8 = icmp sgt i32 %n, 1
@@ -99,11 +99,11 @@ for.body: ; preds = %for.body.preheader,
9999
define void @bug53942_bar(i32 noundef %n, ptr noalias noundef %A, ptr noalias noundef %B) {
100100
; CHECK-LABEL: 'bug53942_bar'
101101
; CHECK-NEXT: Src: %0 = load double, ptr %arrayidx, align 8 --> Dst: %0 = load double, ptr %arrayidx, align 8
102-
; CHECK-NEXT: da analyze - input [*]!
102+
; CHECK-NEXT: da analyze - confused!
103103
; CHECK-NEXT: Src: %0 = load double, ptr %arrayidx, align 8 --> Dst: store double %0, ptr %arrayidx8, align 8
104104
; CHECK-NEXT: da analyze - confused!
105105
; CHECK-NEXT: Src: store double %0, ptr %arrayidx8, align 8 --> Dst: store double %0, ptr %arrayidx8, align 8
106-
; CHECK-NEXT: da analyze - output [*]!
106+
; CHECK-NEXT: da analyze - confused!
107107
;
108108
entry:
109109
br label %for.cond
@@ -166,14 +166,14 @@ for.end: ; preds = %for.cond.cleanup
166166
; (j % 2 == 0 ? A[i][j] : A[i][j+1]) = 1;
167167
; }
168168
;
169-
; FIXME: There are loop-carried dependencies between the store instruction. For
169+
; There are loop-carried dependencies between the store instruction. For
170170
; example, the value of %ptr0 when (i, j) = (0, 1) is %A+8, which is the same
171171
; as when (i, j) = (0, 2).
172172

173173
define void @non_invariant_baseptr_with_identical_obj(ptr %A) {
174174
; CHECK-LABEL: 'non_invariant_baseptr_with_identical_obj'
175175
; CHECK-NEXT: Src: store i32 1, ptr %idx, align 4 --> Dst: store i32 1, ptr %idx, align 4
176-
; CHECK-NEXT: da analyze - none!
176+
; CHECK-NEXT: da analyze - confused!
177177
;
178178
entry:
179179
br label %loop.i.header
@@ -216,13 +216,13 @@ exit:
216216
; Similar to the above case, but ptr0 is loop-invariant with respsect to the
217217
; k-loop.
218218
;
219-
; FIXME: Same as the above case, there are loop-carried dependencies between
220-
; the store.
219+
; Same as the above case, there are loop-carried dependencies between the
220+
; store.
221221

222222
define void @non_invariant_baseptr_with_identical_obj2(ptr %A) {
223223
; CHECK-LABEL: 'non_invariant_baseptr_with_identical_obj2'
224224
; CHECK-NEXT: Src: store i32 1, ptr %idx, align 4 --> Dst: store i32 1, ptr %idx, align 4
225-
; CHECK-NEXT: da analyze - none!
225+
; CHECK-NEXT: da analyze - confused!
226226
;
227227
entry:
228228
br label %loop.i.header
@@ -259,3 +259,58 @@ loop.i.latch:
259259
exit:
260260
ret void
261261
}
262+
263+
; Pseudo-code that is approximately semantically equivalent to the below IR:
264+
;
265+
; void f(int A[][32]) {
266+
; for (int i = 0; i < 100; i++)
267+
; for (int j = 0; j < 15; j++) {
268+
; int offset = (j % 2 == 0) ? 1 : 0;
269+
; A[i][2 * j + offset + 0] = 1;
270+
; A[i][2 * j + offset + 1] = 1;
271+
; }
272+
; }
273+
;
274+
; There are loop-carried dependencies between the two stores. For example,
275+
; A[0][2] is accessed from both the former one when (i, j) = (0, 1) and the
276+
; latter one when (i, j) = (0, 0).
277+
;
278+
define void @non_invariant_baseptr_with_identical_obj3(ptr %A) {
279+
; CHECK-LABEL: 'non_invariant_baseptr_with_identical_obj3'
280+
; CHECK-NEXT: Src: store i32 1, ptr %idx0, align 4 --> Dst: store i32 1, ptr %idx0, align 4
281+
; CHECK-NEXT: da analyze - confused!
282+
; CHECK-NEXT: Src: store i32 1, ptr %idx0, align 4 --> Dst: store i32 1, ptr %idx1, align 4
283+
; CHECK-NEXT: da analyze - confused!
284+
; CHECK-NEXT: Src: store i32 1, ptr %idx1, align 4 --> Dst: store i32 1, ptr %idx1, align 4
285+
; CHECK-NEXT: da analyze - confused!
286+
;
287+
entry:
288+
br label %loop.i.header
289+
290+
loop.i.header:
291+
%i = phi i32 [ 0, %entry ], [ %i.inc, %loop.i.latch ]
292+
%A1 = getelementptr i32, ptr %A, i32 1
293+
br label %loop.j
294+
295+
loop.j:
296+
%j = phi i32 [ 0, %loop.i.header ], [ %j.inc, %loop.j ]
297+
%ptr0 = phi ptr [ %A1, %loop.i.header ], [ %ptr1, %loop.j ]
298+
%ptr1 = phi ptr [ %A, %loop.i.header ], [ %ptr0, %loop.j ]
299+
%j2_0 = shl i32 %j, 1
300+
%j2_1 = add i32 %j2_0, 1
301+
%idx0 = getelementptr [32 x i32], ptr %ptr0, i32 %i, i32 %j2_0
302+
%idx1 = getelementptr [32 x i32], ptr %ptr0, i32 %i, i32 %j2_1
303+
store i32 1, ptr %idx0
304+
store i32 1, ptr %idx1
305+
%j.inc = add i32 %j, 1
306+
%cmp.j = icmp slt i32 %j.inc, 15
307+
br i1 %cmp.j, label %loop.j, label %loop.i.latch
308+
309+
loop.i.latch:
310+
%i.inc = add i32 %i, 1
311+
%cmp.i = icmp slt i32 %i.inc, 100
312+
br i1 %cmp.i, label %loop.i.header, label %exit
313+
314+
exit:
315+
ret void
316+
}

‎llvm/test/Analysis/DependenceAnalysis/GCD.ll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ define void @gcd6(i64 %n, ptr %A, ptr %B) nounwind uwtable ssp {
398398
; CHECK-NEXT: Src: %2 = load i32, ptr %arrayidx9, align 4 --> Dst: store i32 %2, ptr %B.addr.12, align 4
399399
; CHECK-NEXT: da analyze - confused!
400400
; CHECK-NEXT: Src: store i32 %2, ptr %B.addr.12, align 4 --> Dst: store i32 %2, ptr %B.addr.12, align 4
401-
; CHECK-NEXT: da analyze - output [* *]!
401+
; CHECK-NEXT: da analyze - confused!
402402
;
403403
entry:
404404
%cmp4 = icmp sgt i64 %n, 0
@@ -475,7 +475,7 @@ define void @gcd7(i32 %n, ptr %A, ptr %B) nounwind uwtable ssp {
475475
; CHECK-NEXT: Src: %11 = load i32, ptr %arrayidx12, align 4 --> Dst: store i32 %11, ptr %B.addr.12, align 4
476476
; CHECK-NEXT: da analyze - confused!
477477
; CHECK-NEXT: Src: store i32 %11, ptr %B.addr.12, align 4 --> Dst: store i32 %11, ptr %B.addr.12, align 4
478-
; CHECK-NEXT: da analyze - output [* *]!
478+
; CHECK-NEXT: da analyze - confused!
479479
;
480480
entry:
481481
%0 = zext i32 %n to i64
@@ -566,7 +566,7 @@ define void @gcd8(i32 %n, ptr %A, ptr %B) nounwind uwtable ssp {
566566
; CHECK-NEXT: Src: %5 = load i32, ptr %arrayidx12, align 4 --> Dst: store i32 %5, ptr %B.addr.12, align 4
567567
; CHECK-NEXT: da analyze - confused!
568568
; CHECK-NEXT: Src: store i32 %5, ptr %B.addr.12, align 4 --> Dst: store i32 %5, ptr %B.addr.12, align 4
569-
; CHECK-NEXT: da analyze - output [* *]!
569+
; CHECK-NEXT: da analyze - confused!
570570
;
571571
entry:
572572
%cmp4 = icmp sgt i32 %n, 0
@@ -650,7 +650,7 @@ define void @gcd9(i32 %n, ptr %A, ptr %B) nounwind uwtable ssp {
650650
; CHECK-NEXT: Src: %11 = load i32, ptr %arrayidx12, align 4 --> Dst: store i32 %11, ptr %B.addr.12, align 4
651651
; CHECK-NEXT: da analyze - confused!
652652
; CHECK-NEXT: Src: store i32 %11, ptr %B.addr.12, align 4 --> Dst: store i32 %11, ptr %B.addr.12, align 4
653-
; CHECK-NEXT: da analyze - output [* *]!
653+
; CHECK-NEXT: da analyze - confused!
654654
;
655655
entry:
656656
%0 = zext i32 %n to i64

‎llvm/test/Analysis/DependenceAnalysis/NonAffineExpr.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ define void @f(ptr %a, i32 %n, i1 %arg) align 2 {
1212
; CHECK-NEXT: Src: %t.2 = load ptr, ptr %a, align 4 --> Dst: %t.4 = load i32, ptr %t.3, align 4
1313
; CHECK-NEXT: da analyze - confused!
1414
; CHECK-NEXT: Src: %t.4 = load i32, ptr %t.3, align 4 --> Dst: %t.4 = load i32, ptr %t.3, align 4
15-
; CHECK-NEXT: da analyze - input [* *]!
15+
; CHECK-NEXT: da analyze - confused!
1616
;
1717
for.preheader:
1818
%t.0 = ashr exact i32 %n, 3

‎llvm/test/Analysis/DependenceAnalysis/Preliminary.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ define void @p2(i64 %n, ptr %A, ptr %B) nounwind uwtable ssp {
6969
; CHECK-NEXT: Src: %0 = load i64, ptr %arrayidx17, align 8 --> Dst: store i64 %0, ptr %B.addr.24, align 8
7070
; CHECK-NEXT: da analyze - confused!
7171
; CHECK-NEXT: Src: store i64 %0, ptr %B.addr.24, align 8 --> Dst: store i64 %0, ptr %B.addr.24, align 8
72-
; CHECK-NEXT: da analyze - output [* * *]!
72+
; CHECK-NEXT: da analyze - confused!
7373
;
7474
entry:
7575
%cmp10 = icmp sgt i64 %n, 0

‎llvm/test/Analysis/DependenceAnalysis/PreliminaryNoValidityCheckFixedSize.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ define void @p2(i64 %n, ptr %A, ptr %B) nounwind uwtable ssp {
2828
; CHECK-NEXT: Src: %0 = load i64, ptr %arrayidx17, align 8 --> Dst: store i64 %0, ptr %B.addr.24, align 8
2929
; CHECK-NEXT: da analyze - confused!
3030
; CHECK-NEXT: Src: store i64 %0, ptr %B.addr.24, align 8 --> Dst: store i64 %0, ptr %B.addr.24, align 8
31-
; CHECK-NEXT: da analyze - output [* * *]!
31+
; CHECK-NEXT: da analyze - confused!
3232
;
3333
; LIN-LABEL: 'p2'
3434
; LIN-NEXT: Src: store i64 %i.011, ptr %arrayidx8, align 8 --> Dst: store i64 %i.011, ptr %arrayidx8, align 8
@@ -42,7 +42,7 @@ define void @p2(i64 %n, ptr %A, ptr %B) nounwind uwtable ssp {
4242
; LIN-NEXT: Src: %0 = load i64, ptr %arrayidx17, align 8 --> Dst: store i64 %0, ptr %B.addr.24, align 8
4343
; LIN-NEXT: da analyze - confused!
4444
; LIN-NEXT: Src: store i64 %0, ptr %B.addr.24, align 8 --> Dst: store i64 %0, ptr %B.addr.24, align 8
45-
; LIN-NEXT: da analyze - output [* * *]!
45+
; LIN-NEXT: da analyze - confused!
4646
;
4747
entry:
4848
%cmp10 = icmp sgt i64 %n, 0

‎llvm/test/Analysis/DependenceAnalysis/SymbolicRDIV.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ define void @symbolicrdiv6(ptr %A, ptr %B, i64 %n1, i64 %n2) nounwind uwtable ss
437437
; CHECK-NEXT: Src: %0 = load i32, ptr %arrayidx4, align 4 --> Dst: store i32 %0, ptr %B.addr.12, align 4
438438
; CHECK-NEXT: da analyze - confused!
439439
; CHECK-NEXT: Src: store i32 %0, ptr %B.addr.12, align 4 --> Dst: store i32 %0, ptr %B.addr.12, align 4
440-
; CHECK-NEXT: da analyze - output [* *]!
440+
; CHECK-NEXT: da analyze - confused!
441441
;
442442
entry:
443443
%cmp4 = icmp eq i64 %n1, 0

0 commit comments

Comments
 (0)