From a24b0de03e3d9a6972ea674637623188b0f8d357 Mon Sep 17 00:00:00 2001 From: Simon Pilgrim Date: Fri, 2 May 2025 16:59:26 +0100 Subject: [PATCH 1/3] [DAG] Add SDPatternMatch::m_BitwiseLogic common matcher for AND/OR/XOR nodes --- llvm/include/llvm/CodeGen/SDPatternMatch.h | 5 +++++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 6 +++--- llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp | 5 +++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/CodeGen/SDPatternMatch.h b/llvm/include/llvm/CodeGen/SDPatternMatch.h index 9532be67c92e1..23dc7d9b13ee1 100644 --- a/llvm/include/llvm/CodeGen/SDPatternMatch.h +++ b/llvm/include/llvm/CodeGen/SDPatternMatch.h @@ -729,6 +729,11 @@ inline BinaryOpc_match m_Xor(const LHS &L, const RHS &R) { return BinaryOpc_match(ISD::XOR, L, R); } +template +inline auto m_BitwiseLogic(const LHS &L, const RHS &R) { + return m_AnyOf(m_And(L, R), m_Or(L, R), m_Xor(L, R)); +} + template inline BinaryOpc_match m_SMin(const LHS &L, const RHS &R) { return BinaryOpc_match(ISD::SMIN, L, R); diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index ea1435c3934be..262fe052cedd2 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -10553,10 +10553,10 @@ static SDValue foldBitOrderCrossLogicOp(SDNode *N, SelectionDAG &DAG) { SDValue N0 = N->getOperand(0); EVT VT = N->getValueType(0); SDLoc DL(N); - if (ISD::isBitwiseLogicOp(N0.getOpcode()) && N0.hasOneUse()) { - SDValue OldLHS = N0.getOperand(0); - SDValue OldRHS = N0.getOperand(1); + SDValue OldLHS, OldRHS; + if (sd_match(N0, + m_OneUse(m_BitwiseLogic(m_Value(OldLHS), m_Value(OldRHS))))) { // If both operands are bswap/bitreverse, ignore the multiuse // Otherwise need to ensure logic_op and bswap/bitreverse(x) have one use. if (OldLHS.getOpcode() == Opcode && OldRHS.getOpcode() == Opcode) { diff --git a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp index df35a8678e0a4..67a0d948dcace 100644 --- a/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp +++ b/llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp @@ -302,13 +302,18 @@ TEST_F(SelectionDAGPatternMatchTest, matchBinaryOp) { EXPECT_TRUE( sd_match(SFAdd, m_ChainedBinOp(ISD::STRICT_FADD, m_SpecificVT(Float32VT), m_SpecificVT(Float32VT)))); + EXPECT_FALSE(sd_match(Add, m_BitwiseLogic(m_Value(), m_Value()))); + EXPECT_FALSE(sd_match(Sub, m_BitwiseLogic(m_Value(), m_Value()))); EXPECT_TRUE(sd_match(And, m_c_BinOp(ISD::AND, m_Value(), m_Value()))); EXPECT_TRUE(sd_match(And, m_And(m_Value(), m_Value()))); + EXPECT_TRUE(sd_match(And, m_BitwiseLogic(m_Value(), m_Value()))); EXPECT_TRUE(sd_match(Xor, m_c_BinOp(ISD::XOR, m_Value(), m_Value()))); EXPECT_TRUE(sd_match(Xor, m_Xor(m_Value(), m_Value()))); + EXPECT_TRUE(sd_match(Xor, m_BitwiseLogic(m_Value(), m_Value()))); EXPECT_TRUE(sd_match(Or, m_c_BinOp(ISD::OR, m_Value(), m_Value()))); EXPECT_TRUE(sd_match(Or, m_Or(m_Value(), m_Value()))); + EXPECT_TRUE(sd_match(Or, m_BitwiseLogic(m_Value(), m_Value()))); EXPECT_FALSE(sd_match(Or, m_DisjointOr(m_Value(), m_Value()))); EXPECT_TRUE(sd_match(DisOr, m_Or(m_Value(), m_Value()))); From b802a454eb5c2901edda7d45c9c1ff4ed49bc26e Mon Sep 17 00:00:00 2001 From: Simon Pilgrim Date: Fri, 2 May 2025 17:05:41 +0100 Subject: [PATCH 2/3] Make use of commutable matching in foldBitOrderCrossLogicOp --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 31 +++++++------------ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 262fe052cedd2..f51a85977c1f6 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -10553,29 +10553,20 @@ static SDValue foldBitOrderCrossLogicOp(SDNode *N, SelectionDAG &DAG) { SDValue N0 = N->getOperand(0); EVT VT = N->getValueType(0); SDLoc DL(N); + SDValue X, Y; - SDValue OldLHS, OldRHS; - if (sd_match(N0, - m_OneUse(m_BitwiseLogic(m_Value(OldLHS), m_Value(OldRHS))))) { - // If both operands are bswap/bitreverse, ignore the multiuse - // Otherwise need to ensure logic_op and bswap/bitreverse(x) have one use. - if (OldLHS.getOpcode() == Opcode && OldRHS.getOpcode() == Opcode) { - return DAG.getNode(N0.getOpcode(), DL, VT, OldLHS.getOperand(0), - OldRHS.getOperand(0)); - } + // If both operands are bswap/bitreverse, ignore the multiuse + if (sd_match(N0, m_OneUse(m_BitwiseLogic(m_UnaryOp(Opcode, m_Value(X)), + m_UnaryOp(Opcode, m_Value(Y)))))) + return DAG.getNode(N0.getOpcode(), DL, VT, X, Y); - if (OldLHS.getOpcode() == Opcode && OldLHS.hasOneUse()) { - SDValue NewBitReorder = DAG.getNode(Opcode, DL, VT, OldRHS); - return DAG.getNode(N0.getOpcode(), DL, VT, OldLHS.getOperand(0), - NewBitReorder); - } - - if (OldRHS.getOpcode() == Opcode && OldRHS.hasOneUse()) { - SDValue NewBitReorder = DAG.getNode(Opcode, DL, VT, OldLHS); - return DAG.getNode(N0.getOpcode(), DL, VT, NewBitReorder, - OldRHS.getOperand(0)); - } + // Otherwise need to ensure logic_op and bswap/bitreverse(x) have one use. + if (sd_match(N0, m_OneUse(m_BitwiseLogic( + m_OneUse(m_UnaryOp(Opcode, m_Value(X))), m_Value(Y))))) { + SDValue NewBitReorder = DAG.getNode(Opcode, DL, VT, Y); + return DAG.getNode(N0.getOpcode(), DL, VT, X, NewBitReorder); } + return SDValue(); } From 4e63cdf0c91c31218aae11ef7fc2e60dc8621c22 Mon Sep 17 00:00:00 2001 From: Simon Pilgrim Date: Sun, 4 May 2025 10:50:29 +0100 Subject: [PATCH 3/3] Regenerate test to account for commutation --- llvm/test/CodeGen/ARM/combine-bswap.ll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/ARM/combine-bswap.ll b/llvm/test/CodeGen/ARM/combine-bswap.ll index 16a1d79de28ad..e9436fa24db26 100644 --- a/llvm/test/CodeGen/ARM/combine-bswap.ll +++ b/llvm/test/CodeGen/ARM/combine-bswap.ll @@ -23,7 +23,7 @@ define i64 @bs_or_rhs_bs64(i64 %a, i64 %b) #0 { ; CHECK-NEXT: rev r1, r1 ; CHECK-NEXT: rev r0, r0 ; CHECK-NEXT: orrs r2, r1 -; CHECK-NEXT: orr.w r1, r0, r3 +; CHECK-NEXT: orr.w r1, r3, r0 ; CHECK-NEXT: mov r0, r2 ; CHECK-NEXT: bx lr %1 = tail call i64 @llvm.bswap.i64(i64 %b)