[llvm] [SLP] Fix PoisonValue in the argument VL of setOperand. (PR #118949)
Han-Kuan Chen via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 6 06:25:01 PST 2024
https://github.com/HanKuanChen updated https://github.com/llvm/llvm-project/pull/118949
>From 2a81c2c03389511ff1a33e9374b0cb27afe8dbbd Mon Sep 17 00:00:00 2001
From: Han-Kuan Chen <hankuan.chen at sifive.com>
Date: Fri, 6 Dec 2024 02:05:55 -0800
Subject: [PATCH 1/3] [SLP] Pre-commit test.
---
llvm/test/Transforms/SLPVectorizer/fix-113880.ll | 15 +++++++++++++++
1 file changed, 15 insertions(+)
create mode 100644 llvm/test/Transforms/SLPVectorizer/fix-113880.ll
diff --git a/llvm/test/Transforms/SLPVectorizer/fix-113880.ll b/llvm/test/Transforms/SLPVectorizer/fix-113880.ll
new file mode 100644
index 00000000000000..9d097c2081b0fd
--- /dev/null
+++ b/llvm/test/Transforms/SLPVectorizer/fix-113880.ll
@@ -0,0 +1,15 @@
+; 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() {
+ 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
+}
>From 4a407ef9e796729e61f4216b0a2cbe1321df6b21 Mon Sep 17 00:00:00 2001
From: Han-Kuan Chen <hankuan.chen at sifive.com>
Date: Fri, 6 Dec 2024 02:00:36 -0800
Subject: [PATCH 2/3] [SLP] Fix PoisonValue in the argument VL of setOperand.
In addition, Scalars is already in the struct, we don't need to pass VL here.
---
.../Transforms/Vectorize/SLPVectorizer.cpp | 25 +++++++++----------
.../Transforms/SLPVectorizer/fix-113880.ll | 4 +++
2 files changed, 16 insertions(+), 13 deletions(-)
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
index 9d097c2081b0fd..6c3c95b0d9359f 100644
--- a/llvm/test/Transforms/SLPVectorizer/fix-113880.ll
+++ b/llvm/test/Transforms/SLPVectorizer/fix-113880.ll
@@ -2,6 +2,10 @@
; 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
>From bd9491e6b310e06ca92843e58fde107a5d671ada Mon Sep 17 00:00:00 2001
From: Han-Kuan Chen <hankuan.chen at sifive.com>
Date: Fri, 6 Dec 2024 06:24:42 -0800
Subject: [PATCH 3/3] apply comment
---
.../Transforms/Vectorize/SLPVectorizer.cpp | 21 +++++++++++--------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 180238f4832293..23d4823f72dee4 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -2403,14 +2403,13 @@ class BoUpSLP {
}
/// Go through the instructions in VL and append their operands.
- void appendOperandsOfVL(ArrayRef<Value *> VL) {
+ void appendOperandsOfVL(ArrayRef<Value *> VL, Instruction *VL0) {
assert(!VL.empty() && "Bad VL");
assert((empty() || VL.size() == getNumLanes()) &&
"Expected same number of lanes");
// IntrinsicInst::isCommutative returns true if swapping the first "two"
// arguments to the intrinsic produces the same result.
constexpr unsigned IntrinsicNumOperands = 2;
- auto *VL0 = cast<Instruction>(*find_if(VL, IsaPred<Instruction>));
unsigned NumOperands = VL0->getNumOperands();
ArgSize = isa<IntrinsicInst>(VL0) ? IntrinsicNumOperands : NumOperands;
OpsVec.resize(NumOperands);
@@ -2542,15 +2541,19 @@ class BoUpSLP {
public:
/// Initialize with all the operands of the instruction vector \p RootVL.
- VLOperands(ArrayRef<Value *> RootVL, const BoUpSLP &R)
+ VLOperands(ArrayRef<Value *> RootVL, const BoUpSLP &R, Instruction *VL0)
: TLI(*R.TLI), DL(*R.DL), SE(*R.SE), R(R),
- L(R.LI->getLoopFor(
- (cast<Instruction>(*find_if(RootVL, IsaPred<Instruction>))
- ->getParent()))) {
+ L(R.LI->getLoopFor((VL0->getParent()))) {
// Append all the operands of RootVL.
- appendOperandsOfVL(RootVL);
+ appendOperandsOfVL(RootVL, VL0);
}
+ /// Initialize with all the operands of the instruction vector \p RootVL.
+ VLOperands(ArrayRef<Value *> RootVL, const BoUpSLP &R)
+ : VLOperands(
+ RootVL, R,
+ cast<Instruction>(*find_if(RootVL, IsaPred<Instruction>))) {}
+
/// \Returns a value vector with the operands across all lanes for the
/// opearnd at \p OpIdx.
ValueList getVL(unsigned OpIdx) const {
@@ -3340,10 +3343,10 @@ class BoUpSLP {
/// Set this bundle's operand from Scalars.
void setOperand(const BoUpSLP &R, bool RequireReorder = false) {
- VLOperands Ops(Scalars, R);
+ auto *I0 = cast<Instruction>(*find_if(Scalars, IsaPred<Instruction>));
+ VLOperands Ops(Scalars, R, I0);
if (RequireReorder)
Ops.reorder();
- auto *I0 = cast<Instruction>(*find_if(Scalars, IsaPred<Instruction>));
for (unsigned I : seq<unsigned>(I0->getNumOperands()))
setOperand(I, Ops.getVL(I));
}
More information about the llvm-commits
mailing list