From f41d259d0faf9dba8d465a3a03581a74d6e757dc Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 15 May 2025 13:10:21 +0100 Subject: [PATCH 1/2] [LAA] Clean up APInt-overflow related code Co-authored-by: Florian Hahn --- llvm/lib/Analysis/LoopAccessAnalysis.cpp | 32 ++++++++----------- .../dependences-i128-inductions.ll | 14 ++++---- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp index 23dfbe0209e45..4c1688d3f27cf 100644 --- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp +++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp @@ -948,19 +948,12 @@ getStrideFromAddRec(const SCEVAddRecExpr *AR, const Loop *Lp, Type *AccessTy, TypeSize AllocSize = DL.getTypeAllocSize(AccessTy); int64_t Size = AllocSize.getFixedValue(); - // Huge step value - give up. - if (APStepVal->getBitWidth() > 64) + std::optional StepVal = APStepVal->trySExtValue(); + if (!StepVal) return std::nullopt; - int64_t StepVal = APStepVal->getSExtValue(); - // Strided access. - int64_t Stride = StepVal / Size; - int64_t Rem = StepVal % Size; - if (Rem) - return std::nullopt; - - return Stride; + return *StepVal % Size ? std::nullopt : std::make_optional(*StepVal / Size); } /// Check whether \p AR is a non-wrapping AddRec. If \p Ptr is not nullptr, use @@ -2135,15 +2128,18 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx, DL, SE, *(PSE.getSymbolicMaxBackedgeTakenCount()), *Dist, MaxStride)) return Dependence::NoDep; - // Attempt to prove strided accesses independent. - const APInt *ConstDist = nullptr; - if (match(Dist, m_scev_APInt(ConstDist))) { - uint64_t Distance = ConstDist->abs().getZExtValue(); + // The rest of this function relies on ConstDist being at most 64-bits, which + // is checked earlier. Will assert if the calling code changes. + const APInt *APDist = nullptr; + uint64_t ConstDist = + match(Dist, m_scev_APInt(APDist)) ? APDist->abs().getZExtValue() : 0; + // Attempt to prove strided accesses independent. + if (APDist) { // If the distance between accesses and their strides are known constants, // check whether the accesses interlace each other. - if (Distance > 0 && CommonStride && CommonStride > 1 && HasSameSize && - areStridedAccessesIndependent(Distance, *CommonStride, TypeByteSize)) { + if (ConstDist > 0 && CommonStride && CommonStride > 1 && HasSameSize && + areStridedAccessesIndependent(ConstDist, *CommonStride, TypeByteSize)) { LLVM_DEBUG(dbgs() << "LAA: Strided accesses are independent\n"); return Dependence::NoDep; } @@ -2184,8 +2180,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx, FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck; return Dependence::Unknown; } - if (!HasSameSize || couldPreventStoreLoadForward( - ConstDist->abs().getZExtValue(), TypeByteSize)) { + if (!HasSameSize || + couldPreventStoreLoadForward(ConstDist, TypeByteSize)) { LLVM_DEBUG( dbgs() << "LAA: Forward but may prevent st->ld forwarding\n"); return Dependence::ForwardButPreventsForwarding; diff --git a/llvm/test/Analysis/LoopAccessAnalysis/dependences-i128-inductions.ll b/llvm/test/Analysis/LoopAccessAnalysis/dependences-i128-inductions.ll index 5108cbf07d706..2df451d5df738 100644 --- a/llvm/test/Analysis/LoopAccessAnalysis/dependences-i128-inductions.ll +++ b/llvm/test/Analysis/LoopAccessAnalysis/dependences-i128-inductions.ll @@ -10,9 +10,9 @@ define void @backward_i128(ptr %A, i128 %n) { ; CHECK-LABEL: 'backward_i128' ; 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: Unsafe indirect dependence. +; CHECK-NEXT: Backward loop carried data dependence. ; CHECK-NEXT: Dependences: -; CHECK-NEXT: IndirectUnsafe: +; CHECK-NEXT: Backward: ; CHECK-NEXT: %l = load i32, ptr %gep.A, align 4 -> ; CHECK-NEXT: store i32 %l, ptr %gep.A.1, align 4 ; CHECK-EMPTY: @@ -45,10 +45,9 @@ exit: define void @forward_i128(ptr %A, i128 %n) { ; CHECK-LABEL: 'forward_i128' ; 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: Unsafe indirect dependence. +; CHECK-NEXT: Memory dependences are safe ; CHECK-NEXT: Dependences: -; CHECK-NEXT: IndirectUnsafe: +; CHECK-NEXT: Forward: ; CHECK-NEXT: %l = load i32, ptr %gep.A.1, align 4 -> ; CHECK-NEXT: store i32 %l, ptr %gep.A, align 4 ; CHECK-EMPTY: @@ -81,10 +80,9 @@ exit: define void @forward_negative_step(ptr %A, i128 %n) { ; CHECK-LABEL: 'forward_negative_step' ; 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: Unsafe indirect dependence. +; CHECK-NEXT: Memory dependences are safe ; CHECK-NEXT: Dependences: -; CHECK-NEXT: IndirectUnsafe: +; CHECK-NEXT: Forward: ; CHECK-NEXT: %l = load i32, ptr %gep.A, align 4 -> ; CHECK-NEXT: store i32 %l, ptr %gep.A.1, align 4 ; CHECK-EMPTY: From 02f832511be3e805d125e06f8f771665707cc202 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Mon, 30 Jun 2025 13:46:50 +0100 Subject: [PATCH 2/2] [LAA] Add comment --- llvm/lib/Analysis/LoopAccessAnalysis.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp index 4c1688d3f27cf..007ee3cf01502 100644 --- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp +++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp @@ -948,6 +948,7 @@ getStrideFromAddRec(const SCEVAddRecExpr *AR, const Loop *Lp, Type *AccessTy, TypeSize AllocSize = DL.getTypeAllocSize(AccessTy); int64_t Size = AllocSize.getFixedValue(); + // Huge step value - give up. std::optional StepVal = APStepVal->trySExtValue(); if (!StepVal) return std::nullopt;