[llvm] [DAG] SDPatternMatch - Fix m_Reassociatable mismatching (PR #170061)
Artur Bermond Torres via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 5 11:15:26 PST 2025
https://github.com/bermondd updated https://github.com/llvm/llvm-project/pull/170061
>From ab6d06a1b3c810cbaa1d16cd46267a56c03fb3ce Mon Sep 17 00:00:00 2001
From: Artur Bermond Torres <41002679+bermondd at users.noreply.github.com>
Date: Sun, 30 Nov 2025 19:36:53 -0300
Subject: [PATCH 1/5] [DAG] SDPatternMatch - Fix m_Reassociatable mismatching
---
llvm/include/llvm/CodeGen/SDPatternMatch.h | 41 ++++++++++---------
.../CodeGen/SelectionDAGPatternMatchTest.cpp | 10 +++++
2 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/SDPatternMatch.h b/llvm/include/llvm/CodeGen/SDPatternMatch.h
index daafd3fc9d825..435f340c5b9c9 100644
--- a/llvm/include/llvm/CodeGen/SDPatternMatch.h
+++ b/llvm/include/llvm/CodeGen/SDPatternMatch.h
@@ -1315,19 +1315,12 @@ template <typename... PatternTs> struct ReassociatableOpc_match {
if (Leaves.size() != NumPatterns)
return false;
- // Matches[I][J] == true iff sd_context_match(Leaves[I], Ctx,
- // std::get<J>(Patterns)) == true
- std::array<SmallBitVector, NumPatterns> Matches;
- for (size_t I = 0; I != NumPatterns; I++) {
- std::apply(
- [&](auto &...P) {
- (Matches[I].push_back(sd_context_match(Leaves[I], Ctx, P)), ...);
- },
- Patterns);
- }
-
SmallBitVector Used(NumPatterns);
- return reassociatableMatchHelper(Matches, Used);
+ return std::apply(
+ [&](auto &...P) -> bool {
+ return reassociatableMatchHelper(Ctx, Leaves, Used, P...);
+ },
+ Patterns);
}
void collectLeaves(SDValue V, SmallVector<SDValue> &Leaves) {
@@ -1339,21 +1332,31 @@ template <typename... PatternTs> struct ReassociatableOpc_match {
}
}
+ // Searchs for a matching leaf for every sub-pattern.
+ template <typename MatchContext, typename PatternHd, typename... PatternTl>
[[nodiscard]] inline bool
- reassociatableMatchHelper(ArrayRef<SmallBitVector> Matches,
- SmallBitVector &Used, size_t Curr = 0) {
- if (Curr == Matches.size())
- return true;
- for (size_t Match = 0, N = Matches[Curr].size(); Match < N; Match++) {
- if (!Matches[Curr][Match] || Used[Match])
+ reassociatableMatchHelper(const MatchContext &Ctx,
+ SmallVector<SDValue> &Leaves, SmallBitVector &Used,
+ PatternHd &HeadPattern,
+ PatternTl &...TailPatterns) {
+ for (size_t Match = 0, N = Used.size(); Match < N; Match++) {
+ if (Used[Match] || !(sd_context_match(Leaves[Match], Ctx, HeadPattern)))
continue;
Used[Match] = true;
- if (reassociatableMatchHelper(Matches, Used, Curr + 1))
+ if (reassociatableMatchHelper(Ctx, Leaves, Used, TailPatterns...))
return true;
Used[Match] = false;
}
return false;
}
+
+ template <typename MatchContext>
+ [[nodiscard]] inline bool
+ reassociatableMatchHelper(const MatchContext &Ctx,
+ SmallVector<SDValue> &Leaves,
+ SmallBitVector &Used) {
+ return true;
+ }
};
template <typename... PatternTs>
diff --git a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
index c32ceee73472d..f071b4133e7dc 100644
--- a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
+++ b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
@@ -842,6 +842,16 @@ TEST_F(SelectionDAGPatternMatchTest, matchReassociatableOp) {
EXPECT_TRUE(sd_match(
MUL, m_ReassociatableMul(m_Value(), m_Value(), m_Value(), m_Value())));
+ // (Op0 + Op1) + Op0 binds correctly, allowing commutation
+ SDValue ADD010 = DAG->getNode(ISD::ADD, DL, Int32VT, ADD01, Op0);
+ SDValue A, B;
+ EXPECT_TRUE(sd_match(
+ ADD010, m_ReassociatableAdd(m_Value(A), m_Value(B), m_Deferred(A))));
+ EXPECT_TRUE(sd_match(
+ ADD010, m_ReassociatableAdd(m_Value(A), m_Value(B), m_Deferred(B))));
+ EXPECT_FALSE(sd_match(
+ ADD010, m_ReassociatableAdd(m_Value(A), m_Deferred(A), m_Deferred(A))));
+
// Op0 * (Op1 * (Op2 * Op3))
SDValue MUL123 = DAG->getNode(ISD::MUL, DL, Int32VT, Op1, MUL23);
SDValue MUL0123 = DAG->getNode(ISD::MUL, DL, Int32VT, Op0, MUL123);
>From d989ca18b25d0e264cc6b8e61f5f07ea3300bf86 Mon Sep 17 00:00:00 2001
From: Artur Bermond Torres <git-arturbtorres at proton.me>
Date: Thu, 4 Dec 2025 13:10:36 -0300
Subject: [PATCH 2/5] [DAG] SDPatternMatch - Changed SmallVector to ArrayRef
and added more tests
---
llvm/include/llvm/CodeGen/SDPatternMatch.h | 4 ++--
.../CodeGen/SelectionDAGPatternMatchTest.cpp | 12 ++++++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/SDPatternMatch.h b/llvm/include/llvm/CodeGen/SDPatternMatch.h
index 435f340c5b9c9..9431d542f3267 100644
--- a/llvm/include/llvm/CodeGen/SDPatternMatch.h
+++ b/llvm/include/llvm/CodeGen/SDPatternMatch.h
@@ -1336,7 +1336,7 @@ template <typename... PatternTs> struct ReassociatableOpc_match {
template <typename MatchContext, typename PatternHd, typename... PatternTl>
[[nodiscard]] inline bool
reassociatableMatchHelper(const MatchContext &Ctx,
- SmallVector<SDValue> &Leaves, SmallBitVector &Used,
+ ArrayRef<SDValue> Leaves, SmallBitVector &Used,
PatternHd &HeadPattern,
PatternTl &...TailPatterns) {
for (size_t Match = 0, N = Used.size(); Match < N; Match++) {
@@ -1353,7 +1353,7 @@ template <typename... PatternTs> struct ReassociatableOpc_match {
template <typename MatchContext>
[[nodiscard]] inline bool
reassociatableMatchHelper(const MatchContext &Ctx,
- SmallVector<SDValue> &Leaves,
+ ArrayRef<SDValue> Leaves,
SmallBitVector &Used) {
return true;
}
diff --git a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
index f071b4133e7dc..69e48758b6be5 100644
--- a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
+++ b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
@@ -847,8 +847,20 @@ TEST_F(SelectionDAGPatternMatchTest, matchReassociatableOp) {
SDValue A, B;
EXPECT_TRUE(sd_match(
ADD010, m_ReassociatableAdd(m_Value(A), m_Value(B), m_Deferred(A))));
+ EXPECT_TRUE(sd_match(Op0, m_Deferred(A)));
+ EXPECT_TRUE(sd_match(Op1, m_Deferred(B)));
+ EXPECT_FALSE(sd_match(Op0, m_Deferred(B)));
+ EXPECT_FALSE(sd_match(Op1, m_Deferred(A)));
+ A.setNode(nullptr);
+ B.setNode(nullptr);
EXPECT_TRUE(sd_match(
ADD010, m_ReassociatableAdd(m_Value(A), m_Value(B), m_Deferred(B))));
+ EXPECT_TRUE(sd_match(Op0, m_Deferred(B)));
+ EXPECT_TRUE(sd_match(Op1, m_Deferred(A)));
+ EXPECT_FALSE(sd_match(Op0, m_Deferred(A)));
+ EXPECT_FALSE(sd_match(Op1, m_Deferred(B)));
+ A.setNode(nullptr);
+ B.setNode(nullptr);
EXPECT_FALSE(sd_match(
ADD010, m_ReassociatableAdd(m_Value(A), m_Deferred(A), m_Deferred(A))));
>From f049fd9924bf1f1100e9144c7d0330e4cac7decf Mon Sep 17 00:00:00 2001
From: Artur Bermond Torres <git-arturbtorres at proton.me>
Date: Thu, 4 Dec 2025 13:40:20 -0300
Subject: [PATCH 3/5] fixup! [DAG] SDPatternMatch - Changed SmallVector to
ArrayRef and added more tests
---
llvm/include/llvm/CodeGen/SDPatternMatch.h | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/SDPatternMatch.h b/llvm/include/llvm/CodeGen/SDPatternMatch.h
index 9431d542f3267..dda3b3827c7aa 100644
--- a/llvm/include/llvm/CodeGen/SDPatternMatch.h
+++ b/llvm/include/llvm/CodeGen/SDPatternMatch.h
@@ -1335,9 +1335,8 @@ template <typename... PatternTs> struct ReassociatableOpc_match {
// Searchs for a matching leaf for every sub-pattern.
template <typename MatchContext, typename PatternHd, typename... PatternTl>
[[nodiscard]] inline bool
- reassociatableMatchHelper(const MatchContext &Ctx,
- ArrayRef<SDValue> Leaves, SmallBitVector &Used,
- PatternHd &HeadPattern,
+ reassociatableMatchHelper(const MatchContext &Ctx, ArrayRef<SDValue> Leaves,
+ SmallBitVector &Used, PatternHd &HeadPattern,
PatternTl &...TailPatterns) {
for (size_t Match = 0, N = Used.size(); Match < N; Match++) {
if (Used[Match] || !(sd_context_match(Leaves[Match], Ctx, HeadPattern)))
@@ -1351,10 +1350,9 @@ template <typename... PatternTs> struct ReassociatableOpc_match {
}
template <typename MatchContext>
- [[nodiscard]] inline bool
- reassociatableMatchHelper(const MatchContext &Ctx,
- ArrayRef<SDValue> Leaves,
- SmallBitVector &Used) {
+ [[nodiscard]] inline bool reassociatableMatchHelper(const MatchContext &Ctx,
+ ArrayRef<SDValue> Leaves,
+ SmallBitVector &Used) {
return true;
}
};
>From 15adee2d3c1fbe941c54ff770929394e7cae7d50 Mon Sep 17 00:00:00 2001
From: Artur Bermond Torres <git-arturbtorres at proton.me>
Date: Thu, 4 Dec 2025 18:09:11 -0300
Subject: [PATCH 4/5] Moved from EXPECT_TRUE to EXPECT_EQ, where appropriate
---
.../CodeGen/SelectionDAGPatternMatchTest.cpp | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
index 69e48758b6be5..2bc165631080c 100644
--- a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
+++ b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
@@ -847,18 +847,18 @@ TEST_F(SelectionDAGPatternMatchTest, matchReassociatableOp) {
SDValue A, B;
EXPECT_TRUE(sd_match(
ADD010, m_ReassociatableAdd(m_Value(A), m_Value(B), m_Deferred(A))));
- EXPECT_TRUE(sd_match(Op0, m_Deferred(A)));
- EXPECT_TRUE(sd_match(Op1, m_Deferred(B)));
- EXPECT_FALSE(sd_match(Op0, m_Deferred(B)));
- EXPECT_FALSE(sd_match(Op1, m_Deferred(A)));
+ EXPECT_EQ(Op0, A);
+ EXPECT_EQ(Op1, B);
+ EXPECT_NE(A, B);
+
A.setNode(nullptr);
B.setNode(nullptr);
EXPECT_TRUE(sd_match(
ADD010, m_ReassociatableAdd(m_Value(A), m_Value(B), m_Deferred(B))));
- EXPECT_TRUE(sd_match(Op0, m_Deferred(B)));
- EXPECT_TRUE(sd_match(Op1, m_Deferred(A)));
- EXPECT_FALSE(sd_match(Op0, m_Deferred(A)));
- EXPECT_FALSE(sd_match(Op1, m_Deferred(B)));
+ EXPECT_EQ(Op0, B);
+ EXPECT_EQ(Op1, A);
+ EXPECT_NE(A, B);
+
A.setNode(nullptr);
B.setNode(nullptr);
EXPECT_FALSE(sd_match(
>From d9833cf3f310ab14d464f1d9a076f528eb0e3cf9 Mon Sep 17 00:00:00 2001
From: Artur Bermond Torres <git-arturbtorres at proton.me>
Date: Fri, 5 Dec 2025 16:14:03 -0300
Subject: [PATCH 5/5] Moved test lines upward; removed redundant EXPECT_NE;
added new tests
---
.../CodeGen/SelectionDAGPatternMatchTest.cpp | 36 ++++++++++++-------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
index 2bc165631080c..4fcd3fcb8c5c7 100644
--- a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
+++ b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp
@@ -832,24 +832,13 @@ TEST_F(SelectionDAGPatternMatchTest, matchReassociatableOp) {
EXPECT_FALSE(sd_match(ADDS0123, m_ReassociatableAdd(m_Value(), m_Value(),
m_Value(), m_Value())));
- // (Op0 * Op1) * (Op2 * Op3)
- SDValue MUL01 = DAG->getNode(ISD::MUL, DL, Int32VT, Op0, Op1);
- SDValue MUL23 = DAG->getNode(ISD::MUL, DL, Int32VT, Op2, Op3);
- SDValue MUL = DAG->getNode(ISD::MUL, DL, Int32VT, MUL01, MUL23);
-
- EXPECT_TRUE(sd_match(MUL01, m_ReassociatableMul(m_Value(), m_Value())));
- EXPECT_TRUE(sd_match(MUL23, m_ReassociatableMul(m_Value(), m_Value())));
- EXPECT_TRUE(sd_match(
- MUL, m_ReassociatableMul(m_Value(), m_Value(), m_Value(), m_Value())));
-
- // (Op0 + Op1) + Op0 binds correctly, allowing commutation
+ // (Op0 + Op1) + Op0 binds correctly, allowing commutation on leaf nodes
SDValue ADD010 = DAG->getNode(ISD::ADD, DL, Int32VT, ADD01, Op0);
SDValue A, B;
EXPECT_TRUE(sd_match(
ADD010, m_ReassociatableAdd(m_Value(A), m_Value(B), m_Deferred(A))));
EXPECT_EQ(Op0, A);
EXPECT_EQ(Op1, B);
- EXPECT_NE(A, B);
A.setNode(nullptr);
B.setNode(nullptr);
@@ -857,13 +846,34 @@ TEST_F(SelectionDAGPatternMatchTest, matchReassociatableOp) {
ADD010, m_ReassociatableAdd(m_Value(A), m_Value(B), m_Deferred(B))));
EXPECT_EQ(Op0, B);
EXPECT_EQ(Op1, A);
- EXPECT_NE(A, B);
+
+ A.setNode(nullptr);
+ B.setNode(nullptr);
+ EXPECT_TRUE(sd_match(
+ ADD010, m_ReassociatableAdd(m_Value(A), m_Deferred(A), m_Value(B))));
+ EXPECT_EQ(Op0, A);
+ EXPECT_EQ(Op1, B);
A.setNode(nullptr);
B.setNode(nullptr);
EXPECT_FALSE(sd_match(
ADD010, m_ReassociatableAdd(m_Value(A), m_Deferred(A), m_Deferred(A))));
+ A.setNode(nullptr);
+ B.setNode(nullptr);
+ EXPECT_FALSE(sd_match(
+ ADD010, m_ReassociatableAdd(m_Value(A), m_Deferred(B), m_Value(B))));
+
+ // (Op0 * Op1) * (Op2 * Op3)
+ SDValue MUL01 = DAG->getNode(ISD::MUL, DL, Int32VT, Op0, Op1);
+ SDValue MUL23 = DAG->getNode(ISD::MUL, DL, Int32VT, Op2, Op3);
+ SDValue MUL = DAG->getNode(ISD::MUL, DL, Int32VT, MUL01, MUL23);
+
+ EXPECT_TRUE(sd_match(MUL01, m_ReassociatableMul(m_Value(), m_Value())));
+ EXPECT_TRUE(sd_match(MUL23, m_ReassociatableMul(m_Value(), m_Value())));
+ EXPECT_TRUE(sd_match(
+ MUL, m_ReassociatableMul(m_Value(), m_Value(), m_Value(), m_Value())));
+
// Op0 * (Op1 * (Op2 * Op3))
SDValue MUL123 = DAG->getNode(ISD::MUL, DL, Int32VT, Op1, MUL23);
SDValue MUL0123 = DAG->getNode(ISD::MUL, DL, Int32VT, Op0, MUL123);
More information about the llvm-commits
mailing list