Skip to content

Commit 35c2a0f

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 8a3fe30 commit 35c2a0f

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
@@ -372,8 +372,6 @@ class MemoryDepChecker {
372372
uint64_t MaxStride;
373373
std::optional<uint64_t> CommonStride;
374374

375-
bool ShouldRetryWithRuntimeCheck;
376-
377375
/// TypeByteSize is either the common store size of both accesses, or 0 when
378376
/// store sizes mismatch.
379377
uint64_t TypeByteSize;
@@ -383,11 +381,9 @@ class MemoryDepChecker {
383381

384382
DepDistanceStrideAndSizeInfo(const SCEV *Dist, uint64_t MaxStride,
385383
std::optional<uint64_t> CommonStride,
386-
bool ShouldRetryWithRuntimeCheck,
387384
uint64_t TypeByteSize, bool AIsWrite,
388385
bool BIsWrite)
389386
: Dist(Dist), MaxStride(MaxStride), CommonStride(CommonStride),
390-
ShouldRetryWithRuntimeCheck(ShouldRetryWithRuntimeCheck),
391387
TypeByteSize(TypeByteSize), AIsWrite(AIsWrite), BIsWrite(BIsWrite) {}
392388
};
393389

‎llvm/lib/Analysis/LoopAccessAnalysis.cpp‎

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2009,14 +2009,14 @@ MemoryDepChecker::getDependenceDistanceStrideAndSize(
20092009
if (StrideAScaled == StrideBScaled)
20102010
CommonStride = StrideAScaled;
20112011

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

20172018
return DepDistanceStrideAndSizeInfo(Dist, MaxStride, CommonStride,
2018-
ShouldRetryWithRuntimeCheck, TypeByteSize,
2019-
AIsWrite, BIsWrite);
2019+
TypeByteSize, AIsWrite, BIsWrite);
20202020
}
20212021

20222022
MemoryDepChecker::Dependence::DepType
@@ -2031,15 +2031,11 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
20312031
if (std::holds_alternative<Dependence::DepType>(Res))
20322032
return std::get<Dependence::DepType>(Res);
20332033

2034-
auto &[Dist, MaxStride, CommonStride, ShouldRetryWithRuntimeCheck,
2035-
TypeByteSize, AIsWrite, BIsWrite] =
2034+
auto &[Dist, MaxStride, CommonStride, TypeByteSize, AIsWrite, BIsWrite] =
20362035
std::get<DepDistanceStrideAndSizeInfo>(Res);
20372036
bool HasSameSize = TypeByteSize > 0;
20382037

20392038
if (isa<SCEVCouldNotCompute>(Dist)) {
2040-
// TODO: Relax requirement that there is a common unscaled stride to retry
2041-
// with non-constant distance dependencies.
2042-
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
20432039
LLVM_DEBUG(dbgs() << "LAA: Dependence because of uncomputable distance.\n");
20442040
return Dependence::Unknown;
20452041
}
@@ -2099,14 +2095,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
20992095
// forward dependency will allow vectorization using any width.
21002096

21012097
if (IsTrueDataDependence && EnableForwardingConflictDetection) {
2102-
if (!ConstDist) {
2103-
// TODO: FoundNonConstantDistanceDependence is used as a necessary
2104-
// condition to consider retrying with runtime checks. Historically, we
2105-
// did not set it when strides were different but there is no inherent
2106-
// reason to.
2107-
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
2098+
if (!ConstDist)
21082099
return Dependence::Unknown;
2109-
}
21102100
if (!HasSameSize ||
21112101
couldPreventStoreLoadForward(
21122102
ConstDist->getAPInt().abs().getZExtValue(), TypeByteSize)) {
@@ -2122,22 +2112,8 @@ MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
21222112

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

21422118
if (!HasSameSize) {
21432119
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)