Skip to content

Commit 9c5c2fe

Browse files
artagnonartagnon
committed
[LAA] hoist setting condition for rt-checks (NFC)
Strip ShouldRetyWithRuntimeCheck from the DepedenceDistanceStrideAndSizeInfo struct, and free isDependent from the responsibility of setting the condition for when runtime-checks are needed, transferring this responsibility to getDependenceDistanceStrideAndSize. The change is simple enough, but proving that this was the original intent of the code, making the change non-functional is non-trivial. The proof is as follows. The condition for runtime-checks is only respected when we return a Dependence::DepType of Dependence::Unknown, and the only possible place that could introduce a functional change with this patch is when Dist is possibly zero, but HasSameSize is false. Previously, we weren't checking that the Dist is not a SCEVConstant, and setting the runtime-check condition. Now, with the runtime-check condition hoisted, we do. We note that for a functional change to occur, the distance must be both non-constant, not provably non-positive and non-negative, and this is an impossibility.
1 parent 47ce75e commit 9c5c2fe

File tree

3 files changed

+32
-54
lines changed

3 files changed

+32
-54
lines changed

‎llvm/include/llvm/Analysis/LoopAccessAnalysis.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -393,8 +393,6 @@ class MemoryDepChecker {
393393
uint64_t MaxStride;
394394
std::optional<uint64_t> CommonStride;
395395

396-
bool ShouldRetryWithRuntimeCheck;
397-
398396
/// TypeByteSize is either the common store size of both accesses, or 0 when
399397
/// store sizes mismatch.
400398
uint64_t TypeByteSize;
@@ -404,11 +402,9 @@ class MemoryDepChecker {
404402

405403
DepDistanceStrideAndSizeInfo(const SCEV *Dist, uint64_t MaxStride,
406404
std::optional<uint64_t> CommonStride,
407-
bool ShouldRetryWithRuntimeCheck,
408405
uint64_t TypeByteSize, bool AIsWrite,
409406
bool BIsWrite)
410407
: Dist(Dist), MaxStride(MaxStride), CommonStride(CommonStride),
411-
ShouldRetryWithRuntimeCheck(ShouldRetryWithRuntimeCheck),
412408
TypeByteSize(TypeByteSize), AIsWrite(AIsWrite), BIsWrite(BIsWrite) {}
413409
};
414410

‎llvm/lib/Analysis/LoopAccessAnalysis.cpp

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2014,14 +2014,14 @@ MemoryDepChecker::getDependenceDistanceStrideAndSize(
20142014
if (StrideAScaled == StrideBScaled)
20152015
CommonStride = StrideAScaled;
20162016

2017-
// TODO: Historically, we don't retry with runtime checks unless the
2018-
// (unscaled) strides are the same. Fix this once the condition for runtime
2019-
// checks in isDependent is fixed.
2020-
bool ShouldRetryWithRuntimeCheck = StrideAPtrInt == StrideBPtrInt;
2017+
// TODO: FoundNonConstantDistanceDependence is used as a necessary condition
2018+
// to consider retrying with runtime checks. Historically, we did not set it
2019+
// when (unscaled) strides were different but there is no inherent reason to.
2020+
if (!isa<SCEVConstant>(Dist))
2021+
FoundNonConstantDistanceDependence |= StrideAPtrInt == StrideBPtrInt;
20212022

20222023
return DepDistanceStrideAndSizeInfo(Dist, MaxStride, CommonStride,
2023-
ShouldRetryWithRuntimeCheck, TypeByteSize,
2024-
AIsWrite, BIsWrite);
2024+
TypeByteSize, AIsWrite, BIsWrite);
20252025
}
20262026

20272027
MemoryDepChecker::Dependence::DepType
@@ -2036,15 +2036,11 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
20362036
if (std::holds_alternative<Dependence::DepType>(Res))
20372037
return std::get<Dependence::DepType>(Res);
20382038

2039-
auto &[Dist, MaxStride, CommonStride, ShouldRetryWithRuntimeCheck,
2040-
TypeByteSize, AIsWrite, BIsWrite] =
2039+
auto &[Dist, MaxStride, CommonStride, TypeByteSize, AIsWrite, BIsWrite] =
20412040
std::get<DepDistanceStrideAndSizeInfo>(Res);
20422041
bool HasSameSize = TypeByteSize > 0;
20432042

20442043
if (isa<SCEVCouldNotCompute>(Dist)) {
2045-
// TODO: Relax requirement that there is a common unscaled stride to retry
2046-
// with non-constant distance dependencies.
2047-
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
20482044
LLVM_DEBUG(dbgs() << "LAA: Dependence because of uncomputable distance.\n");
20492045
return Dependence::Unknown;
20502046
}
@@ -2103,14 +2099,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
21032099
// forward dependency will allow vectorization using any width.
21042100

21052101
if (IsTrueDataDependence && EnableForwardingConflictDetection) {
2106-
if (!ConstDist) {
2107-
// TODO: FoundNonConstantDistanceDependence is used as a necessary
2108-
// condition to consider retrying with runtime checks. Historically, we
2109-
// did not set it when strides were different but there is no inherent
2110-
// reason to.
2111-
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
2102+
if (!ConstDist)
21122103
return Dependence::Unknown;
2113-
}
21142104
if (!HasSameSize || couldPreventStoreLoadForward(
21152105
ConstDist->abs().getZExtValue(), TypeByteSize)) {
21162106
LLVM_DEBUG(
@@ -2125,22 +2115,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
21252115

21262116
int64_t MinDistance = SE.getSignedRangeMin(Dist).getSExtValue();
21272117
// Below we only handle strictly positive distances.
2128-
if (MinDistance <= 0) {
2129-
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
2118+
if (MinDistance <= 0)
21302119
return Dependence::Unknown;
2131-
}
2132-
2133-
if (!ConstDist) {
2134-
// Previously this case would be treated as Unknown, possibly setting
2135-
// FoundNonConstantDistanceDependence to force re-trying with runtime
2136-
// checks. Until the TODO below is addressed, set it here to preserve
2137-
// original behavior w.r.t. re-trying with runtime checks.
2138-
// TODO: FoundNonConstantDistanceDependence is used as a necessary
2139-
// condition to consider retrying with runtime checks. Historically, we
2140-
// did not set it when strides were different but there is no inherent
2141-
// reason to.
2142-
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
2143-
}
21442120

21452121
if (!HasSameSize) {
21462122
LLVM_DEBUG(dbgs() << "LAA: ReadWrite-Write positive dependency with "

‎llvm/test/Analysis/LoopAccessAnalysis/retry-runtime-checks-after-dependence-analysis.ll

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -320,28 +320,34 @@ exit:
320320
define void @retry_after_dep_check_with_unknown_offset(ptr %A, i32 %offset) {
321321
; CHECK-LABEL: 'retry_after_dep_check_with_unknown_offset'
322322
; CHECK-NEXT: loop:
323-
; 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
324-
; CHECK-NEXT: Unknown data dependence.
323+
; CHECK-NEXT: Memory dependences are safe with run-time checks
325324
; CHECK-NEXT: Dependences:
326-
; CHECK-NEXT: Unknown:
327-
; CHECK-NEXT: %l.A = load float, ptr %A, align 4 ->
328-
; CHECK-NEXT: store float 0.000000e+00, ptr %A.100.iv.offset.3, align 4
329-
; CHECK-EMPTY:
330-
; CHECK-NEXT: Unknown:
331-
; CHECK-NEXT: %l.A = load float, ptr %A, align 4 ->
332-
; CHECK-NEXT: store float %l.A, ptr %A.100.iv, align 8
333-
; CHECK-EMPTY:
334325
; CHECK-NEXT: Run-time memory checks:
326+
; CHECK-NEXT: Check 0:
327+
; CHECK-NEXT: Comparing group ([[GRP16:0x[0-9a-f]+]]):
328+
; CHECK-NEXT: %A.100.iv = getelementptr { float, float }, ptr %A.100, i64 %iv
329+
; CHECK-NEXT: Against group ([[GRP17:0x[0-9a-f]+]]):
330+
; CHECK-NEXT: %A.100.iv.offset.3 = getelementptr i8, ptr %A.100, i64 %iv.offset.3
331+
; CHECK-NEXT: Check 1:
332+
; CHECK-NEXT: Comparing group ([[GRP16]]):
333+
; CHECK-NEXT: %A.100.iv = getelementptr { float, float }, ptr %A.100, i64 %iv
334+
; CHECK-NEXT: Against group ([[GRP18:0x[0-9a-f]+]]):
335+
; CHECK-NEXT: ptr %A
336+
; CHECK-NEXT: Check 2:
337+
; CHECK-NEXT: Comparing group ([[GRP17]]):
338+
; CHECK-NEXT: %A.100.iv.offset.3 = getelementptr i8, ptr %A.100, i64 %iv.offset.3
339+
; CHECK-NEXT: Against group ([[GRP18]]):
340+
; CHECK-NEXT: ptr %A
335341
; CHECK-NEXT: Grouped accesses:
336-
; CHECK-NEXT: Group [[GRP16:0x[0-9a-f]+]]:
337-
; CHECK-NEXT: (Low: %A High: (4 + %A))
338-
; CHECK-NEXT: Member: %A
339-
; CHECK-NEXT: Group [[GRP17:0x[0-9a-f]+]]:
340-
; CHECK-NEXT: (Low: (100 + (8 * (zext i32 %offset to i64))<nuw><nsw> + %A) High: (96 + (16 * (zext i32 %offset to i64))<nuw><nsw> + %A))
341-
; CHECK-NEXT: Member: {(100 + (8 * (zext i32 %offset to i64))<nuw><nsw> + %A),+,8}<%loop>
342-
; CHECK-NEXT: Group [[GRP18:0x[0-9a-f]+]]:
342+
; CHECK-NEXT: Group [[GRP16]]:
343343
; CHECK-NEXT: (Low: (100 + %A) High: (96 + (8 * (zext i32 %offset to i64))<nuw><nsw> + %A))
344344
; CHECK-NEXT: Member: {(100 + %A),+,8}<%loop>
345+
; CHECK-NEXT: Group [[GRP17]]:
346+
; CHECK-NEXT: (Low: (100 + (8 * (zext i32 %offset to i64))<nuw><nsw> + %A) High: (96 + (16 * (zext i32 %offset to i64))<nuw><nsw> + %A))
347+
; CHECK-NEXT: Member: {(100 + (8 * (zext i32 %offset to i64))<nuw><nsw> + %A),+,8}<%loop>
348+
; CHECK-NEXT: Group [[GRP18]]:
349+
; CHECK-NEXT: (Low: %A High: (4 + %A))
350+
; CHECK-NEXT: Member: %A
345351
; CHECK-EMPTY:
346352
; CHECK-NEXT: Non vectorizable stores to invariant address were not found in loop.
347353
; CHECK-NEXT: SCEV assumptions:

0 commit comments

Comments
 (0)