[llvm] [LAA] Fix incorrect dependency classification. (PR #70819)

[Alexandros Lamprineas via llvm-commits]

https://github.com/labrinea updated https://github.com/llvm/llvm-project/pull/70819

>From 76ef2767ef5a77e400562f1a6d093bbb96cb699a Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Tue, 31 Oct 2023 15:23:40 +0000
Subject: [PATCH] [LAA] Fix incorrect dependency classification.

As shown in #70473, the following loop was not considered safe to vectorize.
When determining the memory access dependencies in a loop which has negative
iteration step, we invert the source and sink of the dependence. Perhaps we
should just invert the operands to getMinusSCEV(). This way the dependency
is not regarded to be true.

void vectorizable_Read_Write(int *A) {
  for (unsigned i = 1022; i >= 0; i--)
    A[i+1] = A[i] + 1;
}
---
 llvm/lib/Analysis/LoopAccessAnalysis.cpp           | 14 +++-----------
 .../LoopAccessAnalysis/forward-negative-step.ll    | 13 +++++--------
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 6803960585d07ea..ce49a0c334303c2 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -1906,18 +1906,10 @@ MemoryDepChecker::Dependence::DepType MemoryDepChecker::isDependent(
   const SCEV *Sink = PSE.getSCEV(BPtr);
 
   // If the induction step is negative we have to invert source and sink of the
-  // dependence.
-  if (StrideAPtr < 0) {
-    std::swap(APtr, BPtr);
-    std::swap(ATy, BTy);
-    std::swap(Src, Sink);
-    std::swap(AIsWrite, BIsWrite);
-    std::swap(AIdx, BIdx);
-    std::swap(StrideAPtr, StrideBPtr);
-  }
-
+  // dependence when measuring the distance between them.
   ScalarEvolution &SE = *PSE.getSE();
-  const SCEV *Dist = SE.getMinusSCEV(Sink, Src);
+  const SCEV *Dist =
+      StrideAPtr < 0 ? SE.getMinusSCEV(Src, Sink) : SE.getMinusSCEV(Sink, Src);
 
   LLVM_DEBUG(dbgs() << "LAA: Src Scev: " << *Src << "Sink Scev: " << *Sink
                     << "(Induction step: " << StrideAPtr << ")\n");
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/forward-negative-step.ll b/llvm/test/Analysis/LoopAccessAnalysis/forward-negative-step.ll
index 35e2109ab74476d..46e81cd74ab31fe 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/forward-negative-step.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/forward-negative-step.ll
@@ -3,8 +3,6 @@
 
 target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
 
-; FIXME: This should be vectorizable
-
 ; void vectorizable_Read_Write(int *A) {
 ;  for (unsigned i = 1022; i >= 0; i--)
 ;    A[i+1] = A[i] + 1;
@@ -13,10 +11,9 @@ target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
 define void @vectorizable_Read_Write(ptr nocapture %A) {
 ; CHECK-LABEL: 'vectorizable_Read_Write'
 ; 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:            %l = load i32, ptr %gep.A, align 4 ->
 ; CHECK-NEXT:            store i32 %add, ptr %gep.A.plus.1, align 4
 ; CHECK-EMPTY:
@@ -47,13 +44,13 @@ exit:
   ret void
 }
 
-; FIXME: There's a forward dependency that prevents forwarding here.
 define void @neg_step_ForwardButPreventsForwarding(ptr nocapture %A, ptr noalias %B) {
 ; CHECK-LABEL: 'neg_step_ForwardButPreventsForwarding'
 ; CHECK-NEXT:    loop:
-; CHECK-NEXT:      Memory dependences are safe
+; 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:      Dependences:
-; CHECK-NEXT:        Forward:
+; CHECK-NEXT:        ForwardButPreventsForwarding:
 ; CHECK-NEXT:            store i32 0, ptr %gep.A, align 4 ->
 ; CHECK-NEXT:            %l = load i32, ptr %gep.A.plus.1, align 4
 ; CHECK-EMPTY: