[llvm] [SLP] Fix isCommutative to check uses of the original instruction instead of the converted instruction. (PR #143094)
Han-Kuan Chen via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 7 02:15:02 PDT 2025
https://github.com/HanKuanChen updated https://github.com/llvm/llvm-project/pull/143094
>From 5365abc1d6a89e3a3e5b9fbb18029ff306256c2d Mon Sep 17 00:00:00 2001
From: Han-Kuan Chen <hankuan.chen at sifive.com>
Date: Fri, 6 Jun 2025 01:33:05 -0700
Subject: [PATCH 1/3] [SLP] Pre-commit test.
---
.../Transforms/SLPVectorizer/bbi-107513.ll | 34 +++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100644 llvm/test/Transforms/SLPVectorizer/bbi-107513.ll
diff --git a/llvm/test/Transforms/SLPVectorizer/bbi-107513.ll b/llvm/test/Transforms/SLPVectorizer/bbi-107513.ll
new file mode 100644
index 0000000000000..4dd768e704c75
--- /dev/null
+++ b/llvm/test/Transforms/SLPVectorizer/bbi-107513.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=slp-vectorizer -S %s | FileCheck %s
+
+define i16 @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[COND3:%.*]] = select i1 false, i16 1, i16 0
+; CHECK-NEXT: ret i16 [[COND3]]
+;
+entry:
+ %sub = sub i16 0, -1
+ %cmp = icmp eq i16 %sub, 1
+
+ %sub1 = sub i16 0, -1
+ %cmp2 = icmp eq i16 %sub1, 1
+ %cond3 = select i1 %cmp2, i16 1, i16 0
+
+ %sub5 = sub nsw i16 0, 0
+ %cmp6 = icmp eq i16 %sub5, 0
+ %cmp9 = icmp eq i16 %sub5, 0
+
+ %sub12 = sub nsw i16 0, 0
+ %cmp13 = icmp eq i16 %sub12, 0
+
+ %sub16 = sub nsw i16 0, 0
+ %cmp17 = icmp eq i16 %sub16, 0
+
+ %sub20 = sub nsw i16 0, 0
+ %cmp21 = icmp eq i16 %sub20, 0
+ %cmp24 = icmp eq i16 %sub20, 0
+
+ ret i16 %cond3
+}
+
>From 0b666c769c38a9e9ca64f2b127a037a1817b043b Mon Sep 17 00:00:00 2001
From: Han-Kuan Chen <hankuan.chen at sifive.com>
Date: Fri, 6 Jun 2025 01:43:19 -0700
Subject: [PATCH 2/3] [SLP] Fix isCommutative to check uses of the original
instruction instead of the converted instruction.
---
.../lib/Transforms/Vectorize/SLPVectorizer.cpp | 18 ++++++++++++------
.../Transforms/SLPVectorizer/bbi-107513.ll | 2 +-
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 6fcd606afaa22..3d24f31e37918 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -511,15 +511,15 @@ static bool isSplat(ArrayRef<Value *> VL) {
}
/// \returns True if \p I is commutative, handles CmpInst and BinaryOperator.
-static bool isCommutative(Instruction *I) {
+static bool isCommutative(Instruction *I, Instruction *InstWithUses) {
if (auto *Cmp = dyn_cast<CmpInst>(I))
return Cmp->isCommutative();
if (auto *BO = dyn_cast<BinaryOperator>(I))
return BO->isCommutative() ||
(BO->getOpcode() == Instruction::Sub &&
- !BO->hasNUsesOrMore(UsesLimit) &&
+ !InstWithUses->hasNUsesOrMore(UsesLimit) &&
all_of(
- BO->uses(),
+ InstWithUses->uses(),
[](const Use &U) {
// Commutative, if icmp eq/ne sub, 0
CmpPredicate Pred;
@@ -536,14 +536,16 @@ static bool isCommutative(Instruction *I) {
Flag->isOne());
})) ||
(BO->getOpcode() == Instruction::FSub &&
- !BO->hasNUsesOrMore(UsesLimit) &&
- all_of(BO->uses(), [](const Use &U) {
+ !InstWithUses->hasNUsesOrMore(UsesLimit) &&
+ all_of(InstWithUses->uses(), [](const Use &U) {
return match(U.getUser(),
m_Intrinsic<Intrinsic::fabs>(m_Specific(U.get())));
}));
return I->isCommutative();
}
+static bool isCommutative(Instruction *I) { return isCommutative(I, I); }
+
template <typename T>
static std::optional<unsigned> getInsertExtractIndex(const Value *Inst,
unsigned Offset) {
@@ -2898,7 +2900,11 @@ class BoUpSLP {
continue;
}
auto [SelectedOp, Ops] = convertTo(cast<Instruction>(V), S);
- bool IsInverseOperation = !isCommutative(SelectedOp);
+ // We cannot check commutativity by the converted instruction
+ // (SelectedOp) because isCommutative also examines def-use
+ // relationships.
+ bool IsInverseOperation =
+ !isCommutative(SelectedOp, cast<Instruction>(V));
for (unsigned OpIdx : seq<unsigned>(ArgSize)) {
bool APO = (OpIdx == 0) ? false : IsInverseOperation;
OpsVec[OpIdx][Lane] = {Operands[OpIdx][Lane], APO, false};
diff --git a/llvm/test/Transforms/SLPVectorizer/bbi-107513.ll b/llvm/test/Transforms/SLPVectorizer/bbi-107513.ll
index 4dd768e704c75..14ead464943de 100644
--- a/llvm/test/Transforms/SLPVectorizer/bbi-107513.ll
+++ b/llvm/test/Transforms/SLPVectorizer/bbi-107513.ll
@@ -4,7 +4,7 @@
define i16 @foo() {
; CHECK-LABEL: @foo(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[COND3:%.*]] = select i1 false, i16 1, i16 0
+; CHECK-NEXT: [[COND3:%.*]] = select i1 true, i16 1, i16 0
; CHECK-NEXT: ret i16 [[COND3]]
;
entry:
>From ed476d6bb8936392851f1e1ee450663576457e6e Mon Sep 17 00:00:00 2001
From: Han-Kuan Chen <hankuan.chen at sifive.com>
Date: Sat, 7 Jun 2025 02:14:51 -0700
Subject: [PATCH 3/3] add comments
---
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 3d24f31e37918..55f8719c24970 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -511,6 +511,12 @@ static bool isSplat(ArrayRef<Value *> VL) {
}
/// \returns True if \p I is commutative, handles CmpInst and BinaryOperator.
+/// For BinaryOperator, it also checks if \p InstWithUses is used in specific
+/// patterns that make it effectively commutative (like equality comparisons
+/// with zero).
+/// \param I The instruction to check for commutativity
+/// \param InstWithUses The instruction whose uses are analyzed for special
+/// patterns
static bool isCommutative(Instruction *I, Instruction *InstWithUses) {
if (auto *Cmp = dyn_cast<CmpInst>(I))
return Cmp->isCommutative();
More information about the llvm-commits
mailing list