[llvm] Bug fixes for Array Dependence Analysis (PR #116389)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 15 22:27:49 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
Author: Sebastian Pop (sebpop)
<details>
<summary>Changes</summary>
The Array Dependence Analysis (DA) was built based on several assumptions (mostly coming from properties guaranteed by the Fortran language.) DA needs to conservatively answer "don't know" when the assumptions are not met. This patch series adds code to detect some of the assumptions.
The first patch [[DA] disambiguate evolution of base addresses](https://github.com/llvm/llvm-project/commit/6683b55fbff149a001bb02a8a88645eecbcdf0ca) ensures that the base address is invariant in DA. The patch uses SCEV analysis. This fixes two bugs: https://github.com/llvm/llvm-project/issues/41488 and https://github.com/llvm/llvm-project/issues/53942
The second patch [[DA] Dependence analysis does not handle array accesses of different sizes](https://github.com/llvm/llvm-project/commit/503da3b7f9b371f87b274580916101ba7546d2f2) makes sure all memory accesses have the same byte width. This fixes bug https://github.com/llvm/llvm-project/issues/16183
The third patch [[DA] add testcase](https://github.com/llvm/llvm-project/commit/0755bb4953cef9a9a380a01d57b7bff3af811d6f) adds a missing test for a bug that seems to be already fixed: https://github.com/llvm/llvm-project/issues/31196
The fourth patch [[DA] use NSW arithmetic](https://github.com/llvm/llvm-project/commit/f729a6c84fbcf88c088bbc958208ce525bdb9e12) fixes https://github.com/llvm/llvm-project/issues/51512 where the constraints solver in DA was using wrap-around instead of NSW arithmetic.
The patch series was tested on arm64-linux.
---
Full diff: https://github.com/llvm/llvm-project/pull/116389.diff
5 Files Affected:
- (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (+69-27)
- (added) llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll (+15)
- (added) llvm/test/Analysis/DependenceAnalysis/FlipFlopBaseAddress.ll (+136)
- (added) llvm/test/Analysis/DependenceAnalysis/PR31848.ll (+67)
- (added) llvm/test/Analysis/DependenceAnalysis/PR51512.ll (+34)
``````````diff
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index a4a98ea0bae146..5e1aa523923b02 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -712,10 +712,66 @@ void Dependence::dump(raw_ostream &OS) const {
// tbaa, non-overlapping regions etc), then it is known there is no dependecy.
// Otherwise the underlying objects are checked to see if they point to
// different identifiable objects.
-static AliasResult underlyingObjectsAlias(AAResults *AA,
- const DataLayout &DL,
- const MemoryLocation &LocA,
- const MemoryLocation &LocB) {
+static AliasResult underlyingObjectsAlias(AAResults *AA, LoopInfo *LI,
+ ScalarEvolution *SE, Instruction *A,
+ Instruction *B) {
+ const MemoryLocation &LocA = MemoryLocation::get(A);
+ const MemoryLocation &LocB = MemoryLocation::get(B);
+
+ // Check the underlying objects are the same
+ const Value *AObj = getUnderlyingObject(LocA.Ptr);
+ const Value *BObj = getUnderlyingObject(LocB.Ptr);
+
+ if (AObj == BObj) {
+ // The dependence test gets confused if the size of the memory accesses
+ // differ.
+ if (LocA.Size != LocB.Size)
+ return AliasResult::MayAlias;
+
+ // If the underlying objects are the same, they must alias.
+ return AliasResult::MustAlias;
+ }
+
+ if (auto *APhi = dyn_cast<PHINode>(AObj)) {
+ if (auto *BPhi = dyn_cast<PHINode>(BObj)) {
+ Loop *ALoop = LI->getLoopFor(APhi->getParent());
+ Loop *BLoop = LI->getLoopFor(BPhi->getParent());
+ if (ALoop == BLoop) {
+ auto *SCEVa = SE->getSCEV(const_cast<Value *>(AObj));
+ auto *SCEVb = SE->getSCEV(const_cast<Value *>(BObj));
+
+ // If the SCEVs are the same, they must alias.
+ if (SCEVa == SCEVb)
+ return AliasResult::MustAlias;
+
+ // If SCEV cannot analyze one of the values, then they may alias.
+ if (isa<SCEVUnknown>(SCEVa) || isa<SCEVUnknown>(SCEVb))
+ return AliasResult::MayAlias;
+
+ // Check whether the start values alias.
+ const SCEV *StartA = SCEVa;
+ while (const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(StartA))
+ StartA = AR->getStart();
+
+ const SCEV *StartB = SCEVb;
+ while (const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(StartB))
+ StartB = AR->getStart();
+
+ if (const SCEVUnknown *UA = dyn_cast<SCEVUnknown>(StartA)) {
+ if (const SCEVUnknown *UB = dyn_cast<SCEVUnknown>(StartB)) {
+ MemoryLocation LocAS =
+ MemoryLocation::getBeforeOrAfter(UA->getValue());
+ MemoryLocation LocBS =
+ MemoryLocation::getBeforeOrAfter(UB->getValue());
+ if (AA->isNoAlias(LocAS, LocBS))
+ return AliasResult::NoAlias;
+ }
+ }
+ return AliasResult::MayAlias;
+ }
+ }
+ }
+
// Check the original locations (minus size) for noalias, which can happen for
// tbaa, incompatible underlying object locations, etc.
MemoryLocation LocAS =
@@ -725,14 +781,6 @@ static AliasResult underlyingObjectsAlias(AAResults *AA,
if (AA->isNoAlias(LocAS, LocBS))
return AliasResult::NoAlias;
- // Check the underlying objects are the same
- const Value *AObj = getUnderlyingObject(LocA.Ptr);
- const Value *BObj = getUnderlyingObject(LocB.Ptr);
-
- // If the underlying objects are the same, they must alias
- if (AObj == BObj)
- return AliasResult::MustAlias;
-
// We may have hit the recursion limit for underlying objects, or have
// underlying objects where we don't know they will alias.
if (!isIdentifiedObject(AObj) || !isIdentifiedObject(BObj))
@@ -743,7 +791,6 @@ static AliasResult underlyingObjectsAlias(AAResults *AA,
return AliasResult::NoAlias;
}
-
// Returns true if the load or store can be analyzed. Atomic and volatile
// operations have properties which this analysis does not understand.
static
@@ -3116,11 +3163,9 @@ const SCEV *DependenceInfo::addToCoefficient(const SCEV *Expr,
const Loop *TargetLoop,
const SCEV *Value) const {
const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(Expr);
- if (!AddRec) // create a new addRec
- return SE->getAddRecExpr(Expr,
- Value,
- TargetLoop,
- SCEV::FlagAnyWrap); // Worst case, with no info.
+ if (!AddRec)
+ return SE->getAddRecExpr(Expr, Value, TargetLoop, SCEV::FlagNSW);
+
if (AddRec->getLoop() == TargetLoop) {
const SCEV *Sum = SE->getAddExpr(AddRec->getStepRecurrence(*SE), Value);
if (Sum->isZero())
@@ -3131,7 +3176,7 @@ const SCEV *DependenceInfo::addToCoefficient(const SCEV *Expr,
AddRec->getNoWrapFlags());
}
if (SE->isLoopInvariant(AddRec, TargetLoop))
- return SE->getAddRecExpr(AddRec, Value, TargetLoop, SCEV::FlagAnyWrap);
+ return SE->getAddRecExpr(AddRec, Value, TargetLoop, SCEV::FlagNSW);
return SE->getAddRecExpr(
addToCoefficient(AddRec->getStart(), TargetLoop, Value),
AddRec->getStepRecurrence(*SE), AddRec->getLoop(),
@@ -3606,9 +3651,7 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
Value *SrcPtr = getLoadStorePointerOperand(Src);
Value *DstPtr = getLoadStorePointerOperand(Dst);
- switch (underlyingObjectsAlias(AA, F->getDataLayout(),
- MemoryLocation::get(Dst),
- MemoryLocation::get(Src))) {
+ switch (underlyingObjectsAlias(AA, LI, SE, Dst, Src)) {
case AliasResult::MayAlias:
case AliasResult::PartialAlias:
// cannot analyse objects if we don't understand their aliasing.
@@ -4030,11 +4073,8 @@ const SCEV *DependenceInfo::getSplitIteration(const Dependence &Dep,
assert(Dst->mayReadFromMemory() || Dst->mayWriteToMemory());
assert(isLoadOrStore(Src));
assert(isLoadOrStore(Dst));
- Value *SrcPtr = getLoadStorePointerOperand(Src);
- Value *DstPtr = getLoadStorePointerOperand(Dst);
- assert(underlyingObjectsAlias(
- AA, F->getDataLayout(), MemoryLocation::get(Dst),
- MemoryLocation::get(Src)) == AliasResult::MustAlias);
+ assert(underlyingObjectsAlias(AA, LI, SE, Dst, Src) ==
+ AliasResult::MustAlias);
// establish loop nesting levels
establishNestingLevels(Src, Dst);
@@ -4043,6 +4083,8 @@ const SCEV *DependenceInfo::getSplitIteration(const Dependence &Dep,
unsigned Pairs = 1;
SmallVector<Subscript, 2> Pair(Pairs);
+ Value *SrcPtr = getLoadStorePointerOperand(Src);
+ Value *DstPtr = getLoadStorePointerOperand(Dst);
const SCEV *SrcSCEV = SE->getSCEV(SrcPtr);
const SCEV *DstSCEV = SE->getSCEV(DstPtr);
Pair[0].Src = SrcSCEV;
diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll
new file mode 100644
index 00000000000000..4d5da005ea437b
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll
@@ -0,0 +1,15 @@
+; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa 2>&1 \
+; RUN: | FileCheck %s
+
+; The dependence test does not handle array accesses of different sizes: i32 and i64.
+; Bug 16183 - https://github.com/llvm/llvm-project/issues/16183
+; CHECK-LABEL: bug16183_alias
+; CHECK: da analyze - confused!
+
+define i64 @bug16183_alias(i32* nocapture %A) {
+entry:
+ %arrayidx = getelementptr inbounds i32, ptr %A, i64 1
+ store i32 2, ptr %arrayidx, align 4
+ %0 = load i64, ptr %A, align 8
+ ret i64 %0
+}
diff --git a/llvm/test/Analysis/DependenceAnalysis/FlipFlopBaseAddress.ll b/llvm/test/Analysis/DependenceAnalysis/FlipFlopBaseAddress.ll
new file mode 100644
index 00000000000000..2d80b2493a389b
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/FlipFlopBaseAddress.ll
@@ -0,0 +1,136 @@
+; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa 2>&1 \
+; RUN: | FileCheck %s
+
+; Check that dependence analysis correctly handles flip-flop of base addresses.
+; Bug 41488 - https://github.com/llvm/llvm-project/issues/41488
+
+; CHECK-LABEL: bug41488_test1
+; CHECK-NOT: da analyze - none!
+
+define float @bug41488_test1() {
+entry:
+ %g = alloca float, align 4
+ %h = alloca float, align 4
+ br label %for.body
+
+for.body:
+ %p = phi float* [ %g, %entry ], [ %q, %for.body ]
+ %q = phi float* [ %h, %entry ], [ %p, %for.body ]
+ %0 = load float, float* %p, align 4
+ store float undef, float* %q, align 4
+ %branch_cond = fcmp ugt float %0, 0.0
+ br i1 %branch_cond, label %for.cond.cleanup, label %for.body
+
+for.cond.cleanup:
+ ret float undef
+}
+
+; CHECK-LABEL: bug41488_test2
+; CHECK-NOT: da analyze - none!
+
+define void @bug41488_test2(i32 %n) {
+entry:
+ %g = alloca float, align 4
+ %h = alloca float, align 4
+ br label %for.body
+
+for.body:
+ %i = phi i32 [0, %entry ], [ %inc, %for.body ]
+ %p = phi float* [ %g, %entry ], [ %q, %for.body ]
+ %q = phi float* [ %h, %entry ], [ %p, %for.body ]
+ %0 = load float, float* %p, align 4
+ store float 0.0, float* %q, align 4
+ %inc = add nuw i32 %i, 1
+ %branch_cond = icmp ult i32 %i, %n
+ br i1 %branch_cond, label %for.body, label %for.cond.cleanup
+
+for.cond.cleanup:
+ ret void
+}
+
+; Bug 53942 - https://github.com/llvm/llvm-project/issues/53942
+; CHECK-LABEL: bug53942_foo
+; CHECK-NOT: da analyze - none!
+
+define void @bug53942_foo(i32 noundef %n, ptr noalias nocapture noundef writeonly %A, ptr noalias nocapture noundef %B) {
+entry:
+ %cmp8 = icmp sgt i32 %n, 1
+ br i1 %cmp8, label %for.body.preheader, label %for.cond.cleanup
+
+for.body.preheader: ; preds = %entry
+ %wide.trip.count = zext nneg i32 %n to i64
+ br label %for.body
+
+for.cond.cleanup: ; preds = %for.body, %entry
+ ret void
+
+for.body: ; preds = %for.body.preheader, %for.body
+ %indvars.iv = phi i64 [ 1, %for.body.preheader ], [ %indvars.iv.next, %for.body ]
+ %ptr1.011 = phi ptr [ %A, %for.body.preheader ], [ %ptr2.09, %for.body ]
+ %ptr2.09 = phi ptr [ %B, %for.body.preheader ], [ %ptr1.011, %for.body ]
+ %.pre = load double, ptr %B, align 8
+ %arrayidx2 = getelementptr inbounds double, ptr %ptr1.011, i64 %indvars.iv
+ store double %.pre, ptr %arrayidx2, align 8
+ %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+ %exitcond.not = icmp eq i64 %indvars.iv.next, %wide.trip.count
+ br i1 %exitcond.not, label %for.cond.cleanup, label %for.body
+}
+
+
+; Bug 53942 - https://github.com/llvm/llvm-project/issues/53942
+; CHECK-LABEL: bug53942_bar
+; CHECK-NOT: da analyze - none!
+
+define void @bug53942_bar(i32 noundef %n, ptr noalias noundef %A, ptr noalias noundef %B) {
+entry:
+ br label %for.cond
+
+for.cond: ; preds = %for.inc, %entry
+ %i.0 = phi i32 [ 1, %entry ], [ %inc, %for.inc ]
+ %cmp = icmp slt i32 %i.0, %n
+ br i1 %cmp, label %for.body, label %for.cond.cleanup
+
+for.cond.cleanup: ; preds = %for.cond
+ br label %for.end
+
+for.body: ; preds = %for.cond
+ %and = and i32 %i.0, 2
+ %tobool.not = icmp eq i32 %and, 0
+ br i1 %tobool.not, label %cond.false, label %cond.true
+
+cond.true: ; preds = %for.body
+ br label %cond.end
+
+cond.false: ; preds = %for.body
+ br label %cond.end
+
+cond.end: ; preds = %cond.false, %cond.true
+ %cond = phi ptr [ %A, %cond.true ], [ %B, %cond.false ]
+ %and1 = and i32 %i.0, 2
+ %tobool2.not = icmp eq i32 %and1, 0
+ br i1 %tobool2.not, label %cond.false4, label %cond.true3
+
+cond.true3: ; preds = %cond.end
+ br label %cond.end5
+
+cond.false4: ; preds = %cond.end
+ br label %cond.end5
+
+cond.end5: ; preds = %cond.false4, %cond.true3
+ %cond6 = phi ptr [ %B, %cond.true3 ], [ %A, %cond.false4 ]
+ %sub = add nsw i32 %i.0, -1
+ %idxprom = sext i32 %sub to i64
+ %arrayidx = getelementptr inbounds double, ptr %cond6, i64 %idxprom
+ %0 = load double, ptr %arrayidx, align 8
+ %idxprom7 = zext nneg i32 %i.0 to i64
+ %arrayidx8 = getelementptr inbounds double, ptr %cond, i64 %idxprom7
+ store double %0, ptr %arrayidx8, align 8
+ br label %for.inc
+
+for.inc: ; preds = %cond.end5
+ %inc = add nuw nsw i32 %i.0, 1
+ br label %for.cond
+
+for.end: ; preds = %for.cond.cleanup
+ ret void
+}
diff --git a/llvm/test/Analysis/DependenceAnalysis/PR31848.ll b/llvm/test/Analysis/DependenceAnalysis/PR31848.ll
new file mode 100644
index 00000000000000..02e7c54f9e941d
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/PR31848.ll
@@ -0,0 +1,67 @@
+; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa 2>&1 \
+; RUN: | FileCheck %s
+
+define void @foo(i32* nocapture %A, i32 %n) {
+entry:
+ %n.cmp = icmp sgt i32 %n, 0
+ br i1 %n.cmp, label %for.j.header, label %exit
+
+for.j.header:
+ %j= phi i32 [ %j.inc, %for.j.latch ], [ 0, %entry ]
+ br label %for.i.header
+
+for.i.header:
+ %i = phi i32 [ %i.inc, %for.i.latch ], [ 0, %for.j.header ]
+ br label %for.di.header
+
+for.di.header:
+ %di = phi i32 [ -1, %for.i.header ], [ %di.inc, %for.di.latch ]
+ %di.add = add nsw i32 %di, %i
+ br label %for.dj.header
+
+for.dj.header:
+ %dj = phi i32 [ -1, %for.di.header ], [ %dj.inc, %body ]
+ %dj.add = add nsw i32 %dj, %j
+ br label %while.x
+
+while.x:
+ %x = phi i32 [ %di.add, %for.dj.header ], [ %x.inc, %while.x ]
+ %x.cmp = icmp slt i32 %x, 0
+ %x.inc = add nsw i32 %x, %n
+ br i1 %x.cmp, label %while.x, label %while.y
+
+while.y:
+ %y = phi i32 [ %y.inc, %while.y ], [ %dj.add, %while.x ]
+ %y.cmp = icmp slt i32 %y, 0
+ %y.inc = add nsw i32 %y, %n
+ br i1 %y.cmp, label %while.y, label %body
+
+body:
+ %mul = mul nsw i32 %y, %n
+ %add = add nsw i32 %mul, %x
+ %idxprom = sext i32 %add to i64
+ %arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom
+; CHECK: da analyze - output [* * * *]
+ store i32 7, i32* %arrayidx, align 4
+ %dj.inc = add nsw i32 %dj, 1
+ %dj.exitcond = icmp eq i32 %dj.inc, 2
+ br i1 %dj.exitcond, label %for.di.latch, label %for.dj.header
+
+for.di.latch:
+ %di.inc = add nsw i32 %di, 1
+ %di.exitcond = icmp eq i32 %di.inc, 2
+ br i1 %di.exitcond, label %for.i.latch, label %for.di.header
+
+for.i.latch:
+ %i.inc = add nuw nsw i32 %i, 1
+ %i.exitcond = icmp eq i32 %i.inc, %n
+ br i1 %i.exitcond, label %for.j.latch, label %for.i.header
+
+for.j.latch:
+ %j.inc = add nuw nsw i32 %j, 1
+ %j.exitcond = icmp eq i32 %j.inc, %n
+ br i1 %j.exitcond, label %exit, label %for.j.header
+
+exit:
+ ret void
+}
diff --git a/llvm/test/Analysis/DependenceAnalysis/PR51512.ll b/llvm/test/Analysis/DependenceAnalysis/PR51512.ll
new file mode 100644
index 00000000000000..52419adff8f8b5
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/PR51512.ll
@@ -0,0 +1,34 @@
+; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa
+
+; Check that the testcase does not crash the compiler.
+; https://github.com/llvm/llvm-project/issues/51512
+
+define void @func_1() {
+entry:
+ %l_83.i.i = alloca [2 x [5 x i32]], align 1
+ br label %for.cond857.preheader.i.i
+
+for.cond857.preheader.i.i: ; preds = %cleanup.cont1138.i.i, %entry
+ %l_89.08.i.i = phi i32 [ 0, %entry ], [ %add1140.i.i, %cleanup.cont1138.i.i ]
+ %0 = trunc i32 %l_89.08.i.i to i16
+ %1 = add i16 %0, 3
+ %arrayidx916.i.i = getelementptr inbounds [2 x [5 x i32]], [2 x [5 x i32]]* %l_83.i.i, i16 0, i16 %0, i16 %1
+ br label %for.body860.i.i
+
+for.body860.i.i: ; preds = %for.body860.i.i, %for.cond857.preheader.i.i
+ %l_74.07.i.i = phi i32 [ 0, %for.cond857.preheader.i.i ], [ %add964.i.i, %for.body860.i.i ]
+ store i32 undef, i32* %arrayidx916.i.i, align 1
+ %2 = trunc i32 %l_74.07.i.i to i16
+ %arrayidx962.i.i = getelementptr inbounds [2 x [5 x i32]], [2 x [5 x i32]]* %l_83.i.i, i16 0, i16 %2, i16 %1
+ store i32 0, i32* %arrayidx962.i.i, align 1
+ %add964.i.i = add nuw nsw i32 %l_74.07.i.i, 1
+ br i1 false, label %for.body860.i.i, label %cleanup.cont1138.i.i
+
+cleanup.cont1138.i.i: ; preds = %for.body860.i.i
+ %add1140.i.i = add nuw nsw i32 %l_89.08.i.i, 1
+ %cmp602.i.i = icmp eq i32 %l_89.08.i.i, 0
+ br i1 %cmp602.i.i, label %for.cond857.preheader.i.i, label %for.cond1480.i.i.preheader
+
+for.cond1480.i.i.preheader: ; preds = %cleanup.cont1138.i.i
+ unreachable
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/116389
More information about the llvm-commits
mailing list