[llvm] LSV: forbid load-cycles when vectorizing; fix bug (PR #104815)

[Ramkumar Ramachandra via llvm-commits]

https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/104815

>From 6bfb08b3559bfbfce6643e588bcd6a832aa0c96d Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 19 Aug 2024 16:53:31 +0100
Subject: [PATCH 1/2] LSV: forbid load-cycles when vectorizing; fix bug

Forbid zero SCEV diff in getConstantOffset, hence forbidding cycles
which would crash LoadStoreVectorizer when vectorizing.

Fixes #37865.
---
 .../Vectorize/LoadStoreVectorizer.cpp           |  6 ++++++
 .../LoadStoreVectorizer/AArch64/pr37865.ll      | 17 +++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp b/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
index c35ea431296b70..385349b338af93 100644
--- a/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
@@ -1499,6 +1499,12 @@ std::optional<APInt> Vectorizer::getConstantOffset(Value *PtrA, Value *PtrB,
 
   // Try to compute B - A.
   const SCEV *DistScev = SE.getMinusSCEV(SE.getSCEV(PtrB), SE.getSCEV(PtrA));
+  if (DistScev->isZero()) {
+    // A load in the chain is dependent on another load in the chain, and
+    // attempting to vectorize this chain would create a cycle.
+    LLVM_DEBUG(dbgs() << "LSV: SCEV diff is zero; not vectorizing\n");
+    return std::nullopt;
+  }
   if (DistScev != SE.getCouldNotCompute()) {
     LLVM_DEBUG(dbgs() << "LSV: SCEV PtrB - PtrA =" << *DistScev << "\n");
     ConstantRange DistRange = SE.getSignedRange(DistScev);
diff --git a/llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll b/llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll
index 833e70814c2917..bf5d8a555bd908 100644
--- a/llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll
+++ b/llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll
@@ -1,9 +1,18 @@
-; REQUIRES: asserts
-; RUN: not --crash opt -mtriple=aarch64 -passes=load-store-vectorizer \
-; RUN:   -disable-output %s 2>&1 | FileCheck %s
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -mtriple=aarch64 -passes=load-store-vectorizer -S %s | FileCheck %s
 
 define i32 @load_cycle(ptr %x) {
-; CHECK: Unexpected cycle while re-ordering instructions
+; CHECK-LABEL: define i32 @load_cycle(
+; CHECK-SAME: ptr [[X:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[GEP_X_1:%.*]] = getelementptr inbounds [2 x i32], ptr [[X]], i32 0, i32 1
+; CHECK-NEXT:    [[LOAD_X_1:%.*]] = load i32, ptr [[GEP_X_1]], align 4
+; CHECK-NEXT:    [[REM:%.*]] = urem i32 [[LOAD_X_1]], 1
+; CHECK-NEXT:    [[GEP_X_2:%.*]] = getelementptr inbounds [2 x i32], ptr [[X]], i32 [[REM]], i32 0
+; CHECK-NEXT:    [[LOAD_X_2:%.*]] = load i32, ptr [[GEP_X_2]], align 4
+; CHECK-NEXT:    [[RET:%.*]] = add i32 [[LOAD_X_2]], [[LOAD_X_1]]
+; CHECK-NEXT:    ret i32 [[RET]]
+;
 entry:
   %gep.x.1 = getelementptr inbounds [2 x i32], ptr %x, i32 0, i32 1
   %load.x.1 = load i32, ptr %gep.x.1

>From 4465fef283cc626ec26a8352cd06d450ae5f5659 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 19 Aug 2024 18:11:43 +0100
Subject: [PATCH 2/2] LSV: address review

---
 llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp       | 1 +
 llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp b/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
index 385349b338af93..187a6d3641dead 100644
--- a/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
@@ -1505,6 +1505,7 @@ std::optional<APInt> Vectorizer::getConstantOffset(Value *PtrA, Value *PtrB,
     LLVM_DEBUG(dbgs() << "LSV: SCEV diff is zero; not vectorizing\n");
     return std::nullopt;
   }
+
   if (DistScev != SE.getCouldNotCompute()) {
     LLVM_DEBUG(dbgs() << "LSV: SCEV PtrB - PtrA =" << *DistScev << "\n");
     ConstantRange DistRange = SE.getSignedRange(DistScev);
diff --git a/llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll b/llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll
index bf5d8a555bd908..d657ea03cff82e 100644
--- a/llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll
+++ b/llvm/test/Transforms/LoadStoreVectorizer/AArch64/pr37865.ll
@@ -1,6 +1,10 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt -mtriple=aarch64 -passes=load-store-vectorizer -S %s | FileCheck %s
 
+; LSV was attempting to vectorize this earlier, but crashed while re-ordering
+; instructions due to the load-load cycle. Now, the chain is no longer a
+; candidate for vectorization.
+
 define i32 @load_cycle(ptr %x) {
 ; CHECK-LABEL: define i32 @load_cycle(
 ; CHECK-SAME: ptr [[X:%.*]]) {