[llvm] LAA: be more precise on different store sizes (PR #122318)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 10 03:24:10 PST 2025
================
@@ -1989,11 +1990,24 @@ MemoryDepChecker::getDependenceDistanceStrideAndSize(
return MemoryDepChecker::Dependence::Unknown;
}
- uint64_t TypeByteSize = DL.getTypeAllocSize(ATy);
- bool HasSameSize =
- DL.getTypeStoreSizeInBits(ATy) == DL.getTypeStoreSizeInBits(BTy);
- if (!HasSameSize)
- TypeByteSize = 0;
+ // When the distance is zero, we're reading/writing the same memory location:
+ // check that the store sizes are equal. Otherwise, fail with an unknown
+ // dependence for which we should not generate runtime checks.
+ TypeSize AStoreSz = DL.getTypeStoreSize(ATy),
+ BStoreSz = DL.getTypeStoreSize(BTy);
+ if (Dist->isZero() && AStoreSz != BStoreSz) {
----------------
artagnon wrote:
So I think this is wrong, and results in test regressions, because if the distance is not constant, we should retry with runtime checks. We're handling the scenario where we know that the distance is zero and hence know that there is a read/write with different store sizes: this is the condition on which we should bail, and not generate any runtime checks. If the store sizes are different and the distance is possibly zero, I think this existing code handles the case fine already:
```cpp
bool IsTrueDataDependence = (AIsWrite && !BIsWrite);
// Check if the first access writes to a location that is read in a later
// iteration, where the distance between them is not a multiple of a vector
// factor and relatively small.
//
// NOTE: There is no need to update MaxSafeVectorWidthInBits after call to
// couldPreventStoreLoadForward, even if it changed MinDepDistBytes, since a
// forward dependency will allow vectorization using any width.
if (IsTrueDataDependence && EnableForwardingConflictDetection) {
if (!ConstDist) {
// 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
// reason to.
FoundNonConstantDistanceDependence |= ShouldRetryWithRuntimeCheck;
return Dependence::Unknown;
}
if (couldPreventStoreLoadForward(
ConstDist->getAPInt().abs().getZExtValue(), TypeByteSize)) {
LLVM_DEBUG(
dbgs() << "LAA: Forward but may prevent st->ld forwarding\n");
return Dependence::ForwardButPreventsForwarding;
}
}
```
https://github.com/llvm/llvm-project/pull/122318
More information about the llvm-commits
mailing list