-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
Changes from all commits
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 |
---|---|---|
|
@@ -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. | ||
|
@@ -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) { | ||
// 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"); | ||
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. 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); | ||
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. Can you use 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. No, I can't use const here:
|
||
const SCEV *SrcSCEV = SE->getSCEV(SrcPtr); | ||
const SCEV *DstSCEV = SE->getSCEV(DstPtr); | ||
LLVM_DEBUG(dbgs() << " SrcSCEV = " << *SrcSCEV << "\n"); | ||
|
@@ -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; | ||
|
||
|
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 | ||
} |
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.
Don't we need to check if they are precise or not (with calling
isPrecise
)?