Skip to content

[DA] Dependence analysis does not handle array accesses of different sizes #116630

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 22 additions & 16 deletions llvm/lib/Analysis/DependenceAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3604,14 +3604,10 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
return std::make_unique<Dependence>(Src, Dst);
}

assert(isLoadOrStore(Src) && "instruction is not load or store");
assert(isLoadOrStore(Dst) && "instruction is not load or store");
Value *SrcPtr = getLoadStorePointerOperand(Src);
Value *DstPtr = getLoadStorePointerOperand(Dst);
const MemoryLocation &DstLoc = MemoryLocation::get(Dst);
const MemoryLocation &SrcLoc = MemoryLocation::get(Src);

switch (underlyingObjectsAlias(AA, F->getDataLayout(),
MemoryLocation::get(Dst),
MemoryLocation::get(Src))) {
switch (underlyingObjectsAlias(AA, F->getDataLayout(), DstLoc, SrcLoc)) {
case AliasResult::MayAlias:
case AliasResult::PartialAlias:
// cannot analyse objects if we don't understand their aliasing.
Expand All @@ -3625,16 +3621,15 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
break; // The underlying objects alias; test accesses for dependence.
}

// establish loop nesting levels
establishNestingLevels(Src, Dst);
LLVM_DEBUG(dbgs() << " common nesting levels = " << CommonLevels << "\n");
LLVM_DEBUG(dbgs() << " maximum nesting levels = " << MaxLevels << "\n");

FullDependence Result(Src, Dst, PossiblyLoopIndependent, CommonLevels);
++TotalArrayPairs;
if (DstLoc.Size != SrcLoc.Size) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need to check if they are precise or not (with calling isPrecise)?

// The dependence test gets confused if the size of the memory accesses
// differ.
LLVM_DEBUG(dbgs() << "can't analyze must alias with different sizes\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Only "must alias"?

return std::make_unique<Dependence>(Src, Dst);
}

unsigned Pairs = 1;
SmallVector<Subscript, 2> Pair(Pairs);
Value *SrcPtr = getLoadStorePointerOperand(Src);
Value *DstPtr = getLoadStorePointerOperand(Dst);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use const Value *SrcPtr = LocA.Value; here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I can't use const here: SE->getSCEV takes a Value*:

const llvm::SCEV* llvm::ScalarEvolution::getSCEV(llvm::Value*)

const SCEV *SrcSCEV = SE->getSCEV(SrcPtr);
const SCEV *DstSCEV = SE->getSCEV(DstPtr);
LLVM_DEBUG(dbgs() << " SrcSCEV = " << *SrcSCEV << "\n");
Expand All @@ -3649,6 +3644,17 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
LLVM_DEBUG(dbgs() << "can't analyze SCEV with different pointer base\n");
return std::make_unique<Dependence>(Src, Dst);
}

// establish loop nesting levels
establishNestingLevels(Src, Dst);
LLVM_DEBUG(dbgs() << " common nesting levels = " << CommonLevels << "\n");
LLVM_DEBUG(dbgs() << " maximum nesting levels = " << MaxLevels << "\n");

FullDependence Result(Src, Dst, PossiblyLoopIndependent, CommonLevels);
++TotalArrayPairs;

unsigned Pairs = 1;
SmallVector<Subscript, 2> Pair(Pairs);
Pair[0].Src = SrcSCEV;
Pair[0].Dst = DstSCEV;

Expand Down
22 changes: 22 additions & 0 deletions llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
; 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

; The dependence test does not handle array accesses of different sizes: i32 and i64.
; Bug 16183 - https://github.com/llvm/llvm-project/issues/16183

define i64 @bug16183_alias(ptr nocapture %A) {
; CHECK-LABEL: 'bug16183_alias'
; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 4 --> Dst: store i32 2, ptr %arrayidx, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 4 --> Dst: %0 = load i64, ptr %A, align 8
; CHECK-NEXT: da analyze - confused!
; CHECK-NEXT: Src: %0 = load i64, ptr %A, align 8 --> Dst: %0 = load i64, ptr %A, align 8
; CHECK-NEXT: da analyze - none!
;
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
}