Skip to content

Commit 100b7a3

Browse files
committed
[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.
1 parent 3764ba2 commit 100b7a3

File tree

2 files changed

+63
-24
lines changed

2 files changed

+63
-24
lines changed

llvm/lib/Analysis/LoopAccessAnalysis.cpp

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -827,16 +827,13 @@ getStrideFromAddRec(const SCEVAddRecExpr *AR, const Loop *Lp, Type *AccessTy,
827827
TypeSize AllocSize = DL.getTypeAllocSize(AccessTy);
828828
int64_t Size = AllocSize.getFixedValue();
829829

830-
// Huge step value - give up.
831-
if (APStepVal->getBitWidth() > 64)
830+
std::optional<int64_t> StepVal = APStepVal->trySExtValue();
831+
if (!StepVal)
832832
return std::nullopt;
833833

834-
int64_t StepVal = APStepVal->getSExtValue();
835-
836834
// Strided access.
837-
int64_t Stride = StepVal / Size;
838-
int64_t Rem = StepVal % Size;
839-
if (Rem)
835+
int64_t Stride = *StepVal / Size;
836+
if (*StepVal % Size)
840837
return std::nullopt;
841838

842839
return Stride;
@@ -2067,14 +2064,17 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
20672064
return Dependence::NoDep;
20682065

20692066
// Attempt to prove strided accesses independent.
2070-
const APInt *ConstDist = nullptr;
2071-
if (match(Dist, m_scev_APInt(ConstDist))) {
2072-
uint64_t Distance = ConstDist->abs().getZExtValue();
2067+
const APInt *APDist = nullptr;
2068+
std::optional<uint64_t> ConstDistance = match(Dist, m_scev_APInt(APDist))
2069+
? APDist->abs().tryZExtValue()
2070+
: std::nullopt;
20732071

2072+
if (ConstDistance) {
20742073
// If the distance between accesses and their strides are known constants,
20752074
// check whether the accesses interlace each other.
2076-
if (Distance > 0 && CommonStride && CommonStride > 1 && HasSameSize &&
2077-
areStridedAccessesIndependent(Distance, *CommonStride, TypeByteSize)) {
2075+
if (ConstDistance > 0 && CommonStride && CommonStride > 1 && HasSameSize &&
2076+
areStridedAccessesIndependent(*ConstDistance, *CommonStride,
2077+
TypeByteSize)) {
20782078
LLVM_DEBUG(dbgs() << "LAA: Strided accesses are independent\n");
20792079
return Dependence::NoDep;
20802080
}
@@ -2107,16 +2107,16 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
21072107
// forward dependency will allow vectorization using any width.
21082108

21092109
if (IsTrueDataDependence && EnableForwardingConflictDetection) {
2110-
if (!ConstDist) {
2110+
if (!ConstDistance) {
21112111
// TODO: FoundNonConstantDistanceDependence is used as a necessary
21122112
// condition to consider retrying with runtime checks. Historically, we
21132113
// did not set it when strides were different but there is no inherent
21142114
// reason to.
21152115
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
21162116
return Dependence::Unknown;
21172117
}
2118-
if (!HasSameSize || couldPreventStoreLoadForward(
2119-
ConstDist->abs().getZExtValue(), TypeByteSize)) {
2118+
if (!HasSameSize ||
2119+
couldPreventStoreLoadForward(*ConstDistance, TypeByteSize)) {
21202120
LLVM_DEBUG(
21212121
dbgs() << "LAA: Forward but may prevent st->ld forwarding\n");
21222122
return Dependence::ForwardButPreventsForwarding;
@@ -2127,14 +2127,15 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
21272127
return Dependence::Forward;
21282128
}
21292129

2130-
int64_t MinDistance = SE.getSignedRangeMin(Dist).getSExtValue();
2130+
std::optional<int64_t> MinDistance =
2131+
SE.getSignedRangeMin(Dist).trySExtValue();
21312132
// Below we only handle strictly positive distances.
2132-
if (MinDistance <= 0) {
2133+
if (!MinDistance || MinDistance <= 0) {
21332134
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
21342135
return Dependence::Unknown;
21352136
}
21362137

2137-
if (!ConstDist) {
2138+
if (!ConstDistance) {
21382139
// Previously this case would be treated as Unknown, possibly setting
21392140
// FoundNonConstantDistanceDependence to force re-trying with runtime
21402141
// checks. Until the TODO below is addressed, set it here to preserve
@@ -2193,8 +2194,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
21932194
// minimum for computations below, as this ensures we compute the closest
21942195
// possible dependence distance.
21952196
uint64_t MinDistanceNeeded = MaxStride * (MinNumIter - 1) + TypeByteSize;
2196-
if (MinDistanceNeeded > static_cast<uint64_t>(MinDistance)) {
2197-
if (!ConstDist) {
2197+
if (MinDistanceNeeded > static_cast<uint64_t>(*MinDistance)) {
2198+
if (!ConstDistance) {
21982199
// For non-constant distances, we checked the lower bound of the
21992200
// dependence distance and the distance may be larger at runtime (and safe
22002201
// for vectorization). Classify it as Unknown, so we re-try with runtime
@@ -2231,19 +2232,20 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
22312232
// is 8, which is less than 2 and forbidden vectorization, But actually
22322233
// both A and B could be vectorized by 2 iterations.
22332234
MinDepDistBytes =
2234-
std::min(static_cast<uint64_t>(MinDistance), MinDepDistBytes);
2235+
std::min(static_cast<uint64_t>(*MinDistance), MinDepDistBytes);
22352236

22362237
bool IsTrueDataDependence = (!AIsWrite && BIsWrite);
2237-
if (IsTrueDataDependence && EnableForwardingConflictDetection && ConstDist &&
2238-
couldPreventStoreLoadForward(MinDistance, TypeByteSize, *CommonStride))
2238+
if (IsTrueDataDependence && EnableForwardingConflictDetection &&
2239+
ConstDistance &&
2240+
couldPreventStoreLoadForward(*MinDistance, TypeByteSize, *CommonStride))
22392241
return Dependence::BackwardVectorizableButPreventsForwarding;
22402242

22412243
uint64_t MaxVF = MinDepDistBytes / MaxStride;
22422244
LLVM_DEBUG(dbgs() << "LAA: Positive min distance " << MinDistance
22432245
<< " with max VF = " << MaxVF << '\n');
22442246

22452247
uint64_t MaxVFInBits = MaxVF * TypeByteSize * 8;
2246-
if (!ConstDist && MaxVFInBits < MaxTargetVectorWidthInBits) {
2248+
if (!ConstDistance && MaxVFInBits < MaxTargetVectorWidthInBits) {
22472249
// For non-constant distances, we checked the lower bound of the dependence
22482250
// distance and the distance may be larger at runtime (and safe for
22492251
// vectorization). Classify it as Unknown, so we re-try with runtime checks.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -passes='print<access-info>' -disable-output %s 2>&1 | FileCheck %s
3+
4+
define void @test(ptr %this, i128 %loop.limit) {
5+
; CHECK-LABEL: 'test'
6+
; CHECK-NEXT: loop:
7+
; CHECK-NEXT: Memory dependences are safe
8+
; CHECK-NEXT: Dependences:
9+
; CHECK-NEXT: Forward:
10+
; CHECK-NEXT: store i64 1, ptr %c, align 8 ->
11+
; CHECK-NEXT: store i64 2, ptr %d, align 8
12+
; CHECK-EMPTY:
13+
; CHECK-NEXT: Run-time memory checks:
14+
; CHECK-NEXT: Grouped accesses:
15+
; CHECK-EMPTY:
16+
; CHECK-NEXT: Non vectorizable stores to invariant address were not found in loop.
17+
; CHECK-NEXT: SCEV assumptions:
18+
; CHECK-EMPTY:
19+
; CHECK-NEXT: Expressions re-written:
20+
;
21+
entry:
22+
br label %loop
23+
loop:
24+
%iv = phi i128 [ 0, %entry ], [ %iv.next, %loop ]
25+
%iv.mul.8 = mul i128 %iv, 8
26+
%c = getelementptr i8, ptr %this, i128 %iv.mul.8
27+
store i64 1, ptr %c, align 8
28+
%iv.off = add i128 %iv.mul.8, u0x00FFFFFFFFFFFFFFFF
29+
%d = getelementptr i8, ptr %this, i128 %iv.off
30+
store i64 2, ptr %d, align 8
31+
%iv.next = add nuw nsw i128 %iv, 1
32+
%exit.cond = icmp ult i128 %iv.next, %loop.limit
33+
br i1 %exit.cond, label %loop, label %exit
34+
exit:
35+
ret void
36+
}
37+

0 commit comments

Comments
 (0)