Add m_SelectCCLike matcher to match SELECT_CC or SELECT with SETCC #149646

houngkoungting · 2025-07-19T08:36:54Z

Fix #147282 and Follow-up to #148834
@RKSimon

llvmbot · 2025-07-19T08:37:26Z

@llvm/pr-subscribers-llvm-selectiondag

Author: 黃國庭 (houngkoungting)

Changes

Fix #147282 and Follow-up to #148834
@RKSimon

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

2 Files Affected:

(modified) llvm/include/llvm/CodeGen/SDPatternMatch.h (+4-4)
(modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+46-6)

diff --git a/llvm/include/llvm/CodeGen/SDPatternMatch.h b/llvm/include/llvm/CodeGen/SDPatternMatch.h
index 2967532226197..aeafb4f6dbadf 100644
--- a/llvm/include/llvm/CodeGen/SDPatternMatch.h
+++ b/llvm/include/llvm/CodeGen/SDPatternMatch.h
@@ -93,7 +93,7 @@ struct Value_match {
 
   explicit Value_match(SDValue Match) : MatchVal(Match) {}
 
-  template <typename MatchContext> bool match(const MatchContext &, SDValue N) {
+  template <typename MatchContext> bool match(const MatchContext &, SDValue N) const {
     if (MatchVal)
       return MatchVal == N;
     return N.getNode();
@@ -130,7 +130,7 @@ struct DeferredValue_match {
 
   explicit DeferredValue_match(SDValue &Match) : MatchVal(Match) {}
 
-  template <typename MatchContext> bool match(const MatchContext &, SDValue N) {
+  template <typename MatchContext> bool match(const MatchContext &, SDValue N) const {
     return N == MatchVal;
   }
 };
@@ -196,7 +196,7 @@ struct Value_bind {
 
   explicit Value_bind(SDValue &N) : BindVal(N) {}
 
-  template <typename MatchContext> bool match(const MatchContext &, SDValue N) {
+  template <typename MatchContext> bool match(const MatchContext &, SDValue N) const {
     BindVal = N;
     return true;
   }
@@ -1203,7 +1203,7 @@ struct CondCode_match {
 
   explicit CondCode_match(ISD::CondCode *CC) : BindCC(CC) {}
 
-  template <typename MatchContext> bool match(const MatchContext &, SDValue N) {
+  template <typename MatchContext> bool match(const MatchContext &, SDValue N) const {
     if (auto *CC = dyn_cast<CondCodeSDNode>(N.getNode())) {
       if (CCToMatch && *CCToMatch != CC->get())
         return false;
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index fed5e7238433e..5acc3a23a28b5 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -265,6 +265,47 @@ namespace {
           MaximumLegalStoreInBits = VT.getSizeInBits().getKnownMinValue();
     }
 
+   template <typename LTy, typename RTy, typename TTy, typename FTy,
+             typename CCTy>
+   struct SelectCC_match {
+     LTy L;
+     RTy R;
+     TTy T;
+     FTy F;
+     CCTy CC;
+ 
+     SelectCC_match(LTy L, RTy R, TTy T, FTy F, CCTy CC)
+         : L(std::move(L)), R(std::move(R)), T(std::move(T)), F(std::move(F)),
+           CC(std::move(CC)) {}
+
+     template <typename MatchContext>
+     bool match(const MatchContext &Ctx, SDValue V) const {
+       return V.getOpcode() == ISD::SELECT_CC && L.match(Ctx, V.getOperand(0)) &&
+              R.match(Ctx, V.getOperand(1)) && T.match(Ctx, V.getOperand(2)) &&
+              F.match(Ctx, V.getOperand(3)) && CC.match(Ctx, V.getOperand(4));
+     }
+   };
+
+   template <typename LTy, typename RTy, typename TTy, typename FTy,
+             typename CCTy>
+   inline auto m_SelectCC(LTy &&L, RTy &&R, TTy &&T, FTy &&F, CCTy &&CC) {
+     return SelectCC_match<std::decay_t<LTy>, std::decay_t<RTy>,
+                           std::decay_t<TTy>, std::decay_t<FTy>,
+                           std::decay_t<CCTy>>(
+         std::forward<LTy>(L), std::forward<RTy>(R), std::forward<TTy>(T),
+         std::forward<FTy>(F), std::forward<CCTy>(CC));
+   }
+
+   template <typename LTy, typename RTy, typename TTy, typename FTy,
+             typename CCTy>
+   inline auto m_SelectCCLike(LTy &&L, RTy &&R, TTy &&T, FTy &&F, CCTy &&CC) {
+     return SDPatternMatch::m_AnyOf(
+         SDPatternMatch::m_Select(SDPatternMatch::m_SetCC(L, R, CC), T, F),
+         m_SelectCC(std::forward<LTy>(L), std::forward<RTy>(R),
+                    std::forward<TTy>(T), std::forward<FTy>(F),
+                    std::forward<CCTy>(CC)));
+   }
+
     void ConsiderForPruning(SDNode *N) {
       // Mark this for potential pruning.
       PruningList.insert(N);
@@ -4099,12 +4140,11 @@ SDValue DAGCombiner::visitSUB(SDNode *N) {
   // (sub x, ([v]select (uge x, y), y, 0)) -> (umin x, (sub x, y))
   if (N1.hasOneUse() && hasUMin(VT)) {
     SDValue Y;
-    if (sd_match(N1, m_Select(m_SetCC(m_Specific(N0), m_Value(Y),
-                                      m_SpecificCondCode(ISD::SETULT)),
-                              m_Zero(), m_Deferred(Y))) ||
-        sd_match(N1, m_Select(m_SetCC(m_Specific(N0), m_Value(Y),
-                                      m_SpecificCondCode(ISD::SETUGE)),
-                              m_Deferred(Y), m_Zero())) ||
+    if (sd_match(N1, m_SelectCCLike(m_Specific(N0), m_Value(Y), m_Zero(),
+                                    m_Deferred(Y),
+                                    m_SpecificCondCode(ISD::SETULT))) ||
+        sd_match(N1, m_SelectCCLike(m_Specific(N0), m_Value(Y), m_Deferred(Y),
+                                    m_Zero(), m_SpecificCondCode(ISD::SETUGE)))||
         sd_match(N1, m_VSelect(m_SetCC(m_Specific(N0), m_Value(Y),
                                        m_SpecificCondCode(ISD::SETULT)),
                                m_Zero(), m_Deferred(Y))) ||

github-actions · 2025-07-19T08:40:24Z

✅ With the latest revision this PR passed the C/C++ code formatter.

RKSimon

Please add unit test coverage in SelectionDAGPatternMatchTest.cpp

RKSimon

Something seems to have gone wrong in the patch creation - m_SelectCCLike is missing and clang-format has gone amok

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

llvm/include/llvm/CodeGen/SDPatternMatch.h

houngkoungting · 2025-07-21T05:09:28Z

HI @RKSimon

Got it! Let me figure out the best way to fix this — appreciate the detailed comments! 👍

houngkoungting · 2025-07-22T05:21:31Z

HI @RKSimon , I’ve applied clang-format as suggested and adjusted the function signature to the inline style:

"template bool match(MatchContext &Ctx, SDValue V) {"

However, the pre-commit formatting check still seems to fail at this exact location.

RKSimon · 2025-07-22T07:22:13Z

llvm/include/llvm/CodeGen/SDPatternMatch.h

Why did you have to use this instead of just m_Node(ISD::SELECT_CC,L, R, T, F, CC)?

RKSimon · 2025-07-22T07:23:00Z

llvm/include/llvm/CodeGen/SDPatternMatch.h

Why the namespace?

houngkoungting · 2025-07-22T12:49:15Z

HI @RKSimon , Thank you for pointing that out! I've removed the unnecessary SDPatternMatch:: namespace and replaced the custom matcher with m_Node(...) . Please help take another look when you have time.

RKSimon · 2025-07-22T13:08:30Z

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

probably best not to repeat LHS/RHS like this in the test?

RKSimon

@mshockwave any thoughts?

RKSimon · 2025-07-22T13:43:22Z

llvm/include/llvm/CodeGen/SDPatternMatch.h

why do you have to use forwarding?

HI @RKSimon , I have removed the unnecessary std::forward calls as suggested and updated it.

RKSimon

A few minors but looks almost ready to go

RKSimon · 2025-07-24T10:38:06Z

llvm/include/llvm/CodeGen/SDPatternMatch.h

do we need rvalue && here?

@RKSimon You're right — thanks for pointing that out! Just give me a few minutes to update it .

RKSimon · 2025-07-24T10:38:11Z

llvm/include/llvm/CodeGen/SDPatternMatch.h

do we need rvalue && here?

RKSimon · 2025-07-24T10:38:30Z

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

Please can you add m_SelectCC unit test coverage

RKSimon · 2025-07-25T13:55:34Z

llvm/include/llvm/CodeGen/SDPatternMatch.h

 }

+template <typename LTy, typename RTy, typename TTy, typename FTy, typename CCTy>
+inline auto m_SelectCC(LTy L, RTy R, TTy T, FTy F, CCTy CC) {


still need to by ref m_SelectCC(LTy &L, RTy &R, TTy &T, FTy &F, CCTy &CC) (just not rvalue)

RKSimon · 2025-07-25T13:55:57Z

llvm/include/llvm/CodeGen/SDPatternMatch.h

+}
+
+template <typename LTy, typename RTy, typename TTy, typename FTy, typename CCTy>
+inline auto m_SelectCCLike(LTy L, RTy R, TTy T, FTy F, CCTy CC) {


m_SelectCCLike(LTy &L, RTy &R, TTy &T, FTy &F, CCTy &CC)

RKSimon · 2025-07-29T10:15:32Z

@houngkoungting almost there - please can you fix the references and I can get this committed for you.

houngkoungting · 2025-07-30T01:45:05Z

HI @RKSimon , No problem! Give me some time — I’ll fix the references very soon.

houngkoungting · 2025-07-30T11:57:18Z

HI @RKSimon ,I’ve already fixed it — does this look okay?

I’ve noticed that most matcher utilities in LLVM typically use const & for their parameters. This allows them to accept rvalues directly, which makes the code cleaner and easier to use.

I’m a bit unclear on why it has to be restricted to lvalues only — would you mind explaining?(I'm not very familiar with DAG.) thanks

RKSimon · 2025-07-30T16:15:15Z

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Can you remove matchUMinSubSelect and revert this back to using m_Select/m_VSelect? I'm hoping that once #150019 lands m_SelectCCLike can support m_VSelect as well.

RKSimon · 2025-07-30T16:57:55Z

@mshockwave any comments?

mshockwave · 2025-07-30T18:09:54Z

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

I don't think you need to create a context here. There are variants of sd_match that do not require context or explicit SelectionDAG: https://llvm.org/doxygen/namespacellvm_1_1SDPatternMatch.html#a22e69d98d1e6e8f2308ede1c5809d0ac

mshockwave · 2025-07-30T18:10:37Z

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

ditto unnecessary context

mshockwave · 2025-07-30T18:10:58Z

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

please use sd_match like other tests in this file.

mshockwave · 2025-07-30T18:11:15Z

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp

ditto sd_match

mshockwave · 2025-07-31T17:11:40Z

Please avoid force push unless necessary: https://llvm.org/docs/GitHub.html#rebasing-pull-requests-and-force-pushes

mshockwave

LGTM.
@RKSimon do you have any other comments?

Add m_SelectCCLike matcher to match SELECT_CC or SELECT with SETCC

432880f

llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Jul 19, 2025

Add m_SelectCCLike matcher to match SELECT_CC or SELECT with SETCC-1

060eba0

RKSimon requested review from RKSimon and mshockwave July 20, 2025 11:14

RKSimon reviewed Jul 20, 2025

View reviewed changes

RKSimon requested changes Jul 20, 2025

View reviewed changes

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Outdated Show resolved Hide resolved

llvm/include/llvm/CodeGen/SDPatternMatch.h Outdated Show resolved Hide resolved

houngkoungting added 8 commits July 22, 2025 11:07

[DAGCombiner] Add SelectCCLike matcher and unit test

72ad2f7

[DAGCombiner] Add SelectCCLike matcher and unit test-1

f703bf7

[DAGCombiner] Add SelectCCLike matcher and unit test-2

42279f8

[DAGCombiner] Add SelectCCLike matcher and unit test-3

2e22456

[DAGCombiner] Add SelectCCLike matcher and unit test-3

ebab64b

[DAGCombiner] Add SelectCCLike matcher and unit test-4

6436237

[DAGCombiner] Add SelectCCLike matcher and unit test-5

065aa5a

[DAGCombiner] Add SelectCCLike matcher and unit test-6

eca282d

RKSimon reviewed Jul 22, 2025

View reviewed changes

llvm/unittests/CodeGen/SelectionDAGPatternMatchTest.cpp Outdated

Copy link

Collaborator

RKSimon Jul 22, 2025

Choose a reason for hiding this comment

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

probably best not to repeat LHS/RHS like this in the test?

RKSimon reviewed Jul 22, 2025

View reviewed changes

RKSimon reviewed Jul 24, 2025

View reviewed changes

[DAGCombiner] Add SelectCCLike matcher and unit test-7

4e17482

RKSimon reviewed Jul 25, 2025

View reviewed changes

[DAG] Add m_SelectCCLike matcher for SELECT_CC and SELECT(SETCC(...))

d24f058

[DAG] Add m_SelectCCLike matcher for SELECT_CC and SELECT(SETCC(...))-1

82b9b2c

RKSimon reviewed Jul 30, 2025

View reviewed changes

mshockwave reviewed Jul 30, 2025

View reviewed changes

[DAG] Add m_SelectCCLike matcher for SELECT_CC and SELECT(SETCC(...))-2

4651216

mshockwave approved these changes Jul 31, 2025

View reviewed changes

RKSimon approved these changes Aug 1, 2025

View reviewed changes

Merge branch 'main' into main

06290be

RKSimon merged commit f04ea2e into llvm:main Aug 1, 2025
9 checks passed

Add m_SelectCCLike matcher to match SELECT_CC or SELECT with SETCC #149646

Add m_SelectCCLike matcher to match SELECT_CC or SELECT with SETCC #149646

Uh oh!

Conversation

houngkoungting commented Jul 19, 2025

Uh oh!

llvmbot commented Jul 19, 2025

Uh oh!

github-actions bot commented Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

houngkoungting commented Jul 21, 2025

Uh oh!

houngkoungting commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houngkoungting commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon commented Jul 29, 2025

Uh oh!

houngkoungting commented Jul 30, 2025

Uh oh!

houngkoungting commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon commented Jul 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshockwave commented Jul 31, 2025

Uh oh!

mshockwave left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

github-actions bot commented Jul 19, 2025 •

edited

Loading

houngkoungting commented Jul 30, 2025 •

edited

Loading