Skip to content

Conversation

@RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented May 2, 2025

No description provided.

@RKSimon RKSimon requested a review from mshockwave May 2, 2025 16:01
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label May 2, 2025
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-llvm-selectiondag

Author: Simon Pilgrim (RKSimon)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/138301.diff

3 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/SDPatternMatch.h (+5)
  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+3-3)
  • (modified) llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp (+5)
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<LHS, RHS, true> m_Xor(const LHS &L, const RHS &R) {
   return BinaryOpc_match<LHS, RHS, true>(ISD::XOR, L, R);
 }
 
+template <typename LHS, typename RHS>
+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 <typename LHS, typename RHS>
 inline BinaryOpc_match<LHS, RHS, true> m_SMin(const LHS &L, const RHS &R) {
   return BinaryOpc_match<LHS, RHS, true>(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())));

@RKSimon RKSimon marked this pull request as draft May 2, 2025 17:38
@RKSimon RKSimon marked this pull request as ready for review May 4, 2025 10:55
Copy link
Member

@mshockwave mshockwave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@RKSimon RKSimon merged commit bde39d7 into llvm:main May 6, 2025
11 checks passed
@RKSimon RKSimon deleted the dag-match-bitlogic branch May 6, 2025 11:50
@llvm-ci
Copy link
Collaborator

llvm-ci commented May 6, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/19542

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'Clang-Unit :: ./AllClangUnitTests/6/48' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/unittests/./AllClangUnitTests-Clang-Unit-65783-6-48.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=48 GTEST_SHARD_INDEX=6 /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/unittests/./AllClangUnitTests
--

Script:
--
/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/unittests/./AllClangUnitTests --gtest_filter=TimeProfilerTest.ConstantEvaluationCxx20
--
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/unittests/Support/TimeProfilerTest.cpp:240: Failure
Expected equality of these values:
  R"(
Frontend (test.cc)
| ParseDeclarationOrFunctionDefinition (test.cc:2:1)
| ParseDeclarationOrFunctionDefinition (test.cc:6:1)
| | ParseFunctionDefinition (slow_func)
| | | EvaluateAsRValue (<test.cc:8:21>)
| | | EvaluateForOverflow (<test.cc:8:21, col:25>)
| | | EvaluateForOverflow (<test.cc:8:30, col:32>)
| | | EvaluateAsRValue (<test.cc:9:14>)
| | | EvaluateForOverflow (<test.cc:9:9, col:14>)
| | | isPotentialConstantExpr (slow_namespace::slow_func)
| | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>)
| | | | EvaluateAsRValue (<test.cc:8:21, col:25>)
| | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>)
| | | | EvaluateAsRValue (<test.cc:8:21, col:25>)
| ParseDeclarationOrFunctionDefinition (test.cc:16:1)
| | ParseFunctionDefinition (slow_test)
| | | EvaluateAsInitializer (slow_value)
| | | EvaluateAsConstantExpr (<test.cc:17:33, col:59>)
| | | EvaluateAsConstantExpr (<test.cc:18:11, col:37>)
| ParseDeclarationOrFunctionDefinition (test.cc:22:1)
| | EvaluateAsConstantExpr (<test.cc:23:31, col:57>)
| | EvaluateAsRValue (<test.cc:22:14, line:23:58>)
| ParseDeclarationOrFunctionDefinition (test.cc:25:1)
| | EvaluateAsInitializer (slow_init_list)
| PerformPendingInstantiations
)"
    Which is: "\nFrontend (test.cc)\n| ParseDeclarationOrFunctionDefinition (test.cc:2:1)\n| ParseDeclarationOrFunctionDefinition (test.cc:6:1)\n| | ParseFunctionDefinition (slow_func)\n| | | EvaluateAsRValue (<test.cc:8:21>)\n| | | EvaluateForOverflow (<test.cc:8:21, col:25>)\n| | | EvaluateForOverflow (<test.cc:8:30, col:32>)\n| | | EvaluateAsRValue (<test.cc:9:14>)\n| | | EvaluateForOverflow (<test.cc:9:9, col:14>)\n| | | isPotentialConstantExpr (slow_namespace::slow_func)\n| | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>)\n| | | | EvaluateAsRValue (<test.cc:8:21, col:25>)\n| | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>)\n| | | | EvaluateAsRValue (<test.cc:8:21, col:25>)\n| ParseDeclarationOrFunctionDefinition (test.cc:16:1)\n| | ParseFunctionDefinition (slow_test)\n| | | EvaluateAsInitializer (slow_value)\n| | | EvaluateAsConstantExpr (<test.cc:17:33, col:59>)\n| | | EvaluateAsConstantExpr (<test.cc:18:11, col:37>)\n| ParseDeclarationOrFunctionDefinition (test.cc:22:1)\n| | EvaluateAsConstantExpr (<test.cc:23:31, col:57>)\n| | EvaluateAsRValue (<test.cc:22:14, line:23:58>)\n| ParseDeclarationOrFunctionDefinition (test.cc:25:1)\n| | EvaluateAsInitializer (slow_init_list)\n| PerformPendingInstantiations\n"
  buildTraceGraph(Json)
    Which is: "\nFrontend (test.cc)\n| ParseDeclarationOrFunctionDefinition (test.cc:2:1)\n| ParseDeclarationOrFunctionDefinition (test.cc:6:1)\n| | ParseFunctionDefinition (slow_func)\n| | | EvaluateAsRValue (<test.cc:8:21>)\n| | | EvaluateForOverflow (<test.cc:8:21, col:25>)\n| | | EvaluateForOverflow (<test.cc:8:30, col:32>)\n| | | EvaluateAsRValue (<test.cc:9:14>)\n| | | EvaluateForOverflow (<test.cc:9:9, col:14>)\n| | | isPotentialConstantExpr (slow_namespace::slow_func)\n| | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>)\n| | | | EvaluateAsRValue (<test.cc:8:21, col:25>)\n| | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>)\n| | | | EvaluateAsRValue (<test.cc:8:21, col:25>)\n| ParseDeclarationOrFunctionDefinition (test.cc:16:1)\n| | ParseFunctionDefinition (slow_test)\n| | | EvaluateAsInitializer (slow_value)\n| | | EvaluateAsConstantExpr (<test.cc:17:33, col:59>)\n| | | EvaluateAsConstantExpr (<test.cc:18:11, col:37>)\n| ParseDeclarationOrFunctionDefinition (test.cc:22:1)\n| | EvaluateAsConstantExpr (<test.cc:23:31, col:57>)\n| | EvaluateAsRValue (<test.cc:22:14, line:23:58>)\n| ParseDeclarationOrFunctionDefinition (test.cc:25:1)\n| | EvaluateAsInitializer (slow_init_list)\n| | PerformPendingInstantiations\n"
With diff:
@@ -24,3 +24,3 @@
 | ParseDeclarationOrFunctionDefinition (test.cc:25:1)
 | | EvaluateAsInitializer (slow_init_list)
-| PerformPendingInstantiations\n
+| | PerformPendingInstantiations\n


...

GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:ARM llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants