[llvm] [LAA] Clean up APInt-overflow related code (NFC) (PR #140048)
via llvm-commits
llvm-commits at lists.llvm.org
Thu May 15 05:40:26 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
Author: Ramkumar Ramachandra (artagnon)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/140048.diff
2 Files Affected:
- (modified) llvm/lib/Analysis/LoopAccessAnalysis.cpp (+26-24)
- (added) llvm/test/Analysis/LoopAccessAnalysis/long-pointer-distance.ll (+37)
``````````diff
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
+}
+
``````````
</details>
https://github.com/llvm/llvm-project/pull/140048
More information about the llvm-commits
mailing list