[llvm] [LAA] Don't multiply NumItersForStoreLoadThroughMemory by TypeByteSize. (PR #75382)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: Florian Hahn (fhahn)

<details>
<summary>Changes</summary>

NumItersForStoreLoadThroughMemory is only used in the following expression `Distance / VF < NumItersForStoreLoadThroughMemory`

Both Distance and VF are scaled by TypeByteSize, so the division cancels out TypeByteSize. Presumably the reason for scaling NumItersForStoreLoadThroughMemory by TypeByteSize was that VF and Distance are also scaled. But due to the divison canceling out the scaling, there should be no need to scale by TypeByteSize, unless I am missing a different reason for the additional scaling. Requiring more iterations for wider types seems a bit counter-intuitive to me as I would assume that hardware in general won't be able to track more wider than narrower loads.

The scaling leads to a very large number of iterations being needed to rule out store-to-load prevention, e.g. see the examples in the tests and also the example from
https://discourse.llvm.org/t/rfc-laa-should-we-disable-the-store-to-load-forwarding-conflict-detection/75314 which has a distance of 296.

---
Full diff: https://github.com/llvm/llvm-project/pull/75382.diff


3 Files Affected:

- (modified) llvm/lib/Analysis/LoopAccessAnalysis.cpp (+1-1) 
- (modified) llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll (+2-2) 
- (modified) llvm/test/Analysis/LoopAccessAnalysis/num-iters-for-store-load-conflict.ll (+6-9) 


``````````diff
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 0894560fd07898..f4023f71f3cc97 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -1734,7 +1734,7 @@ bool MemoryDepChecker::couldPreventStoreLoadForward(uint64_t Distance,
 
   // After this many iterations store-to-load forwarding conflicts should not
   // cause any slowdowns.
-  const uint64_t NumItersForStoreLoadThroughMemory = 8 * TypeByteSize;
+  const uint64_t NumItersForStoreLoadThroughMemory = 8;
   // Maximum vector factor.
   uint64_t MaxVFWithoutSLForwardIssues = std::min(
       VectorizerParams::MaxVectorWidth * TypeByteSize, MinDepDistBytes);
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll b/llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll
index 8c436de4c3f6ac..eeffc5ba3e3cbe 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll
@@ -127,11 +127,11 @@ exit:
 ; CHECK-NEXT:            %ld.i64 = load i64, ptr %gep.iv, align 8 ->
 ; CHECK-NEXT:		         store i32 %ld.i64.i32, ptr %gep.iv.n.i64, align 8
 ; CHECK-EMPTY:
-; CHECK-NEXT:        BackwardVectorizableButPreventsForwarding:
+; CHECK-NEXT:        BackwardVectorizable:
 ; CHECK-NEXT:            %ld.f64 = load double, ptr %gep.iv, align 8 ->
 ; CHECK-NEXT:            store double %val, ptr %gep.iv.101.i64, align 8
 ; CHECK-EMPTY:
-; CHECK-NEXT:        ForwardButPreventsForwarding:
+; CHECK-NEXT:        Forward:
 ; CHECK-NEXT:            store double %val, ptr %gep.iv.101.i64, align 8 ->
 ; CHECK-NEXT:            %ld.i64 = load i64, ptr %gep.iv, align 8
 ; CHECK-EMPTY:
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/num-iters-for-store-load-conflict.ll b/llvm/test/Analysis/LoopAccessAnalysis/num-iters-for-store-load-conflict.ll
index d3eda21dee27e6..8441c81cf1ce6e 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/num-iters-for-store-load-conflict.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/num-iters-for-store-load-conflict.ll
@@ -184,10 +184,9 @@ exit:
 define void @forward_dist_17(ptr %A, ptr noalias %B) {
 ; CHECK-LABEL: 'forward_dist_17'
 ; CHECK-NEXT:    loop:
-; 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
-; CHECK-NEXT:  Forward loop carried data dependence that prevents store-to-load forwarding.
+; CHECK-NEXT:      Memory dependences are safe
 ; CHECK-NEXT:      Dependences:
-; CHECK-NEXT:        ForwardButPreventsForwarding:
+; CHECK-NEXT:        Forward:
 ; CHECK-NEXT:            store i32 0, ptr %gep.2, align 4 ->
 ; CHECK-NEXT:            %l = load i32, ptr %gep.1, align 4
 ; CHECK-EMPTY:
@@ -220,10 +219,9 @@ exit:
 define void @forward_dist_19(ptr %A, ptr noalias %B) {
 ; CHECK-LABEL: 'forward_dist_19'
 ; CHECK-NEXT:    loop:
-; 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
-; CHECK-NEXT:  Forward loop carried data dependence that prevents store-to-load forwarding.
+; CHECK-NEXT:      Memory dependences are safe
 ; CHECK-NEXT:      Dependences:
-; CHECK-NEXT:        ForwardButPreventsForwarding:
+; CHECK-NEXT:        Forward:
 ; CHECK-NEXT:            store i32 0, ptr %gep.2, align 4 ->
 ; CHECK-NEXT:            %l = load i32, ptr %gep.1, align 4
 ; CHECK-EMPTY:
@@ -258,10 +256,9 @@ exit:
 define void @unknown_loop_bounds(i64 %x, i64 %y) {
 ; CHECK-LABEL: 'unknown_loop_bounds'
 ; CHECK-NEXT:    inner:
-; 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
-; CHECK-NEXT:  Backward loop carried data dependence that prevents store-to-load forwarding.
+; CHECK-NEXT:      Memory dependences are safe with a maximum safe vector width of 256 bits
 ; CHECK-NEXT:      Dependences:
-; CHECK-NEXT:        BackwardVectorizableButPreventsForwarding:
+; CHECK-NEXT:        BackwardVectorizable:
 ; CHECK-NEXT:            %l = load double, ptr %gep.0, align 8 ->
 ; CHECK-NEXT:            store double %l, ptr %gep.1, align 8
 ; CHECK-EMPTY:

``````````

</details>


https://github.com/llvm/llvm-project/pull/75382