[llvm] LAA: scale strides using type-size (NFC) (PR #124529)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 31 06:35:54 PST 2025
https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/124529
>From 94ef46559b090406847f614aefbfa01b1ee1d7e0 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 27 Jan 2025 10:51:31 +0000
Subject: [PATCH 1/2] LAA: scale strides using type-size
Change getDependenceDistanceStrideAndSize to scale strides by
TypeByteSize, scaling the returned CommonStride and MaxStride. We now
correctly detect that there is indeed a common stride, when we failed to
previously due to differing type sizes.
---
.../llvm/Analysis/LoopAccessAnalysis.h | 2 +-
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 60 ++++++++++---------
2 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/llvm/include/llvm/Analysis/LoopAccessAnalysis.h b/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
index 6fc6ca14d088956..d84b5a03d70a965 100644
--- a/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
+++ b/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
@@ -370,7 +370,7 @@ class MemoryDepChecker {
/// Strides could either be scaled (in bytes, taking the size of the
/// underlying type into account), or unscaled (in indexing units; unscaled
/// stride = scaled stride / size of underlying type). Here, strides are
- /// unscaled.
+ /// scaled.
uint64_t MaxStride;
std::optional<uint64_t> CommonStride;
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 697b40403902cc1..4bb3c3569ba4f40 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -1789,8 +1789,7 @@ void MemoryDepChecker::mergeInStatus(VectorizationSafetyStatus S) {
/// }
static bool isSafeDependenceDistance(const DataLayout &DL, ScalarEvolution &SE,
const SCEV &MaxBTC, const SCEV &Dist,
- uint64_t MaxStride,
- uint64_t TypeByteSize) {
+ uint64_t MaxStride) {
// If we can prove that
// (**) |Dist| > MaxBTC * Step
@@ -1809,8 +1808,7 @@ static bool isSafeDependenceDistance(const DataLayout &DL, ScalarEvolution &SE,
// will be executed only if LoopCount >= VF, proving distance >= LoopCount
// also guarantees that distance >= VF.
//
- const uint64_t ByteStride = MaxStride * TypeByteSize;
- const SCEV *Step = SE.getConstant(MaxBTC.getType(), ByteStride);
+ const SCEV *Step = SE.getConstant(MaxBTC.getType(), MaxStride);
const SCEV *Product = SE.getMulExpr(&MaxBTC, Step);
const SCEV *CastedDist = &Dist;
@@ -1854,14 +1852,12 @@ static bool areStridedAccessesIndependent(uint64_t Distance, uint64_t Stride,
if (Distance % TypeByteSize)
return false;
- uint64_t ScaledDist = Distance / TypeByteSize;
-
- // No dependence if the scaled distance is not multiple of the stride.
+ // No dependence if the distance is not multiple of the stride.
// E.g.
// for (i = 0; i < 1024 ; i += 4)
// A[i+2] = A[i] + 1;
//
- // Two accesses in memory (scaled distance is 2, stride is 4):
+ // Two accesses in memory (distance is 2, stride is 4):
// | A[0] | | | | A[4] | | | |
// | | | A[2] | | | | A[6] | |
//
@@ -1869,10 +1865,10 @@ static bool areStridedAccessesIndependent(uint64_t Distance, uint64_t Stride,
// for (i = 0; i < 1024 ; i += 3)
// A[i+4] = A[i] + 1;
//
- // Two accesses in memory (scaled distance is 4, stride is 3):
+ // Two accesses in memory (distance is 4, stride is 3):
// | A[0] | | | A[3] | | | A[6] | | |
// | | | | | A[4] | | | A[7] | |
- return ScaledDist % Stride;
+ return Distance % Stride;
}
std::variant<MemoryDepChecker::Dependence::DepType,
@@ -1981,25 +1977,32 @@ MemoryDepChecker::getDependenceDistanceStrideAndSize(
return MemoryDepChecker::Dependence::Unknown;
}
- uint64_t TypeByteSize = DL.getTypeAllocSize(ATy);
- bool HasSameSize =
- DL.getTypeStoreSizeInBits(ATy) == DL.getTypeStoreSizeInBits(BTy);
- if (!HasSameSize)
- TypeByteSize = 0;
+ TypeSize AStoreSz = DL.getTypeStoreSize(ATy),
+ BStoreSz = DL.getTypeStoreSize(BTy);
+
+ // Fail early if either store size is scalable.
+ if (AStoreSz.isScalable() || BStoreSz.isScalable())
+ return MemoryDepChecker::Dependence::Unknown;
+
+ // If store sizes are not the same, set TypeByteSize to zero, so we can check
+ // it in the caller.
+ uint64_t ASz = alignTo(AStoreSz, DL.getABITypeAlign(ATy)),
+ BSz = alignTo(BStoreSz, DL.getABITypeAlign(BTy)),
+ TypeByteSize = AStoreSz == BStoreSz ? BSz : 0;
- StrideAPtrInt = std::abs(StrideAPtrInt);
- StrideBPtrInt = std::abs(StrideBPtrInt);
+ uint64_t StrideAScaled = std::abs(StrideAPtrInt) * ASz;
+ uint64_t StrideBScaled = std::abs(StrideBPtrInt) * BSz;
- uint64_t MaxStride = std::max(StrideAPtrInt, StrideBPtrInt);
+ uint64_t MaxStride = std::max(StrideAScaled, StrideBScaled);
std::optional<uint64_t> CommonStride;
- if (StrideAPtrInt == StrideBPtrInt)
- CommonStride = StrideAPtrInt;
+ if (StrideAScaled == StrideBScaled)
+ CommonStride = StrideAScaled;
// TODO: Historically, we don't retry with runtime checks unless the
// (unscaled) strides are the same. Fix this once the condition for runtime
// checks in isDependent is fixed.
- bool ShouldRetryWithRuntimeCheck = CommonStride.has_value();
+ bool ShouldRetryWithRuntimeCheck = StrideAPtrInt == StrideBPtrInt;
return DepDistanceStrideAndSizeInfo(Dist, MaxStride, CommonStride,
ShouldRetryWithRuntimeCheck, TypeByteSize,
@@ -2039,9 +2042,9 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
// upper bound of the number of iterations), the accesses are independet, i.e.
// they are far enough appart that accesses won't access the same location
// across all loop ierations.
- if (HasSameSize && isSafeDependenceDistance(
- DL, SE, *(PSE.getSymbolicMaxBackedgeTakenCount()),
- *Dist, MaxStride, TypeByteSize))
+ if (HasSameSize &&
+ isSafeDependenceDistance(
+ DL, SE, *(PSE.getSymbolicMaxBackedgeTakenCount()), *Dist, MaxStride))
return Dependence::NoDep;
const SCEVConstant *ConstDist = dyn_cast<SCEVConstant>(Dist);
@@ -2145,8 +2148,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
// It's not vectorizable if the distance is smaller than the minimum distance
// needed for a vectroized/unrolled version. Vectorizing one iteration in
- // front needs TypeByteSize * Stride. Vectorizing the last iteration needs
- // TypeByteSize (No need to plus the last gap distance).
+ // front needs CommonStride. Vectorizing the last iteration needs TypeByteSize
+ // (No need to plus the last gap distance).
//
// E.g. Assume one char is 1 byte in memory and one int is 4 bytes.
// foo(int *A) {
@@ -2173,8 +2176,7 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
// We know that Dist is positive, but it may not be constant. Use the signed
// minimum for computations below, as this ensures we compute the closest
// possible dependence distance.
- uint64_t MinDistanceNeeded =
- TypeByteSize * *CommonStride * (MinNumIter - 1) + TypeByteSize;
+ uint64_t MinDistanceNeeded = *CommonStride * (MinNumIter - 1) + TypeByteSize;
if (MinDistanceNeeded > static_cast<uint64_t>(MinDistance)) {
if (!ConstDist) {
// For non-constant distances, we checked the lower bound of the
@@ -2230,7 +2232,7 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
// An update to MinDepDistBytes requires an update to MaxSafeVectorWidthInBits
// since there is a backwards dependency.
- uint64_t MaxVF = MinDepDistBytes / (TypeByteSize * *CommonStride);
+ uint64_t MaxVF = MinDepDistBytes / *CommonStride;
LLVM_DEBUG(dbgs() << "LAA: Positive min distance " << MinDistance
<< " with max VF = " << MaxVF << '\n');
>From 1b92628f35641d9857230cba46fd1795dd2d4ba7 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Fri, 31 Jan 2025 14:34:13 +0000
Subject: [PATCH 2/2] LAA: address review
---
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 4bb3c3569ba4f40..fe8b750dfbbdf73 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -1980,14 +1980,9 @@ MemoryDepChecker::getDependenceDistanceStrideAndSize(
TypeSize AStoreSz = DL.getTypeStoreSize(ATy),
BStoreSz = DL.getTypeStoreSize(BTy);
- // Fail early if either store size is scalable.
- if (AStoreSz.isScalable() || BStoreSz.isScalable())
- return MemoryDepChecker::Dependence::Unknown;
-
// If store sizes are not the same, set TypeByteSize to zero, so we can check
// it in the caller.
- uint64_t ASz = alignTo(AStoreSz, DL.getABITypeAlign(ATy)),
- BSz = alignTo(BStoreSz, DL.getABITypeAlign(BTy)),
+ uint64_t ASz = DL.getTypeAllocSize(ATy), BSz = DL.getTypeAllocSize(BTy),
TypeByteSize = AStoreSz == BStoreSz ? BSz : 0;
uint64_t StrideAScaled = std::abs(StrideAPtrInt) * ASz;
More information about the llvm-commits
mailing list