[llvm] [LAA] Clean up APInt-overflow related code (NFC) (PR #140048)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Thu May 15 10:15:34 PDT 2025
https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/140048
>From 100b7a3818480cb317841d238834a5554914ab1a Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Thu, 15 May 2025 13:10:21 +0100
Subject: [PATCH 1/4] [LAA] Clean up APInt-overflow related code (NFC)
Use APInt::{trySExtValue,tryZExtValue} instead of the asserting
variants. The change is untestable, as the test demonstrates, since
getTypeSizeInBits is not equipped to handle type-sizes over 64-bits, and
the constant distance flowing through LAA is -1 as a consequence.
---
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 50 ++++++++++---------
.../long-pointer-distance.ll | 37 ++++++++++++++
2 files changed, 63 insertions(+), 24 deletions(-)
create mode 100644 llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index ab407e945bc53..63eb97eaeca5b 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -827,16 +827,13 @@ 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<int64_t> 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)
+ int64_t Stride = *StepVal / Size;
+ if (*StepVal % Size)
return std::nullopt;
return Stride;
@@ -2067,14 +2064,17 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
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();
+ const APInt *APDist = nullptr;
+ std::optional<uint64_t> ConstDistance = match(Dist, m_scev_APInt(APDist))
+ ? APDist->abs().tryZExtValue()
+ : std::nullopt;
+ if (ConstDistance) {
// 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 (ConstDistance > 0 && CommonStride && CommonStride > 1 && HasSameSize &&
+ areStridedAccessesIndependent(*ConstDistance, *CommonStride,
+ TypeByteSize)) {
LLVM_DEBUG(dbgs() << "LAA: Strided accesses are independent\n");
return Dependence::NoDep;
}
@@ -2107,7 +2107,7 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
// forward dependency will allow vectorization using any width.
if (IsTrueDataDependence && EnableForwardingConflictDetection) {
- if (!ConstDist) {
+ if (!ConstDistance) {
// TODO: FoundNonConstantDistanceDependence is used as a necessary
// condition to consider retrying with runtime checks. Historically, we
// did not set it when strides were different but there is no inherent
@@ -2115,8 +2115,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
return Dependence::Unknown;
}
- if (!HasSameSize || couldPreventStoreLoadForward(
- ConstDist->abs().getZExtValue(), TypeByteSize)) {
+ if (!HasSameSize ||
+ couldPreventStoreLoadForward(*ConstDistance, TypeByteSize)) {
LLVM_DEBUG(
dbgs() << "LAA: Forward but may prevent st->ld forwarding\n");
return Dependence::ForwardButPreventsForwarding;
@@ -2127,14 +2127,15 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
return Dependence::Forward;
}
- int64_t MinDistance = SE.getSignedRangeMin(Dist).getSExtValue();
+ std::optional<int64_t> MinDistance =
+ SE.getSignedRangeMin(Dist).trySExtValue();
// Below we only handle strictly positive distances.
- if (MinDistance <= 0) {
+ if (!MinDistance || MinDistance <= 0) {
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
return Dependence::Unknown;
}
- if (!ConstDist) {
+ if (!ConstDistance) {
// Previously this case would be treated as Unknown, possibly setting
// FoundNonConstantDistanceDependence to force re-trying with runtime
// checks. Until the TODO below is addressed, set it here to preserve
@@ -2193,8 +2194,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
// minimum for computations below, as this ensures we compute the closest
// possible dependence distance.
uint64_t MinDistanceNeeded = MaxStride * (MinNumIter - 1) + TypeByteSize;
- if (MinDistanceNeeded > static_cast<uint64_t>(MinDistance)) {
- if (!ConstDist) {
+ if (MinDistanceNeeded > static_cast<uint64_t>(*MinDistance)) {
+ if (!ConstDistance) {
// For non-constant distances, we checked the lower bound of the
// dependence distance and the distance may be larger at runtime (and safe
// for vectorization). Classify it as Unknown, so we re-try with runtime
@@ -2231,11 +2232,12 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
// is 8, which is less than 2 and forbidden vectorization, But actually
// both A and B could be vectorized by 2 iterations.
MinDepDistBytes =
- std::min(static_cast<uint64_t>(MinDistance), MinDepDistBytes);
+ std::min(static_cast<uint64_t>(*MinDistance), MinDepDistBytes);
bool IsTrueDataDependence = (!AIsWrite && BIsWrite);
- if (IsTrueDataDependence && EnableForwardingConflictDetection && ConstDist &&
- couldPreventStoreLoadForward(MinDistance, TypeByteSize, *CommonStride))
+ if (IsTrueDataDependence && EnableForwardingConflictDetection &&
+ ConstDistance &&
+ couldPreventStoreLoadForward(*MinDistance, TypeByteSize, *CommonStride))
return Dependence::BackwardVectorizableButPreventsForwarding;
uint64_t MaxVF = MinDepDistBytes / MaxStride;
@@ -2243,7 +2245,7 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
<< " with max VF = " << MaxVF << '\n');
uint64_t MaxVFInBits = MaxVF * TypeByteSize * 8;
- if (!ConstDist && MaxVFInBits < MaxTargetVectorWidthInBits) {
+ if (!ConstDistance && MaxVFInBits < MaxTargetVectorWidthInBits) {
// For non-constant distances, we checked the lower bound of the dependence
// distance and the distance may be larger at runtime (and safe for
// vectorization). Classify it as Unknown, so we re-try with runtime checks.
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll b/llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll
new file mode 100644
index 0000000000000..094d2a0e7fa37
--- /dev/null
+++ b/llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll
@@ -0,0 +1,37 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes='print<access-info>' -disable-output %s 2>&1 | FileCheck %s
+
+define void @test(ptr %this, i128 %loop.limit) {
+; CHECK-LABEL: 'test'
+; CHECK-NEXT: loop:
+; CHECK-NEXT: Memory dependences are safe
+; CHECK-NEXT: Dependences:
+; CHECK-NEXT: Forward:
+; CHECK-NEXT: store i64 1, ptr %c, align 8 ->
+; CHECK-NEXT: store i64 2, ptr %d, align 8
+; CHECK-EMPTY:
+; CHECK-NEXT: Run-time memory checks:
+; CHECK-NEXT: Grouped accesses:
+; CHECK-EMPTY:
+; CHECK-NEXT: Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT: SCEV assumptions:
+; CHECK-EMPTY:
+; CHECK-NEXT: Expressions re-written:
+;
+entry:
+ br label %loop
+loop:
+ %iv = phi i128 [ 0, %entry ], [ %iv.next, %loop ]
+ %iv.mul.8 = mul i128 %iv, 8
+ %c = getelementptr i8, ptr %this, i128 %iv.mul.8
+ store i64 1, ptr %c, align 8
+ %iv.off = add i128 %iv.mul.8, u0x00FFFFFFFFFFFFFFFF
+ %d = getelementptr i8, ptr %this, i128 %iv.off
+ store i64 2, ptr %d, align 8
+ %iv.next = add nuw nsw i128 %iv, 1
+ %exit.cond = icmp ult i128 %iv.next, %loop.limit
+ br i1 %exit.cond, label %loop, label %exit
+exit:
+ ret void
+}
+
>From dc234572106767ff9200d57e77b66c881b23d970 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Thu, 15 May 2025 13:59:46 +0100
Subject: [PATCH 2/4] [LAA] Add additional test
---
.../long-pointer-distance.ll | 40 ++++++++++++++++++-
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll b/llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll
index 094d2a0e7fa37..1e782dba25655 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll
@@ -1,8 +1,8 @@
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
; RUN: opt -passes='print<access-info>' -disable-output %s 2>&1 | FileCheck %s
-define void @test(ptr %this, i128 %loop.limit) {
-; CHECK-LABEL: 'test'
+define void @forward_i128_offset_difference(ptr %this, i128 %loop.limit) {
+; CHECK-LABEL: 'forward_i128_offset_difference'
; CHECK-NEXT: loop:
; CHECK-NEXT: Memory dependences are safe
; CHECK-NEXT: Dependences:
@@ -35,3 +35,39 @@ exit:
ret void
}
+define void @backward_distance_offset_subtract(ptr %A) {
+; CHECK-LABEL: 'backward_distance_offset_subtract'
+; CHECK-NEXT: loop:
+; CHECK-NEXT: Memory dependences are safe
+; CHECK-NEXT: Dependences:
+; CHECK-NEXT: Forward:
+; CHECK-NEXT: %l = load i32, ptr %gep.off.1, align 4 ->
+; CHECK-NEXT: store i32 %add, ptr %gep.off.2, align 4
+; CHECK-EMPTY:
+; CHECK-NEXT: Run-time memory checks:
+; CHECK-NEXT: Grouped accesses:
+; CHECK-EMPTY:
+; CHECK-NEXT: Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT: SCEV assumptions:
+; CHECK-EMPTY:
+; CHECK-NEXT: Expressions re-written:
+;
+entry:
+ %A.off.1 = getelementptr inbounds i8, ptr %A, i64 u0x100000018
+ %A.off.2 = getelementptr inbounds i8, ptr %A, i64 u0x100000010
+ br label %loop
+
+loop:
+ %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ]
+ %gep.off.1 = getelementptr inbounds i32, ptr %A.off.1, i64 %iv
+ %l = load i32, ptr %gep.off.1, align 4
+ %add = add nsw i32 %l, 5
+ %gep.off.2 = getelementptr inbounds i32, ptr %A.off.2, i64 %iv
+ store i32 %add, ptr %gep.off.2, align 4
+ %iv.next = add nuw nsw i64 %iv, 1
+ %exitcond.not = icmp eq i64 %iv.next, 256
+ br i1 %exitcond.not, label %exit, label %loop
+
+exit:
+ ret void
+}
>From 1dc0934c89c58c96b55bde77f08cc457b2d35904 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Thu, 15 May 2025 17:24:04 +0100
Subject: [PATCH 3/4] [LAA] Address review
---
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 63eb97eaeca5b..7bcc37f09ce29 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -2065,15 +2065,17 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
// Attempt to prove strided accesses independent.
const APInt *APDist = nullptr;
- std::optional<uint64_t> ConstDistance = match(Dist, m_scev_APInt(APDist))
- ? APDist->abs().tryZExtValue()
- : std::nullopt;
+
+ // The rest of this function relies on ConstantDistance being at most 64-bits,
+ // which is checked earlier. Will assert if the calling code changes.
+ uint64_t ConstDistance =
+ match(Dist, m_scev_APInt(APDist)) ? APDist->abs().getZExtValue() : 0;
if (ConstDistance) {
// If the distance between accesses and their strides are known constants,
// check whether the accesses interlace each other.
if (ConstDistance > 0 && CommonStride && CommonStride > 1 && HasSameSize &&
- areStridedAccessesIndependent(*ConstDistance, *CommonStride,
+ areStridedAccessesIndependent(ConstDistance, *CommonStride,
TypeByteSize)) {
LLVM_DEBUG(dbgs() << "LAA: Strided accesses are independent\n");
return Dependence::NoDep;
@@ -2116,7 +2118,7 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
return Dependence::Unknown;
}
if (!HasSameSize ||
- couldPreventStoreLoadForward(*ConstDistance, TypeByteSize)) {
+ couldPreventStoreLoadForward(ConstDistance, TypeByteSize)) {
LLVM_DEBUG(
dbgs() << "LAA: Forward but may prevent st->ld forwarding\n");
return Dependence::ForwardButPreventsForwarding;
>From 1c5950651caae4644b7fca14e433b54655118b6c Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Thu, 15 May 2025 18:15:09 +0100
Subject: [PATCH 4/4] [LAA] Fix thinko
---
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 7bcc37f09ce29..24f2c8cf80511 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -2071,7 +2071,7 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
uint64_t ConstDistance =
match(Dist, m_scev_APInt(APDist)) ? APDist->abs().getZExtValue() : 0;
- if (ConstDistance) {
+ if (APDist) {
// If the distance between accesses and their strides are known constants,
// check whether the accesses interlace each other.
if (ConstDistance > 0 && CommonStride && CommonStride > 1 && HasSameSize &&
More information about the llvm-commits
mailing list