[GlobalISel] Refactor Combiner MatchData & Apply C++ Code Handling #92239
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Combiners that use C++ code in their "apply" pattern only use that. They never mix it with MIR patterns as that has little added value. This patch restricts C++ apply code so that if C++ is used, we cannot use MIR patterns or builtins with it. Adding this restriction allows us to merge calls to match and apply C++ code together, which in turns makes it so we can just have MatchData variables on the stack. So before, we would have ``` GIM_CheckCxxInsnPredicate // match GIM_CheckCxxInsnPredicate // apply GIR_Done ``` Alongside a massive C++ struct holding the MatchData of all rules possible (which was a big space/perf issue). Now we just have ``` GIR_DoneWithCustomAction ``` And the function being ran just does ``` unsigned SomeMatchData; if (match(SomeMatchData)) apply(SomeMatchData) ``` This approach solves multiple issues in one: - MatchData handling is greatly simplified and more efficient, "don't pay for what you don'tt use" - We reduce the size of the match table - Calling C++ code has a certain overhead (we need a switch), and this overhead is only paid once now. Handling of C++ code inside PatFrags is unchanged though, that still emits a `GIM_CheckCxxInsnPredicate`. This is completely fine as they can't use MatchDatas.
|
@llvm/pr-subscribers-llvm-globalisel Author: Pierre van Houtryve (Pierre-vh) ChangesCombiners that use C++ code in their "apply" pattern only use that. They never mix it with MIR patterns as that has little added value. This patch restricts C++ apply code so that if C++ is used, we cannot use MIR patterns or builtins with it. Adding this restriction allows us to merge calls to match and apply C++ code together, which in turns makes it so we can just have MatchData variables on the stack. So before, we would have Alongside a massive C++ struct holding the MatchData of all rules possible (which was a big space/perf issue). Now we just have And the function being ran just does This approach solves multiple issues in one:
Handling of C++ code inside PatFrags is unchanged though, that still emits a Patch is 88.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/92239.diff 25 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
index 72c63ecba529f..371c5c5a0a1e1 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
@@ -471,16 +471,11 @@ enum {
/// - RendererFnID(2) - Custom renderer function to call
GIR_CustomRenderer,
- /// Calls a C++ function to perform an action when a match is complete.
- /// The MatcherState is passed to the function to allow it to modify
- /// instructions.
- /// This is less constrained than a custom renderer and can update
- /// instructions
- /// in the state.
+ /// Calls a C++ function that concludes the current match.
+ /// The C++ function is free to return false and reject the match, or
+ /// return true and mutate the instruction(s) (or do nothing, even).
/// - FnID(2) - The function to call.
- /// TODO: Remove this at some point when combiners aren't reliant on it. It's
- /// a bit of a hack.
- GIR_CustomAction,
+ GIR_DoneWithCustomAction,
/// Render operands to the specified instruction using a custom function,
/// reading from a specific operand.
@@ -688,7 +683,7 @@ class GIMatchTableExecutor {
llvm_unreachable("Subclass does not implement testSimplePredicate!");
}
- virtual void runCustomAction(unsigned, const MatcherState &State,
+ virtual bool runCustomAction(unsigned, const MatcherState &State,
NewMIVector &OutMIs) const {
llvm_unreachable("Subclass does not implement runCustomAction!");
}
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 4d147bf20c26a..6b6f5f687fd0c 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -1335,13 +1335,17 @@ bool GIMatchTableExecutor::executeMatchTable(
-1); // Not a source operand of the old instruction.
break;
}
- case GIR_CustomAction: {
+ case GIR_DoneWithCustomAction: {
uint16_t FnID = readU16();
DEBUG_WITH_TYPE(TgtExecutor::getName(),
- dbgs() << CurrentIdx << ": GIR_CustomAction(FnID=" << FnID
- << ")\n");
+ dbgs() << CurrentIdx << ": GIR_DoneWithCustomAction(FnID="
+ << FnID << ")\n");
assert(FnID > GICXXCustomAction_Invalid && "Expected a valid FnID");
- runCustomAction(FnID, State, OutMIs);
+ if (runCustomAction(FnID, State, OutMIs)) {
+ propagateFlags();
+ return true;
+ } else if (handleReject() == RejectAndGiveUp)
+ return false;
break;
}
case GIR_CustomOperandRenderer: {
diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index 98d266c8c0b4f..d61a5759d5a96 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -338,7 +338,7 @@ def bitreverse_shl : GICombineRule<
// Combine bitreverse(lshr (bitreverse x), y)) -> (shl x, y)
def bitreverse_lshr : GICombineRule<
- (defs root:$d, build_fn_matchinfo:$matchinfo),
+ (defs root:$d),
(match (G_BITREVERSE $rev, $val),
(G_LSHR $src, $rev, $amt):$mi,
(G_BITREVERSE $d, $src),
@@ -1352,7 +1352,7 @@ def match_extract_of_element : GICombineRule<
(apply [{ Helper.applyBuildFn(*${root}, ${matchinfo}); }])>;
def extract_vector_element_not_const : GICombineRule<
- (defs root:$root, build_fn_matchinfo:$matchinfo),
+ (defs root:$root),
(match (G_INSERT_VECTOR_ELT $src, $x, $value, $idx),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx)),
(apply (GIReplaceReg $root, $value))>;
@@ -1567,20 +1567,20 @@ def combine_shuffle_concat : GICombineRule<
(apply [{ Helper.applyCombineShuffleConcat(*${root}, ${matchinfo}); }])>;
def insert_vector_element_idx_undef : GICombineRule<
- (defs root:$root, build_fn_matchinfo:$matchinfo),
+ (defs root:$root),
(match (G_IMPLICIT_DEF $idx),
(G_INSERT_VECTOR_ELT $root, $src, $elt, $idx)),
(apply (G_IMPLICIT_DEF $root))>;
def insert_vector_element_elt_undef : GICombineRule<
- (defs root:$root, build_fn_matchinfo:$matchinfo),
+ (defs root:$root),
(match (G_IMPLICIT_DEF $elt),
(G_INSERT_VECTOR_ELT $root, $src, $elt, $idx),
[{ return isGuaranteedNotToBePoison(${src}.getReg(), MRI); }]),
(apply (GIReplaceReg $root, $src))>;
def insert_vector_element_extract_vector_element : GICombineRule<
- (defs root:$root, build_fn_matchinfo:$matchinfo),
+ (defs root:$root),
(match (G_EXTRACT_VECTOR_ELT $elt, $src, $idx),
(G_INSERT_VECTOR_ELT $root, $src, $elt, $idx)),
(apply (GIReplaceReg $root, $src))>;
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-cxx.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-cxx.td
new file mode 100644
index 0000000000000..3e362dca3b56c
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-cxx.td
@@ -0,0 +1,140 @@
+// RUN: llvm-tblgen -I %p/../../../include -gen-global-isel-combiner \
+// RUN: -combiners=MyCombiner %s | \
+// RUN: FileCheck %s
+
+include "llvm/Target/Target.td"
+include "llvm/Target/GlobalISel/Combine.td"
+
+def MyTargetISA : InstrInfo;
+def MyTarget : Target { let InstructionSet = MyTargetISA; }
+
+def OneMatchOneApply : GICombineRule<
+ (defs root:$a),
+ (match (G_FABS $a, $b), "return MATCH0;"),
+ (apply "APPLY0")>;
+
+def TwoMatchTwoApply : GICombineRule<
+ (defs root:$a),
+ (match (G_FNEG $a, $b), "return MATCH0;", "return MATCH1;"),
+ (apply "APPLY0", "APPLY1")>;
+
+def TwoMatchNoApply : GICombineRule<
+ (defs root:$a),
+ (match (G_STORE $x, $y):$a, "return MATCH0;", "return MATCH1;"),
+ (apply (GIEraseRoot))>;
+
+def NoMatchTwoApply : GICombineRule<
+ (defs root:$a),
+ (match (G_SEXT $a, $y)),
+ (apply "APPLY0", "APPLY1")>;
+
+def MyCombiner: GICombiner<"GenMyCombiner", [
+ OneMatchOneApply,
+ TwoMatchTwoApply,
+ TwoMatchNoApply,
+ NoMatchTwoApply
+]>;
+
+// CHECK: bool GenMyCombiner::testMIPredicate_MI(unsigned PredicateID, const MachineInstr & MI, const MatcherState &State) const {
+// CHECK-NEXT: switch (PredicateID) {
+// CHECK-NEXT: case GICXXPred_MI_Predicate_GICombiner0: {
+// CHECK-NEXT: return MATCH0;
+// CHECK-NEXT: }
+// CHECK-NEXT: case GICXXPred_MI_Predicate_GICombiner1: {
+// CHECK-NEXT: return MATCH1;
+// CHECK-NEXT: }
+// CHECK-NEXT: }
+// CHECK-NEXT: llvm_unreachable("Unknown predicate");
+// CHECK-NEXT: return false;
+// CHECK-NEXT: }
+
+// CHECK: bool GenMyCombiner::runCustomAction(unsigned ApplyID, const MatcherState &State, NewMIVector &OutMIs) const {
+// CHECK-NEXT: Helper.getBuilder().setInstrAndDebugLoc(*State.MIs[0]);
+// CHECK-NEXT: switch(ApplyID) {
+// CHECK-NEXT: case GICXXCustomAction_GICombiner0:{
+// CHECK-NEXT: // Match Patterns
+// CHECK-NEXT: if(![&](){return MATCH0;}()) {
+// CHECK-NEXT: return false;
+// CHECK-NEXT: }
+// CHECK-NEXT: // Apply Patterns
+// CHECK-NEXT: APPLY0
+// CHECK-NEXT: return true;
+// CHECK-NEXT: }
+// CHECK-NEXT: case GICXXCustomAction_GICombiner1:{
+// CHECK-NEXT: // Match Patterns
+// CHECK-NEXT: if(![&](){return MATCH0;}()) {
+// CHECK-NEXT: return false;
+// CHECK-NEXT: }
+// CHECK-NEXT: if(![&](){return MATCH1;}()) {
+// CHECK-NEXT: return false;
+// CHECK-NEXT: }
+// CHECK-NEXT: // Apply Patterns
+// CHECK-NEXT: APPLY0
+// CHECK-NEXT: APPLY1
+// CHECK-NEXT: return true;
+// CHECK-NEXT: }
+// CHECK-NEXT: case GICXXCustomAction_GICombiner2:{
+// CHECK-NEXT: // Apply Patterns
+// CHECK-NEXT: APPLY0
+// CHECK-NEXT: APPLY1
+// 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(94), GIMT_Encode2(194), /*)*//*default:*//*Label 4*/ GIMT_Encode4(464),
+// CHECK-NEXT: /*TargetOpcode::G_STORE*//*Label 0*/ GIMT_Encode4(410), 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(428), 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(440), 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(452),
+// CHECK-NEXT: // Label 0: @410
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 5*/ GIMT_Encode4(427), // 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: @427
+// CHECK-NEXT: GIM_Reject,
+// CHECK-NEXT: // Label 1: @428
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 6*/ GIMT_Encode4(439), // 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: GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner2),
+// CHECK-NEXT: // Label 6: @439
+// CHECK-NEXT: GIM_Reject,
+// CHECK-NEXT: // Label 2: @440
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 7*/ GIMT_Encode4(451), // 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: GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner1),
+// CHECK-NEXT: // Label 7: @451
+// CHECK-NEXT: GIM_Reject,
+// CHECK-NEXT: // Label 3: @452
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 8*/ GIMT_Encode4(463), // 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: GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner0),
+// CHECK-NEXT: // Label 8: @463
+// CHECK-NEXT: GIM_Reject,
+// CHECK-NEXT: // Label 4: @464
+// CHECK-NEXT: GIM_Reject,
+// CHECK-NEXT: }; // Size: 465 bytes
+// CHECK-NEXT: return MatchTable0;
+// CHECK-NEXT: }
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-permutations.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-permutations.td
index fda57d5b64e02..7c2c013094061 100644
--- a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-permutations.td
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-permutations.td
@@ -24,7 +24,7 @@ def Test0 : GICombineRule<
(MatchFooPerms $cst1, (i32 20)):$b,
(MatchFooPerms $cst2, (i32 30)):$c
),
- (apply (COPY $dst, (i32 0)), "APPLY ${cst0}")>;
+ (apply "APPLY ${cst0}")>;
def MyCombiner: GICombiner<"GenMyCombiner", [
Test0
@@ -159,9 +159,9 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK: const uint8_t *GenMyCombiner::getMatchTable() const {
// CHECK-NEXT: constexpr static uint8_t MatchTable0[] = {
-// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(738),
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(562),
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_AND),
-// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 1*/ GIMT_Encode4(88), // Rule ID 7 //
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 1*/ GIMT_Encode4(66), // Rule ID 7 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
// CHECK-NEXT: // MIs[0] dst
// CHECK-NEXT: // No operand predicates
@@ -187,16 +187,9 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner22),
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner23),
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*NumInsns*/4,
-// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
-// CHECK-NEXT: GIR_BuildConstant, /*TempRegID*/0, /*Val*/GIMT_Encode8(0),
-// CHECK-NEXT: // Combiner Rule #0: Test0 @ [a[1], b[1], c[1]]
-// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
-// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // dst
-// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
-// CHECK-NEXT: GIR_CustomAction, GIMT_Encode2(GICXXCustomAction_CombineApplyGICombiner0),
-// CHECK-NEXT: GIR_EraseRootFromParent_Done,
-// CHECK-NEXT: // Label 1: @88
-// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 2*/ GIMT_Encode4(175), // Rule ID 6 //
+// CHECK-NEXT: GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner0),
+// CHECK-NEXT: // Label 1: @66
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 2*/ GIMT_Encode4(131), // Rule ID 6 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
// CHECK-NEXT: // MIs[0] dst
// CHECK-NEXT: // No operand predicates
@@ -225,16 +218,9 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner19),
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner20),
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*NumInsns*/5,
-// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
-// CHECK-NEXT: GIR_BuildConstant, /*TempRegID*/0, /*Val*/GIMT_Encode8(0),
-// CHECK-NEXT: // Combiner Rule #0: Test0 @ [a[1], b[1], c[0]]
-// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
-// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // dst
-// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
-// CHECK-NEXT: GIR_CustomAction, GIMT_Encode2(GICXXCustomAction_CombineApplyGICombiner0),
-// CHECK-NEXT: GIR_EraseRootFromParent_Done,
-// CHECK-NEXT: // Label 2: @175
-// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 3*/ GIMT_Encode4(262), // Rule ID 5 //
+// CHECK-NEXT: GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner0),
+// CHECK-NEXT: // Label 2: @131
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 3*/ GIMT_Encode4(196), // Rule ID 5 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
// CHECK-NEXT: // MIs[0] dst
// CHECK-NEXT: // No operand predicates
@@ -263,16 +249,9 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner16),
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner17),
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*NumInsns*/5,
-// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
-// CHECK-NEXT: GIR_BuildConstant, /*TempRegID*/0, /*Val*/GIMT_Encode8(0),
-// CHECK-NEXT: // Combiner Rule #0: Test0 @ [a[1], b[0], c[1]]
-// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
-// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // dst
-// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
-// CHECK-NEXT: GIR_CustomAction, GIMT_Encode2(GICXXCustomAction_CombineApplyGICombiner0),
-// CHECK-NEXT: GIR_EraseRootFromParent_Done,
-// CHECK-NEXT: // Label 3: @262
-// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 4*/ GIMT_Encode4(357), // Rule ID 4 //
+// CHECK-NEXT: GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner0),
+// CHECK-NEXT: // Label 3: @196
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 4*/ GIMT_Encode4(269), // Rule ID 4 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
// CHECK-NEXT: // MIs[0] dst
// CHECK-NEXT: // No operand predicates
@@ -304,16 +283,9 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner13),
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_GICombiner14),
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*NumInsns*/6,
-// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
-// CHECK-NEXT: GIR_BuildConstant, /*TempRegID*/0, /*Val*/GIMT_Encode8(0),
-// CHECK-NEXT: // Combiner Rule #0: Test0 @ [a[1], b[0], c[0]]
-// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
-// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // dst
-// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
-// CHECK-NEXT: GIR_CustomAction, GIMT_Encode2(GICXXCustomAction_CombineApplyGICombiner0),
-// CHECK-NEXT: GIR_EraseRootFromParent_Done,
-// CHECK-NEXT: // Label 4: @357
-// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 5*/ GIMT_Encode4(444), // Rule ID 3 //
+// CHECK-NEXT: GIR_DoneWithCustomAction, /*Fn*/GIMT_Encode2(GICXXCustomAction_GICombiner0),
+// CHECK-NEXT: // Label 4: @269
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 5*/ GIMT_Encode4(334), // Rule ID 3 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
// CHECK-NEXT: // MIs[0] dst
// CHECK-NEXT: // No operand pre...
[truncated]
|
|
Awesome! Thanks for doing this so quickly. Do the match and apply functions get inlined into this? |
They probably will be inlined if it's possible (simplified example: https://godbolt.org/z/17vY68hx3), but it's all in one big switch so maybe size constraints will prevent everything from being inlined all the time |
arsenm
reviewed
May 15, 2024
llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
Outdated
Show resolved
Hide resolved
|
I'll run the CT tests on this too... |
arsenm
reviewed
May 15, 2024
|
A non-LTO release w/o asserts build: A huge improvement, fantastic! |
arsenm
reviewed
May 16, 2024
arsenm
approved these changes
May 16, 2024
Co-authored-by: Matt Arsenault <[email protected]>
Co-authored-by: Matt Arsenault <[email protected]>
Co-authored-by: Matt Arsenault <[email protected]>
You can test this locally with the following command:git-clang-format --diff 4014e2e045f5160ce9cbb9562d151f540d61c0bb 6116cba125fe109d1565430acb787db6fd819cb7 -- llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h llvm/utils/TableGen/Common/GlobalISel/CXXPredicates.cpp llvm/utils/TableGen/Common/GlobalISel/CXXPredicates.h llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTableExecutorEmitter.cpp llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTableExecutorEmitter.h llvm/utils/TableGen/Common/GlobalISel/Patterns.cpp llvm/utils/TableGen/Common/GlobalISel/Patterns.h llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp llvm/utils/TableGen/GlobalISelEmitter.cppView the diff from clang-format here.diff --git a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
index 085841ed96..e6d728b4e4 100644
--- a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
@@ -1839,7 +1839,7 @@ bool CombineRuleBuilder::emitCXXMatchApply(CodeExpansions &CE, RuleMatcher &M,
for (auto &Pat : ApplyPats) {
auto *CXXPat = cast<CXXPattern>(Pat.second.get());
CodeExpander Expander(CXXPat->getRawCode(), CE, RuleDef.getLoc(),
- /*ShowExpansions=*/ false);
+ /*ShowExpansions=*/false);
OS << LS;
Expander.emit(OS);
}
|
You can test this locally with the following command:git-clang-format --diff 4014e2e045f5160ce9cbb9562d151f540d61c0bb 2dc0b8248b9e82de2f5c8c7f63c72108b2a2074b -- llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h llvm/utils/TableGen/Common/GlobalISel/CXXPredicates.cpp llvm/utils/TableGen/Common/GlobalISel/CXXPredicates.h llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTableExecutorEmitter.cpp llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTableExecutorEmitter.h llvm/utils/TableGen/Common/GlobalISel/Patterns.cpp llvm/utils/TableGen/Common/GlobalISel/Patterns.h llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp llvm/utils/TableGen/GlobalISelEmitter.cppView the diff from clang-format here.diff --git a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
index 0672e2ad6d..d02cb2fe4a 100644
--- a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
@@ -1839,7 +1839,7 @@ bool CombineRuleBuilder::emitCXXMatchApply(CodeExpansions &CE, RuleMatcher &M,
for (auto &Pat : ApplyPats) {
auto *CXXPat = cast<CXXPattern>(Pat.second.get());
CodeExpander Expander(CXXPat->getRawCode(), CE, RuleDef.getLoc(),
- /*ShowExpansions=*/ false);
+ /*ShowExpansions=*/false);
OS << LS;
Expander.emit(OS);
}
|
This was referenced May 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Combiners that use C++ code in their "apply" pattern only use that. They never mix it with MIR patterns as that has little added value.
This patch restricts C++ apply code so that if C++ is used, we cannot use MIR patterns or builtins with it. Adding this restriction allows us to merge calls to match and apply C++ code together, which in turns makes it so we can just have MatchData variables on the stack.
So before, we would have
Alongside a massive C++ struct holding the MatchData of all rules possible (which was a big space/perf issue).
Now we just have
And the function being ran just does
This approach solves multiple issues in one:
Handling of C++ code inside PatFrags is unchanged though, that still emits a
GIM_CheckCxxInsnPredicate. This is completely fine as they can't use MatchDatas.