[llvm] [GlobalISel] Prevent hoisting of CheckIsSameOperand from creating invalid match tables (PR #190963)
Pierre van Houtryve via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 8 05:04:30 PDT 2026
https://github.com/Pierre-vh created https://github.com/llvm/llvm-project/pull/190963
Fixes #188513
This patch adds logic to ask PredicateMatchers whether they'd like to be hoisted out of a specific Matcher or not.
SameOperandMatcher can use it to check if it's being hoisted out of the RuleMatcher that defines the operand it relies on.
Assisted-By: Claude Opus 4.6
Context of Use: Claude was only used to add LLVM-style RTTI to the matcher class (repetitive work). Claude-generated code was reviewed and cleaned up before committing.
>From ff6097b9e526dbf3f973ca298863b0b631b28a7f Mon Sep 17 00:00:00 2001
From: pvanhout <pierre.vanhoutryve at amd.com>
Date: Wed, 8 Apr 2026 13:51:49 +0200
Subject: [PATCH] [GlobalISel] Prevent hoisting of CheckIsSameOperand from
creating invalid match tables
Fixes #188513
This patch adds logic to ask PredicateMatchers whether they'd like to be hoisted out of a specific Matcher or not.
SameOperandMatcher can use it to check if it's being hoisted out of the RuleMatcher that defines the operand it relies on.
Assisted-By: Claude Opus 4.6
Context of Use: Claude was only used to add LLVM-style RTTI to the matcher class (repetitive work). Claude-generated code was reviewed and cleaned up before committing.
---
.../match-table-hoisting.td | 92 +++++++++++++++++++
.../GlobalISel/GlobalISelMatchTable.cpp | 30 +++---
.../Common/GlobalISel/GlobalISelMatchTable.h | 39 +++++++-
3 files changed, 143 insertions(+), 18 deletions(-)
create mode 100644 llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-hoisting.td
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-hoisting.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-hoisting.td
new file mode 100644
index 0000000000000..b3f0f760eac72
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-hoisting.td
@@ -0,0 +1,92 @@
+// 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 SharedPreds0 : GICombineRule<
+ (defs root:$root),
+ (match (G_SUB $sub1, $B, $C),
+ (G_ADD $add1, $A, $sub1),
+ (G_SUB $root, $add1, $B)),
+ (apply (G_SUB $root, $A, $C))>;
+
+def SharedPreds1 : GICombineRule<
+ (defs root:$root),
+ (match (G_ADD $add1, $B, $C),
+ (G_ADD $add2, $A, $add1),
+ (G_SUB $root, $add2, $B)),
+ (apply (G_ADD $root, $A, $C))>;
+
+def MyCombiner: GICombiner<"GenMyCombiner", [
+ SharedPreds0,
+ SharedPreds1
+]>;
+
+
+// CHECK: const uint8_t *GenMyCombiner::getMatchTable() const {
+// CHECK-NEXT: constexpr static uint8_t MatchTable0[] = {
+// CHECK-NEXT: /* 0 */ GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(100),
+// CHECK-NEXT: /* 5 */ GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_SUB),
+// CHECK-NEXT: /* 9 */ GIM_Try, /*On fail goto*//*Label 1*/ GIMT_Encode4(54), // Rule ID 1 //
+// CHECK-NEXT: /* 14 */ GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule1Enabled),
+// CHECK-NEXT: /* 17 */ // MIs[0] root
+// CHECK-NEXT: /* 17 */ // No operand predicates
+// CHECK-NEXT: /* 17 */ // MIs[0] add2
+// CHECK-NEXT: /* 17 */ GIM_RecordInsnIgnoreCopies, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
+// CHECK-NEXT: /* 21 */ GIM_CheckOpcode, /*MI*/1, GIMT_Encode2(TargetOpcode::G_ADD),
+// CHECK-NEXT: /* 25 */ // MIs[1] A
+// CHECK-NEXT: /* 25 */ // No operand predicates
+// CHECK-NEXT: /* 25 */ // MIs[1] add1
+// CHECK-NEXT: /* 25 */ GIM_RecordInsnIgnoreCopies, /*DefineMI*/2, /*MI*/1, /*OpIdx*/2, // MIs[2]
+// CHECK-NEXT: /* 29 */ GIM_CheckOpcode, /*MI*/2, GIMT_Encode2(TargetOpcode::G_ADD),
+// CHECK-NEXT: /* 33 */ // MIs[2] B
+// CHECK-NEXT: /* 33 */ // No operand predicates
+// CHECK-NEXT: /* 33 */ // MIs[2] C
+// CHECK-NEXT: /* 33 */ // No operand predicates
+// CHECK-NEXT: /* 33 */ // MIs[0] B
+// CHECK-NEXT: /* 33 */ GIM_CheckIsSameOperandIgnoreCopies, /*MI*/0, /*OpIdx*/2, /*OtherMI*/2, /*OtherOpIdx*/1,
+// CHECK-NEXT: /* 38 */ GIM_CheckIsSafeToFold, /*NumInsns*/2,
+// CHECK-NEXT: /* 40 */ // Combiner Rule #1: SharedPreds1
+// CHECK-NEXT: /* 40 */ GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(TargetOpcode::G_ADD),
+// CHECK-NEXT: /* 43 */ GIR_RootToRootCopy, /*OpIdx*/0, // root
+// CHECK-NEXT: /* 45 */ GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/1, // A
+// CHECK-NEXT: /* 49 */ GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/2, // C
+// CHECK-NEXT: /* 53 */ GIR_EraseRootFromParent_Done,
+// CHECK-NEXT: /* 54 */ // Label 1: @54
+// CHECK-NEXT: /* 54 */ GIM_Try, /*On fail goto*//*Label 2*/ GIMT_Encode4(99), // Rule ID 0 //
+// CHECK-NEXT: /* 59 */ GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
+// CHECK-NEXT: /* 62 */ // MIs[0] root
+// CHECK-NEXT: /* 62 */ // No operand predicates
+// CHECK-NEXT: /* 62 */ // MIs[0] add1
+// CHECK-NEXT: /* 62 */ GIM_RecordInsnIgnoreCopies, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
+// CHECK-NEXT: /* 66 */ GIM_CheckOpcode, /*MI*/1, GIMT_Encode2(TargetOpcode::G_ADD),
+// CHECK-NEXT: /* 70 */ // MIs[1] A
+// CHECK-NEXT: /* 70 */ // No operand predicates
+// CHECK-NEXT: /* 70 */ // MIs[1] sub1
+// CHECK-NEXT: /* 70 */ GIM_RecordInsnIgnoreCopies, /*DefineMI*/2, /*MI*/1, /*OpIdx*/2, // MIs[2]
+// CHECK-NEXT: /* 74 */ GIM_CheckOpcode, /*MI*/2, GIMT_Encode2(TargetOpcode::G_SUB),
+// CHECK-NEXT: /* 78 */ // MIs[2] B
+// CHECK-NEXT: /* 78 */ // No operand predicates
+// CHECK-NEXT: /* 78 */ // MIs[2] C
+// CHECK-NEXT: /* 78 */ // No operand predicates
+// CHECK-NEXT: /* 78 */ // MIs[0] B
+// CHECK-NEXT: /* 78 */ GIM_CheckIsSameOperandIgnoreCopies, /*MI*/0, /*OpIdx*/2, /*OtherMI*/2, /*OtherOpIdx*/1,
+// CHECK-NEXT: /* 83 */ GIM_CheckIsSafeToFold, /*NumInsns*/2,
+// CHECK-NEXT: /* 85 */ // Combiner Rule #0: SharedPreds0
+// CHECK-NEXT: /* 85 */ GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(TargetOpcode::G_SUB),
+// CHECK-NEXT: /* 88 */ GIR_RootToRootCopy, /*OpIdx*/0, // root
+// CHECK-NEXT: /* 90 */ GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/1, // A
+// CHECK-NEXT: /* 94 */ GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/2, // C
+// CHECK-NEXT: /* 98 */ GIR_EraseRootFromParent_Done,
+// CHECK-NEXT: /* 99 */ // Label 2: @99
+// CHECK-NEXT: /* 99 */ GIM_Reject,
+// CHECK-NEXT: /* 100 */ // Label 0: @100
+// CHECK-NEXT: /* 100 */ GIM_Reject,
+// CHECK-NEXT: /* 101 */ }; // Size: 101 bytes
+// CHECK-NEXT: return MatchTable0;
+// CHECK-NEXT: }
diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
index 8232ccacf3b45..1968097f91983 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
+++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
@@ -517,15 +517,6 @@ std::unique_ptr<PredicateMatcher> GroupMatcher::popFirstCondition() {
return P;
}
-/// Check if the Condition, which is a predicate of M, cannot be hoisted outside
-/// of (i.e., checked before) M.
-static bool cannotHoistCondition(const PredicateMatcher &Condition,
- const Matcher &M) {
- // The condition can't be hoisted if it is a C++ predicate that refers to
- // operands and the operands are registered within the matcher.
- return Condition.dependsOnOperands() && M.recordsOperand();
-}
-
bool GroupMatcher::addMatcher(Matcher &Candidate) {
if (!Candidate.hasFirstCondition())
return false;
@@ -534,7 +525,7 @@ bool GroupMatcher::addMatcher(Matcher &Candidate) {
// hoisted into the GroupMatcher.
const PredicateMatcher &Predicate = Candidate.getFirstCondition();
if (!candidateConditionMatches(Predicate) ||
- cannotHoistCondition(Predicate, Candidate))
+ !Predicate.canHoistOutsideOf(Candidate))
return false;
Matchers.push_back(&Candidate);
@@ -555,12 +546,12 @@ void GroupMatcher::finalize() {
// Hoist the first condition if it is identical in all matchers in the group
// and it can be hoisted in every matcher.
const auto &FirstCondition = FirstRule.getFirstCondition();
- if (cannotHoistCondition(FirstCondition, FirstRule))
+ if (!FirstCondition.canHoistOutsideOf(FirstRule))
return;
for (unsigned I = 1, E = Matchers.size(); I < E; ++I) {
const auto &OtherFirstCondition = Matchers[I]->getFirstCondition();
if (!OtherFirstCondition.isIdentical(FirstCondition) ||
- cannotHoistCondition(OtherFirstCondition, *Matchers[I]))
+ !OtherFirstCondition.canHoistOutsideOf(*Matchers[I]))
return;
}
@@ -757,7 +748,7 @@ void SwitchMatcher::emit(MatchTable &Table) {
//===- RuleMatcher --------------------------------------------------------===//
RuleMatcher::RuleMatcher(ArrayRef<SMLoc> SrcLoc)
- : SrcLoc(SrcLoc), RuleID(NextRuleID++) {}
+ : Matcher(Matcher::MK_Rule), SrcLoc(SrcLoc), RuleID(NextRuleID++) {}
uint64_t RuleMatcher::NextRuleID = 0;
@@ -1226,6 +1217,11 @@ void SameOperandMatcher::emitPredicateOpcodes(MatchTable &Table,
<< MatchTable::LineBreak;
}
+bool SameOperandMatcher::canHoistOutsideOf(const Matcher &M) const {
+ const auto *RM = dyn_cast<RuleMatcher>(&M);
+ return !RM || !RM->hasOperand(MatchingName);
+}
+
//===- LLTOperandMatcher --------------------------------------------------===//
std::map<LLTCodeGen, unsigned> LLTOperandMatcher::TypeIDValues;
@@ -1839,8 +1835,8 @@ void InstructionMatcher::emitPredicateOpcodes(MatchTable &Table,
// First emit all instruction level predicates need to be verified before we
// can verify operands.
emitFilteredPredicateListOpcodes(
- [](const PredicateMatcher &P) { return !P.dependsOnOperands(); }, Table,
- Rule);
+ [](const PredicateMatcher &P) { return !P.dependsOnRecordedOperands(); },
+ Table, Rule);
// Emit all operand constraints.
for (const auto &Operand : Operands)
@@ -1849,8 +1845,8 @@ void InstructionMatcher::emitPredicateOpcodes(MatchTable &Table,
// All of the tablegen defined predicates should now be matched. Now emit
// any custom predicates that rely on all generated checks.
emitFilteredPredicateListOpcodes(
- [](const PredicateMatcher &P) { return P.dependsOnOperands(); }, Table,
- Rule);
+ [](const PredicateMatcher &P) { return P.dependsOnRecordedOperands(); },
+ Table, Rule);
}
bool InstructionMatcher::isHigherPriorityThan(InstructionMatcher &B) {
diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
index 6a8017894a486..4749139049861 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
+++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
@@ -306,7 +306,17 @@ inline MatchTable &operator<<(MatchTable &Table,
//===- Matchers -----------------------------------------------------------===//
class Matcher {
public:
+ enum MatcherKind {
+ MK_Group,
+ MK_Switch,
+ MK_Rule,
+ };
+
+ Matcher(MatcherKind Kind) : Kind(Kind) {}
virtual ~Matcher();
+
+ MatcherKind getKind() const { return Kind; }
+
virtual void optimize();
virtual void emit(MatchTable &Table) = 0;
@@ -317,6 +327,9 @@ class Matcher {
/// Check recursively if the matcher records named operands for use in C++
/// predicates.
virtual bool recordsOperand() const = 0;
+
+private:
+ MatcherKind Kind;
};
class GroupMatcher final : public Matcher {
@@ -331,6 +344,10 @@ class GroupMatcher final : public Matcher {
std::vector<std::unique_ptr<Matcher>> MatcherStorage;
public:
+ GroupMatcher() : Matcher(MK_Group) {}
+
+ static bool classof(const Matcher *M) { return M->getKind() == MK_Group; }
+
/// Add a matcher to the collection of nested matchers if it meets the
/// requirements, and return true. If it doesn't, do nothing and return false.
///
@@ -421,6 +438,10 @@ class SwitchMatcher : public Matcher {
std::vector<std::unique_ptr<Matcher>> MatcherStorage;
public:
+ SwitchMatcher() : Matcher(MK_Switch) {}
+
+ static bool classof(const Matcher *M) { return M->getKind() == MK_Switch; }
+
bool addMatcher(Matcher &Candidate);
void finalize();
@@ -551,6 +572,8 @@ class RuleMatcher : public Matcher {
RuleMatcher(RuleMatcher &&Other) = default;
RuleMatcher &operator=(RuleMatcher &&Other) = default;
+ static bool classof(const Matcher *M) { return M->getKind() == MK_Rule; }
+
TempTypeIdx getNextTempTypeIdx() { return NextTempTypeIdx--; }
uint64_t getRuleID() const { return RuleID; }
@@ -858,13 +881,17 @@ class PredicateMatcher {
PredicateKind getKind() const { return Kind; }
- bool dependsOnOperands() const {
+ bool dependsOnRecordedOperands() const {
// Custom predicates really depend on the context pattern of the
// instruction, not just the individual instruction. This therefore
// implicitly depends on all other pattern constraints.
return Kind == IPM_GenericPredicate;
}
+ /// \param M A Matcher that contains this PredicateMatcher.
+ /// \returns true if this PredicateMatcher can be hoisted outside of \p M.
+ virtual bool canHoistOutsideOf(const Matcher &M) const { return true; }
+
bool recordsOperand() const { return Kind == OPM_RecordNamedOperand; }
virtual bool isIdentical(const PredicateMatcher &B) const {
@@ -938,6 +965,8 @@ class SameOperandMatcher : public OperandPredicateMatcher {
OrigOpIdx == cast<SameOperandMatcher>(&B)->OrigOpIdx &&
MatchingName == cast<SameOperandMatcher>(&B)->MatchingName;
}
+
+ virtual bool canHoistOutsideOf(const Matcher &M) const override;
};
/// Generates code to check that an operand is a particular LLT.
@@ -1698,6 +1727,14 @@ class GenericInstructionPredicateMatcher : public InstructionPredicateMatcher {
bool isIdentical(const PredicateMatcher &B) const override;
void emitPredicateOpcodes(MatchTable &Table,
RuleMatcher &Rule) const override;
+
+ bool canHoistOutsideOf(const Matcher &M) const override {
+ // We can only hoist C++ code if the parent Matcher does not define any
+ // symbol that may be used by C++ code.
+ // TODO?: Could we be more precise, e.g. hoist if the Matcher records
+ // operands, but the operands aren't used by this bit of C++.
+ return !M.recordsOperand();
+ }
};
class MIFlagsInstructionPredicateMatcher : public InstructionPredicateMatcher {
More information about the llvm-commits
mailing list