[llvm] [SLP] Fix PoisonValue in the argument VL of setOperand. (PR #118949)

via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 6 02:10:07 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Han-Kuan Chen (HanKuanChen)

<details>
<summary>Changes</summary>

In addition, Scalars is already in the struct, we don't need to pass VL here.

reference: https://github.com/llvm/llvm-project/pull/113880

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


2 Files Affected:

- (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+12-13) 
- (added) llvm/test/Transforms/SLPVectorizer/fix-113880.ll (+19) 


``````````diff
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 7ea039e04ca728..180238f4832293 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -3338,14 +3338,13 @@ class BoUpSLP {
       copy(OpVL, Operands[OpIdx].begin());
     }
 
-    /// Set this bundle's operand from \p VL.
-    void setOperand(ArrayRef<Value *> VL, const BoUpSLP &R,
-                    bool RequireReorder = false) {
-      VLOperands Ops(VL, R);
+    /// Set this bundle's operand from Scalars.
+    void setOperand(const BoUpSLP &R, bool RequireReorder = false) {
+      VLOperands Ops(Scalars, R);
       if (RequireReorder)
         Ops.reorder();
-      for (unsigned I :
-           seq<unsigned>(cast<Instruction>(VL[0])->getNumOperands()))
+      auto *I0 = cast<Instruction>(*find_if(Scalars, IsaPred<Instruction>));
+      for (unsigned I : seq<unsigned>(I0->getNumOperands()))
         setOperand(I, Ops.getVL(I));
     }
 
@@ -8446,7 +8445,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
                                    {}, CurrentOrder);
       LLVM_DEBUG(dbgs() << "SLP: added inserts bundle.\n");
 
-      TE->setOperand(VL, *this);
+      TE->setOperand(*this);
       buildTree_rec(TE->getOperand(1), Depth + 1, {TE, 1});
       return;
     }
@@ -8484,7 +8483,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
       case TreeEntry::NeedToGather:
         llvm_unreachable("Unexpected loads state.");
       }
-      TE->setOperand(VL, *this);
+      TE->setOperand(*this);
       if (State == TreeEntry::ScatterVectorize)
         buildTree_rec(PointerOps, Depth + 1, {TE, 0});
       return;
@@ -8524,7 +8523,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
                                    ReuseShuffleIndices);
       LLVM_DEBUG(dbgs() << "SLP: added a vector of casts.\n");
 
-      TE->setOperand(VL, *this);
+      TE->setOperand(*this);
       for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
         buildTree_rec(TE->getOperand(I), Depth + 1, {TE, I});
       if (ShuffleOrOp == Instruction::Trunc) {
@@ -8621,7 +8620,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
                                    ReuseShuffleIndices);
       LLVM_DEBUG(dbgs() << "SLP: added a vector of un/bin op.\n");
 
-      TE->setOperand(VL, *this, isa<BinaryOperator>(VL0) && isCommutative(VL0));
+      TE->setOperand(*this, isa<BinaryOperator>(VL0) && isCommutative(VL0));
       for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
         buildTree_rec(TE->getOperand(I), Depth + 1, {TE, I});
       return;
@@ -8687,7 +8686,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
         fixupOrderingIndices(CurrentOrder);
       TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
                                    ReuseShuffleIndices, CurrentOrder);
-      TE->setOperand(VL, *this);
+      TE->setOperand(*this);
       buildTree_rec(TE->getOperand(0), Depth + 1, {TE, 0});
       if (Consecutive)
         LLVM_DEBUG(dbgs() << "SLP: added a vector of stores.\n");
@@ -8703,7 +8702,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
 
       TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
                                    ReuseShuffleIndices);
-      TE->setOperand(VL, *this, isCommutative(VL0));
+      TE->setOperand(*this, isCommutative(VL0));
       for (unsigned I : seq<unsigned>(CI->arg_size())) {
         // For scalar operands no need to create an entry since no need to
         // vectorize it.
@@ -8759,7 +8758,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
         return;
       }
 
-      TE->setOperand(VL, *this, isa<BinaryOperator>(VL0) || CI);
+      TE->setOperand(*this, isa<BinaryOperator>(VL0) || CI);
       for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
         buildTree_rec(TE->getOperand(I), Depth + 1, {TE, I});
       return;
diff --git a/llvm/test/Transforms/SLPVectorizer/fix-113880.ll b/llvm/test/Transforms/SLPVectorizer/fix-113880.ll
new file mode 100644
index 00000000000000..6c3c95b0d9359f
--- /dev/null
+++ b/llvm/test/Transforms/SLPVectorizer/fix-113880.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=slp-vectorizer -S -slp-max-reg-size=1024 %s | FileCheck %s
+
+define ptr @test() {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:    store <4 x double> <double poison, double 0.000000e+00, double 0.000000e+00, double 0.000000e+00>, ptr null, align 8
+; CHECK-NEXT:    ret ptr null
+;
+  store double poison, ptr null, align 8
+  %1 = getelementptr i8, ptr null, i64 8
+  %2 = fmul double 0.000000e+00, 0.000000e+00
+  store double %2, ptr %1, align 8
+  %3 = getelementptr i8, ptr null, i64 16
+  %4 = fmul double 0.000000e+00, 0.000000e+00
+  store double %4, ptr %3, align 8
+  %5 = getelementptr i8, ptr null, i64 24
+  store double %2, ptr %5, align 8
+  ret ptr null
+}

``````````

</details>


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


More information about the llvm-commits mailing list