From ab6d06a1b3c810cbaa1d16cd46267a56c03fb3ce Mon Sep 17 00:00:00 2001 From: Artur Bermond Torres <41002679+bermondd@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 struct ReassociatableOpc_match { if (Leaves.size() != NumPatterns) return false; - // Matches[I][J] == true iff sd_context_match(Leaves[I], Ctx, - // std::get(Patterns)) == true - std::array 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 &Leaves) { @@ -1339,21 +1332,31 @@ template struct ReassociatableOpc_match { } } + // Searchs for a matching leaf for every sub-pattern. + template [[nodiscard]] inline bool - reassociatableMatchHelper(ArrayRef 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 &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 + [[nodiscard]] inline bool + reassociatableMatchHelper(const MatchContext &Ctx, + SmallVector &Leaves, + SmallBitVector &Used) { + return true; + } }; template 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 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 struct ReassociatableOpc_match { template [[nodiscard]] inline bool reassociatableMatchHelper(const MatchContext &Ctx, - SmallVector &Leaves, SmallBitVector &Used, + ArrayRef Leaves, SmallBitVector &Used, PatternHd &HeadPattern, PatternTl &...TailPatterns) { for (size_t Match = 0, N = Used.size(); Match < N; Match++) { @@ -1353,7 +1353,7 @@ template struct ReassociatableOpc_match { template [[nodiscard]] inline bool reassociatableMatchHelper(const MatchContext &Ctx, - SmallVector &Leaves, + ArrayRef 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 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 struct ReassociatableOpc_match { // Searchs for a matching leaf for every sub-pattern. template [[nodiscard]] inline bool - reassociatableMatchHelper(const MatchContext &Ctx, - ArrayRef Leaves, SmallBitVector &Used, - PatternHd &HeadPattern, + reassociatableMatchHelper(const MatchContext &Ctx, ArrayRef 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 struct ReassociatableOpc_match { } template - [[nodiscard]] inline bool - reassociatableMatchHelper(const MatchContext &Ctx, - ArrayRef Leaves, - SmallBitVector &Used) { + [[nodiscard]] inline bool reassociatableMatchHelper(const MatchContext &Ctx, + ArrayRef Leaves, + SmallBitVector &Used) { return true; } }; From 15adee2d3c1fbe941c54ff770929394e7cae7d50 Mon Sep 17 00:00:00 2001 From: Artur Bermond Torres 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 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);