Skip to content

[GlobalISel] Add combine action for C++ combine rules #135941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 25, 2025

Conversation

Pierre-vh
Copy link
Contributor

Adds a combine action (DAG operator) which allows for easy definition of
combine rule that only match one or more instructions, and defer all remaining
match/apply logic to C++ code.

This avoids the need for split match/apply function in such cases. One function
can do the trick as long as it returns true if it changed any code.

This is implemented as syntactic sugar over match/apply. The combine rule is
just a match pattern BUT every C++ pattern inside is treated as an "apply" function.
This makes it fit seamlessly with the current backend.

Fixes #92410

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Pierre-vh Pierre-vh marked this pull request as ready for review April 16, 2025 09:10
@llvmbot
Copy link
Member

llvmbot commented Apr 16, 2025

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-tablegen

Author: Pierre van Houtryve (Pierre-vh)

Changes

Adds a combine action (DAG operator) which allows for easy definition of
combine rule that only match one or more instructions, and defer all remaining
match/apply logic to C++ code.

This avoids the need for split match/apply function in such cases. One function
can do the trick as long as it returns true if it changed any code.

This is implemented as syntactic sugar over match/apply. The combine rule is
just a match pattern BUT every C++ pattern inside is treated as an "apply" function.
This makes it fit seamlessly with the current backend.

Fixes #92410


Patch is 22.99 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/135941.diff

6 Files Affected:

  • (modified) llvm/docs/GlobalISel/MIRPatterns.rst (+37-1)
  • (modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+8-7)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-cxx.td (+78-55)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-errors.td (+33-1)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td (+34-1)
  • (modified) llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp (+40-9)
diff --git a/llvm/docs/GlobalISel/MIRPatterns.rst b/llvm/docs/GlobalISel/MIRPatterns.rst
index 42ceb1883f7de..3851698cc6686 100644
--- a/llvm/docs/GlobalISel/MIRPatterns.rst
+++ b/llvm/docs/GlobalISel/MIRPatterns.rst
@@ -74,7 +74,7 @@ Operands are ordered just like they would be in a MachineInstr: the defs (outs)
 come first, then the uses (ins).
 
 Patterns are generally grouped into another DAG datatype with a dummy operator
-such as ``match``, ``apply`` or ``pattern``.
+such as ``match``, ``apply``, ``combine`` or ``pattern``.
 
 Finally, any DAG datatype in TableGen can be named. This also holds for
 patterns. e.g. the following is valid: ``(G_FOO $root, (i32 0):$cst):$mypat``.
@@ -370,6 +370,42 @@ The following expansions are available for MIR patterns:
     (match (G_ZEXT $root, $src):$mi),
     (apply "foobar(${root}.getReg(), ${src}.getReg(), ${mi}->hasImplicitDef())")>;
 
+``combine`` Operator
+~~~~~~~~~~~~~~~~~~~~
+
+``GICombineRule`` also supports a single ``combine`` pattern, which is a shorter way to
+declare patterns that just match one or more instruction, then defer all remaining matching
+and rewriting logic to C++ code.
+
+.. code-block:: text
+  :caption: Example usage of the combine operator.
+
+  // match + apply
+  def FooLong : GICombineRule<
+    (defs root:$root),
+    (match (G_ZEXT $root, $src):$mi, "return matchFoo(${mi});"),
+    (apply "applyFoo(${mi});")>;
+
+  // combine
+  def FooShort : GICombineRule<
+    (defs root:$root),
+    (combine (G_ZEXT $root, $src):$mi, "return combineFoo(${mi});")>;
+
+This has a couple of advantages:
+
+* We only need one C++ function, not two.
+* We no longer need to use ``GIDefMatchData`` to pass information between the match/apply functions.
+
+As described above, this is syntactic sugar for the match+apply form. In a ``combine`` pattern:
+
+* Everything except C++ code is considered the ``match`` part.
+* The C++ code is the ``apply`` part. C++ code is emitted in order of appearance.
+
+.. note::
+
+  The C++ code **must** return true if it changed any instruction. Returning false when changing
+  instructions is undefined behavior.
+
 Common Pattern #1: Replace a Register with Another
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index e5e19a1d93486..efd88524a159e 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -31,6 +31,8 @@ def defs;
 def pattern;
 def match;
 def apply;
+def combine;
+def empty_action;
 
 def wip_match_opcode;
 
@@ -67,18 +69,17 @@ class GIDefMatchData<string type>  {
   string Type = type;
 }
 
-class GICombineRule<dag defs, dag match, dag apply> : GICombine {
+class GICombineRule<dag defs, dag a0, dag a1 = (empty_action)> : GICombine {
   /// Defines the external interface of the match rule. This includes:
   /// * The names of the root nodes (requires at least one)
   /// See GIDefKind for details.
   dag Defs = defs;
 
-  /// Defines the things which must be true for the pattern to match
-  dag Match = match;
-
-  /// Defines the things which happen after the decision is made to apply a
-  /// combine rule.
-  dag Apply = apply;
+  // The patterns that will be used. Two types of list can exist:
+  //  match (Action0) + apply (Action1).
+  //  combine (Action0) + empty_action (Action1).
+  dag Action0 = a0;
+  dag Action1 = a1;
 
   /// Defines the predicates that are checked before the match function
   /// is called. Targets can use this to, for instance, check Subtarget
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-cxx.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-cxx.td
index 9a1a15c8d30df..57f2b2ca53276 100644
--- a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-cxx.td
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-cxx.td
@@ -28,11 +28,16 @@ def NoMatchTwoApply : GICombineRule<
   (match (G_SEXT $a, $y)),
   (apply "APPLY0", "APPLY1")>;
 
+def CopmbineCXXOrder : GICombineRule<
+  (defs root:$a),
+  (combine (G_ZEXT $a, $y), "A0", "return A1")>;
+
 def MyCombiner: GICombiner<"GenMyCombiner", [
   OneMatchOneApply,
   TwoMatchTwoApply,
   TwoMatchNoApply,
-  NoMatchTwoApply
+  NoMatchTwoApply,
+  CopmbineCXXOrder
 ]>;
 
 // CHECK:      bool GenMyCombiner::testMIPredicate_MI(unsigned PredicateID, const MachineInstr & MI, const MatcherState &State) const {
@@ -79,65 +84,83 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
 // CHECK-NEXT:     APPLY1
 // CHECK-NEXT:     return true;
 // CHECK-NEXT:   }
+// CHECK-NEXT:   case GICXXCustomAction_GICombiner3:{
+// CHECK-NEXT:     // Apply Patterns
+// CHECK-NEXT:     A0
+// CHECK-NEXT:     return A1
+// CHECK-NEXT:     return true;
+// CHECK-NEXT:   }
 // CHECK-NEXT:   }
 // CHECK-NEXT:   llvm_unreachable("Unknown Apply Action");
 // CHECK-NEXT: }
 
 // CHECK:      const uint8_t *GenMyCombiner::getMatchTable() const {
 // CHECK-NEXT:   constexpr static uint8_t MatchTable0[] = {
-// CHECK-NEXT:     GIM_SwitchOpcode, /*MI*/0, /*[*/GIMT_Encode2([[#LOWER:]]), GIMT_Encode2([[#UPPER:]]), /*)*//*default:*//*Label 4*/ GIMT_Encode4([[#DEFAULT:]]),
-// CHECK-NEXT:     /*TargetOpcode::G_STORE*//*Label 0*/ GIMT_Encode4([[L418:[0-9]+]]), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0),
-// CHECK-NEXT:     /*TargetOpcode::G_SEXT*//*Label 1*/ GIMT_Encode4([[L436:[0-9]+]]), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0),
-// CHECK-NEXT:     /*TargetOpcode::G_FNEG*//*Label 2*/ GIMT_Encode4([[L448:[0-9]+]]), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0),
-// CHECK-NEXT:     /*TargetOpcode::G_FABS*//*Label 3*/ GIMT_Encode4([[L460:[0-9]+]]),
-// CHECK-NEXT:     // Label 0: @[[#%u, mul(UPPER-LOWER, 4) + 10]]
-// CHECK-NEXT:     GIM_Try, /*On fail goto*//*Label 5*/ GIMT_Encode4([[L435:[0-9]+]]), // Rule ID 2 //
-// CHECK-NEXT:       GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule2Enabled),
-// CHECK-NEXT:       // MIs[0] x
-// CHECK-NEXT:       // No operand predicates
-// CHECK-NEXT:       // MIs[0] y
-// CHECK-NEXT:       // No operand predicates
-// CHECK-NEXT:       GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner0),
-// CHECK-NEXT:       GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner1),
-// CHECK-NEXT:       // Combiner Rule #2: TwoMatchNoApply
-// CHECK-NEXT:       GIR_EraseRootFromParent_Done,
-// CHECK-NEXT:     // Label 5: @[[L435]]
-// CHECK-NEXT:     GIM_Reject,
-// CHECK-NEXT:     // Label 1: @[[L436]]
-// CHECK-NEXT:     GIM_Try, /*On fail goto*//*Label 6*/ GIMT_Encode4([[L447:[0-9]+]]), // Rule ID 3 //
-// CHECK-NEXT:       GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule3Enabled),
-// CHECK-NEXT:       // MIs[0] a
-// CHECK-NEXT:       // No operand predicates
-// CHECK-NEXT:       // MIs[0] y
-// CHECK-NEXT:       // No operand predicates
-// CHECK-NEXT:       // Combiner Rule #3: NoMatchTwoApply
-// CHECK-NEXT:       GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner2),
-// CHECK-NEXT:     // Label 6: @[[L447]]
-// CHECK-NEXT:     GIM_Reject,
-// CHECK-NEXT:     // Label 2: @[[L448]]
-// CHECK-NEXT:     GIM_Try, /*On fail goto*//*Label 7*/ GIMT_Encode4([[L459:[0-9]+]]), // Rule ID 1 //
-// CHECK-NEXT:       GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule1Enabled),
-// CHECK-NEXT:       // MIs[0] a
-// CHECK-NEXT:       // No operand predicates
-// CHECK-NEXT:       // MIs[0] b
-// CHECK-NEXT:       // No operand predicates
-// CHECK-NEXT:       // Combiner Rule #1: TwoMatchTwoApply
-// CHECK-NEXT:       GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner1),
-// CHECK-NEXT:     // Label 7: @[[L459]]
-// CHECK-NEXT:     GIM_Reject,
-// CHECK-NEXT:     // Label 3: @[[L460]]
-// CHECK-NEXT:     GIM_Try, /*On fail goto*//*Label 8*/ GIMT_Encode4([[L471:[0-9]+]]), // Rule ID 0 //
-// CHECK-NEXT:       GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
-// CHECK-NEXT:       // MIs[0] a
-// CHECK-NEXT:       // No operand predicates
-// CHECK-NEXT:       // MIs[0] b
-// CHECK-NEXT:       // No operand predicates
-// CHECK-NEXT:       // Combiner Rule #0: OneMatchOneApply
-// CHECK-NEXT:       GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner0),
-// CHECK-NEXT:     // Label 8: @[[L471]]
-// CHECK-NEXT:     GIM_Reject,
-// CHECK-NEXT:     // Label 4: @[[#%u, DEFAULT]]
-// CHECK-NEXT:     GIM_Reject,
-// CHECK-NEXT:     }; // Size: [[#%u, DEFAULT + 1]] bytes
+// CHECK-NEXT:      /*   0 */ GIM_SwitchOpcode, /*MI*/0, /*[*/GIMT_Encode2(99), GIMT_Encode2(205), /*)*//*default:*//*Label 5*/ GIMT_Encode4(500),
+// CHECK-NEXT:      /*  10 */ /*TargetOpcode::G_STORE*//*Label 0*/ GIMT_Encode4(434), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0),
+// CHECK-NEXT:      /* 162 */ /*TargetOpcode::G_SEXT*//*Label 1*/ GIMT_Encode4(452), GIMT_Encode4(0),
+// CHECK-NEXT:      /* 170 */ /*TargetOpcode::G_ZEXT*//*Label 2*/ GIMT_Encode4(464), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0),
+// CHECK-NEXT:      /* 394 */ /*TargetOpcode::G_FNEG*//*Label 3*/ GIMT_Encode4(476), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0),
+// CHECK-NEXT:      /* 430 */ /*TargetOpcode::G_FABS*//*Label 4*/ GIMT_Encode4(488),
+// CHECK-NEXT:      /* 434 */ // Label 0: @434
+// CHECK-NEXT:      /* 434 */ GIM_Try, /*On fail goto*//*Label 6*/ GIMT_Encode4(451), // Rule ID 2 //
+// CHECK-NEXT:      /* 439 */   GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule2Enabled),
+// CHECK-NEXT:      /* 442 */   // MIs[0] x
+// CHECK-NEXT:      /* 442 */   // No operand predicates
+// CHECK-NEXT:      /* 442 */   // MIs[0] y
+// CHECK-NEXT:      /* 442 */   // No operand predicates
+// CHECK-NEXT:      /* 442 */   GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner0),
+// CHECK-NEXT:      /* 446 */   GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner1),
+// CHECK-NEXT:      /* 450 */   // Combiner Rule #2: TwoMatchNoApply
+// CHECK-NEXT:      /* 450 */   GIR_EraseRootFromParent_Done,
+// CHECK-NEXT:      /* 451 */ // Label 6: @451
+// CHECK-NEXT:      /* 451 */ GIM_Reject,
+// CHECK-NEXT:      /* 452 */ // Label 1: @452
+// CHECK-NEXT:      /* 452 */ GIM_Try, /*On fail goto*//*Label 7*/ GIMT_Encode4(463), // Rule ID 3 //
+// CHECK-NEXT:      /* 457 */   GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule3Enabled),
+// CHECK-NEXT:      /* 460 */   // MIs[0] a
+// CHECK-NEXT:      /* 460 */   // No operand predicates
+// CHECK-NEXT:      /* 460 */   // MIs[0] y
+// CHECK-NEXT:      /* 460 */   // No operand predicates
+// CHECK-NEXT:      /* 460 */   // Combiner Rule #3: NoMatchTwoApply
+// CHECK-NEXT:      /* 460 */   GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner2),
+// CHECK-NEXT:      /* 463 */ // Label 7: @463
+// CHECK-NEXT:      /* 463 */ GIM_Reject,
+// CHECK-NEXT:      /* 464 */ // Label 2: @464
+// CHECK-NEXT:      /* 464 */ GIM_Try, /*On fail goto*//*Label 8*/ GIMT_Encode4(475), // Rule ID 4 //
+// CHECK-NEXT:      /* 469 */   GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule4Enabled),
+// CHECK-NEXT:      /* 472 */   // MIs[0] a
+// CHECK-NEXT:      /* 472 */   // No operand predicates
+// CHECK-NEXT:      /* 472 */   // MIs[0] y
+// CHECK-NEXT:      /* 472 */   // No operand predicates
+// CHECK-NEXT:      /* 472 */   // Combiner Rule #4: CopmbineCXXOrder
+// CHECK-NEXT:      /* 472 */   GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner3),
+// CHECK-NEXT:      /* 475 */ // Label 8: @475
+// CHECK-NEXT:      /* 475 */ GIM_Reject,
+// CHECK-NEXT:      /* 476 */ // Label 3: @476
+// CHECK-NEXT:      /* 476 */ GIM_Try, /*On fail goto*//*Label 9*/ GIMT_Encode4(487), // Rule ID 1 //
+// CHECK-NEXT:      /* 481 */   GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule1Enabled),
+// CHECK-NEXT:      /* 484 */   // MIs[0] a
+// CHECK-NEXT:      /* 484 */   // No operand predicates
+// CHECK-NEXT:      /* 484 */   // MIs[0] b
+// CHECK-NEXT:      /* 484 */   // No operand predicates
+// CHECK-NEXT:      /* 484 */   // Combiner Rule #1: TwoMatchTwoApply
+// CHECK-NEXT:      /* 484 */   GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner1),
+// CHECK-NEXT:      /* 487 */ // Label 9: @487
+// CHECK-NEXT:      /* 487 */ GIM_Reject,
+// CHECK-NEXT:      /* 488 */ // Label 4: @488
+// CHECK-NEXT:      /* 488 */ GIM_Try, /*On fail goto*//*Label 10*/ GIMT_Encode4(499), // Rule ID 0 //
+// CHECK-NEXT:      /* 493 */   GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
+// CHECK-NEXT:      /* 496 */   // MIs[0] a
+// CHECK-NEXT:      /* 496 */   // No operand predicates
+// CHECK-NEXT:      /* 496 */   // MIs[0] b
+// CHECK-NEXT:      /* 496 */   // No operand predicates
+// CHECK-NEXT:      /* 496 */   // Combiner Rule #0: OneMatchOneApply
+// CHECK-NEXT:      /* 496 */   GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner0),
+// CHECK-NEXT:      /* 499 */ // Label 10: @499
+// CHECK-NEXT:      /* 499 */ GIM_Reject,
+// CHECK-NEXT:      /* 500 */ // Label 5: @500
+// CHECK-NEXT:      /* 500 */ GIM_Reject,
+// CHECK-NEXT:      /* 501 */ }; // Size: 501 bytes
 // CHECK-NEXT:   return MatchTable0;
 // CHECK-NEXT: }
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-errors.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-errors.td
index 5017672f5cbd7..1642cca3aced0 100644
--- a/llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-errors.td
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-errors.td
@@ -283,6 +283,33 @@ def matchdata_without_cxx_apply : GICombineRule<
   (match (G_ZEXT $dst, $src):$mi),
   (apply (G_MUL $dst, $src, $src))>;
 
+// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Expected both a 'match' and 'apply' action in combine rule, or a single 'combine' action
+def missing_apply : GICombineRule<
+  (defs root:$dst),
+  (match (G_ZEXT $dst, $src))>;
+
+// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: 'combine' action needs at least one pattern to match, and C++ code to apply
+def combineop_missing_cxx : GICombineRule<
+  (defs root:$d),
+  (combine (wip_match_opcode G_TRUNC):$d)>;
+
+// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: 'combine' action needs at least one pattern to match, and C++ code to apply
+def combineop_missing_mir : GICombineRule<
+  (defs root:$d),
+  (combine "return APPLY;")>;
+
+// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Expected both a 'match' and 'apply' action in combine rule, or a single 'combine' action
+def mixed_combine_match : GICombineRule<
+  (defs root:$d),
+  (combine (G_ZEXT $d, $y), "return APPLY;"),
+  (match (G_ZEXT $d, $y))>;
+
+// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Expected both a 'match' and 'apply' action in combine rule, or a single 'combine' action
+def mixed_combine_apply : GICombineRule<
+  (defs root:$d),
+  (combine "return APPLY;"),
+  (apply (G_ZEXT $d, $y))>;
+
 // CHECK: error: Failed to parse one or more rules
 
 def MyCombiner: GICombiner<"GenMyCombiner", [
@@ -326,5 +353,10 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
   using_flagref_in_match,
   badflagref_in_apply,
   mixed_cxx_apply,
-  matchdata_without_cxx_apply
+  matchdata_without_cxx_apply,
+  missing_apply,
+  combineop_missing_cxx,
+  combineop_missing_mir,
+  mixed_combine_match,
+  mixed_combine_apply
 ]>;
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td
index 85075359df737..7541639480fc5 100644
--- a/llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td
@@ -383,6 +383,37 @@ def IntrinTest1 : GICombineRule<
   (match (int_convergent_1in_1out $a, $b)),
   (apply (int_convergent_sideeffects_1in_1out $a, $b))>;
 
+// CHECK:      (CombineRule name:CombineOperator0 id:14 root:d
+// CHECK-NEXT:   (MatchPats
+// CHECK-NEXT:     <match_root>d:(AnyOpcodePattern [G_TRUNC])
+// CHECK-NEXT:   )
+// CHECK-NEXT:   (ApplyPats
+// CHECK-NEXT:     __CombineOperator0_combine_1:(CXXPattern apply code:"return APPLY;")
+// CHECK-NEXT:   )
+// CHECK-NEXT:   (OperandTable MatchPats <empty>)
+// CHECK-NEXT:   (OperandTable ApplyPats <empty>)
+// CHECK-NEXT: )
+def CombineOperator0 : GICombineRule<
+  (defs root:$d),
+  (combine (wip_match_opcode G_TRUNC):$d, "return APPLY;")>;
+
+// CHECK:      (CombineRule name:CombineOperator1 id:15 root:a
+// CHECK-NEXT:   (MatchPats
+// CHECK-NEXT:     <match_root>__CombineOperator1_combine_0:(CodeGenInstructionPattern G_TRUNC operands:[<def>$a, $b])
+// CHECK-NEXT:   )
+// CHECK-NEXT:   (ApplyPats
+// CHECK-NEXT:     __CombineOperator1_combine_1:(CXXPattern apply code:"return APPLY ${a} ${b};")
+// CHECK-NEXT:   )
+// CHECK-NEXT:   (OperandTable MatchPats
+// CHECK-NEXT:     a -> __CombineOperator1_combine_0
+// CHECK-NEXT:     b -> <live-in>
+// CHECK-NEXT:   )
+// CHECK-NEXT:   (OperandTable ApplyPats <empty>)
+// CHECK-NEXT: )
+def CombineOperator1 : GICombineRule<
+  (defs root:$a),
+  (combine (G_TRUNC $a, $b), "return APPLY ${a} ${b};")>;
+
 def MyCombiner: GICombiner<"GenMyCombiner", [
   WipOpcodeTest0,
   WipOpcodeTest1,
@@ -397,5 +428,7 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
   TypeOfTest,
 ...
[truncated]

Copy link

github-actions bot commented Apr 16, 2025

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

@aemerson
Copy link
Contributor

I like the idea overall. I'm just thinking of whether we should use a different name for this instead of overloading combine? I'm not strongly opposed to it though. @arsenm @jayfoad ?

Comment on lines +390 to +392
def FooShort : GICombineRule<
(defs root:$root),
(combine (G_ZEXT $root, $src):$mi, "return combineFoo(${mi});")>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, is it different from

def FooShort : GICombineRule<
  (defs root:$root),
  (match (G_ZEXT $root, $src):$mi),
  (apply "return combineFoo(${mi});")
>;

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the same thing yes. We have a convention that apply C++ code does not return anything/cannot fail. It's just a convention and not enforced anywhere AFAIK, so your example would work.
With (combine we allow the APPLY code to fail, so it can match/apply all in one.

Adds a `combine` action (DAG operator) which allows for easy definition of
combine rule that only match one or more instructions, and defer all remaining
match/apply logic to C++ code.

This avoids the need for split match/apply function in such cases. One function
can do the trick as long as it returns `true` if it changed any code.

This is implemented as syntactic sugar over match/apply. The combine rule is
just a match pattern BUT every C++ pattern inside is treated as an "apply" function.
This makes it fit seamlessly with the current backend.

Fixes #92410
@Pierre-vh Pierre-vh force-pushed the users/pierre-vh/gicomb-combine-operator branch from 8364585 to 2dbed9a Compare April 22, 2025 07:53
@Pierre-vh Pierre-vh requested a review from arsenm April 22, 2025 07:54
Copy link
Contributor Author

Pierre-vh commented Apr 25, 2025

Merge activity

  • Apr 25, 6:08 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 25, 6:10 AM EDT: @Pierre-vh merged this pull request with Graphite.

@Pierre-vh Pierre-vh merged commit c792b25 into main Apr 25, 2025
12 checks passed
@Pierre-vh Pierre-vh deleted the users/pierre-vh/gicomb-combine-operator branch April 25, 2025 10:10
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Adds a `combine` action (DAG operator) which allows for easy definition of
combine rule that only match one or more instructions, and defer all remaining
match/apply logic to C++ code.

This avoids the need for split match/apply function in such cases. One function
can do the trick as long as it returns `true` if it changed any code.

This is implemented as syntactic sugar over match/apply. The combine rule is
just a match pattern BUT every C++ pattern inside is treated as an "apply" function.
This makes it fit seamlessly with the current backend.

Fixes llvm#92410
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Adds a `combine` action (DAG operator) which allows for easy definition of
combine rule that only match one or more instructions, and defer all remaining
match/apply logic to C++ code.

This avoids the need for split match/apply function in such cases. One function
can do the trick as long as it returns `true` if it changed any code.

This is implemented as syntactic sugar over match/apply. The combine rule is
just a match pattern BUT every C++ pattern inside is treated as an "apply" function.
This makes it fit seamlessly with the current backend.

Fixes llvm#92410
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Adds a `combine` action (DAG operator) which allows for easy definition of
combine rule that only match one or more instructions, and defer all remaining
match/apply logic to C++ code.

This avoids the need for split match/apply function in such cases. One function
can do the trick as long as it returns `true` if it changed any code.

This is implemented as syntactic sugar over match/apply. The combine rule is
just a match pattern BUT every C++ pattern inside is treated as an "apply" function.
This makes it fit seamlessly with the current backend.

Fixes llvm#92410
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
Adds a `combine` action (DAG operator) which allows for easy definition of
combine rule that only match one or more instructions, and defer all remaining
match/apply logic to C++ code.

This avoids the need for split match/apply function in such cases. One function
can do the trick as long as it returns `true` if it changed any code.

This is implemented as syntactic sugar over match/apply. The combine rule is
just a match pattern BUT every C++ pattern inside is treated as an "apply" function.
This makes it fit seamlessly with the current backend.

Fixes llvm#92410
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better ways to write GlobalISel combine "match" and "apply" functions
5 participants