Skip to content

Commit

Permalink
[LAA] Consider unknown SCEVs defined in the loop as IndirectUnsafe.
Browse files Browse the repository at this point in the history
At the moment, access expressions may contain values that change across
loop iterations (like a load). That means they may be classified as
Unknown dependence and if we decide to re-try with runtime checks, the
runtime checks are not sufficient; the accessws with loop varying indices
may overlap with an earlier or subsequent iterations.

To address this, consider accesses containing SCEVUnknown with values
defined inside a loop as unsafe.

Fixes llvm#87189.
  • Loading branch information
fhahn committed Jul 16, 2024
1 parent 4199f80 commit 97efdc7
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 18 deletions.
19 changes: 17 additions & 2 deletions llvm/lib/Analysis/LoopAccessAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1909,6 +1909,19 @@ isLoopVariantIndirectAddress(ArrayRef<const Value *> UnderlyingObjects,
});
}

/// Returns true if \p Expr contains any SCEVUnknown defined in the loop.
/// Expressions containing SCEVUnknown may overlap with either earlier or later
/// iterations.
static bool hasLoopVariantUnknownIndex(const SCEV *Expr, const Loop *L) {
return SCEVExprContains(Expr, [L](const SCEV *S) {
auto *U = dyn_cast<SCEVUnknown>(S);
if (!U)
return false;
auto *I = dyn_cast<Instruction>(U->getValue());
return I && L->contains(I->getParent());
});
}

std::variant<MemoryDepChecker::Dependence::DepType,
MemoryDepChecker::DepDistanceStrideAndSizeInfo>
MemoryDepChecker::getDependenceDistanceStrideAndSize(
Expand Down Expand Up @@ -1959,11 +1972,13 @@ MemoryDepChecker::getDependenceDistanceStrideAndSize(
<< ": " << *Dist << "\n");

// Needs accesses where the addresses of the accessed underlying objects do
// not change within the loop.
// not change within the loop and if all indices can be analyzed.
if (isLoopVariantIndirectAddress(UnderlyingObjects.find(APtr)->second, SE,
InnermostLoop) ||
isLoopVariantIndirectAddress(UnderlyingObjects.find(BPtr)->second, SE,
InnermostLoop))
InnermostLoop) ||
hasLoopVariantUnknownIndex(Src, InnermostLoop) ||
hasLoopVariantUnknownIndex(Sink, InnermostLoop))
return MemoryDepChecker::Dependence::IndirectUnsafe;

// Check if we can prove that Sink only accesses memory after Src's end or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,19 @@
define void @B_indices_loaded_in_loop_A_stored(ptr %A, ptr noalias %B, i64 %N, i64 %off) {
; CHECK-LABEL: 'B_indices_loaded_in_loop_A_stored'
; CHECK-NEXT: loop:
; CHECK-NEXT: Memory dependences are safe with run-time checks
; CHECK-NEXT: Report: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
; CHECK-NEXT: Unsafe indirect dependence.
; CHECK-NEXT: Dependences:
; CHECK-NEXT: IndirectUnsafe:
; CHECK-NEXT: %l = load i32, ptr %gep.B, align 4 ->
; CHECK-NEXT: store i32 %inc, ptr %gep.B, align 4
; CHECK-EMPTY:
; CHECK-NEXT: Unknown:
; CHECK-NEXT: %indices = load i8, ptr %gep.A, align 1 ->
; CHECK-NEXT: store i32 %l, ptr %gep.C, align 4
; CHECK-EMPTY:
; CHECK-NEXT: Run-time memory checks:
; CHECK-NEXT: Check 0:
; CHECK-NEXT: Comparing group ([[GRP1:0x[0-9a-f]+]]):
; CHECK-NEXT: %gep.C = getelementptr inbounds i32, ptr %A, i64 %iv
; CHECK-NEXT: Against group ([[GRP2:0x[0-9a-f]+]]):
; CHECK-NEXT: %gep.A = getelementptr inbounds i8, ptr %A, i64 %iv.off
; CHECK-NEXT: Grouped accesses:
; CHECK-NEXT: Group [[GRP1]]:
; CHECK-NEXT: (Low: %A High: ((4 * %N) + %A))
; CHECK-NEXT: Member: {%A,+,4}<nuw><%loop>
; CHECK-NEXT: Group [[GRP2]]:
; CHECK-NEXT: (Low: (%off + %A) High: (%N + %off + %A))
; CHECK-NEXT: Member: {(%off + %A),+,1}<nw><%loop>
; CHECK-EMPTY:
; CHECK-NEXT: Non vectorizable stores to invariant address were not found in loop.
; CHECK-NEXT: SCEV assumptions:
Expand Down Expand Up @@ -59,9 +57,9 @@ define void @B_indices_loaded_in_loop_A_not_stored(ptr %A, ptr noalias %B, i64 %
; CHECK-LABEL: 'B_indices_loaded_in_loop_A_not_stored'
; CHECK-NEXT: loop:
; CHECK-NEXT: Report: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
; CHECK-NEXT: Unknown data dependence.
; CHECK-NEXT: Unsafe indirect dependence.
; CHECK-NEXT: Dependences:
; CHECK-NEXT: Unknown:
; CHECK-NEXT: IndirectUnsafe:
; CHECK-NEXT: %l = load i32, ptr %gep.B, align 4 ->
; CHECK-NEXT: store i32 %inc, ptr %gep.B, align 4
; CHECK-EMPTY:
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Analysis/LoopAccessAnalysis/select-dependence.ll
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ define void @test(ptr noalias %x, ptr noalias %y, ptr noalias %z) {
; CHECK-LABEL: 'test'
; CHECK-NEXT: loop:
; CHECK-NEXT: Report: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
; CHECK-NEXT: Unknown data dependence.
; CHECK-NEXT: Unsafe indirect dependence.
; CHECK-NEXT: Dependences:
; CHECK-NEXT: Unknown:
; CHECK-NEXT: IndirectUnsafe:
; CHECK-NEXT: %load = load double, ptr %gep.sel, align 8 ->
; CHECK-NEXT: store double %load, ptr %gep.sel2, align 8
; CHECK-EMPTY:
Expand Down

0 comments on commit 97efdc7

Please sign in to comment.