llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-globalisel Author: Pierre van Houtryve (Pierre-vh) <details> <summary>Changes</summary> Some PredicateMatchers/MatchAction/OperandRenderers relied on accessing RuleMatcher at emission as a crutch. Instead, make these classes collect all necessary information in the constructor so the `emit` methods don't depend on RuleMatcher anymore. The primary motivation for this is that I've been looking at ways to optimize the MatchTable better, and the fact that Predicates/Actions/Renderers are not "pure" objects, in the sense that they keep accessing a bunch of data all over the place even as late as emission, was a consistent pain. This is NFCI. There are no changes to any of the match table for AMDGPU/AArch64 in this patch. This patch has a bunch of noise due to function signature changes so I'll highlight the following interesting changes: - `SameOperandMatcher` needed a bit of an update in its `canHoistOutsideOf` function. I had to rewrite it but I think the end result is the same. - `EraseInstAction` has been updated as well, and its users in both Combiner/ISel backends have been updated to. Instead of ignoring this action if the Inst was already erased, it's now the responsibility of the builder to never insert it in the first place. `BuildMIAction` had a small update because of that too. Assisted-By: Claude Sonnet 4.6 Context-of-Use: I used Claude to mass-remove the `RuleMatcher&` arguments from the `emitPredicateOpcode` methods. It did not write any logic. --- Patch is 81.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/200799.diff 4 Files Affected: - (modified) llvm/utils/TableGen/Common/GlobalISel/MatchTable/Matchers.cpp (+109-194) - (modified) llvm/utils/TableGen/Common/GlobalISel/MatchTable/Matchers.h (+161-134) - (modified) llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp (+7-5) - (modified) llvm/utils/TableGen/GlobalISelEmitter.cpp (+21-19) ``````````diff diff --git a/llvm/utils/TableGen/Common/GlobalISel/MatchTable/Matchers.cpp b/llvm/utils/TableGen/Common/GlobalISel/MatchTable/Matchers.cpp index 5111702d6e7e2..5f57537a49458 100644 --- a/llvm/utils/TableGen/Common/GlobalISel/MatchTable/Matchers.cpp +++ b/llvm/utils/TableGen/Common/GlobalISel/MatchTable/Matchers.cpp @@ -249,8 +249,7 @@ void GroupMatcher::emit(MatchTable &Table) { << MatchTable::JumpTarget(LabelID) << MatchTable::LineBreak; } for (auto &Condition : Conditions) - Condition->emitPredicateOpcodes( - Table, *static_cast<RuleMatcher *>(*Matchers.begin())); + Condition->emitPredicateOpcodes(Table); for (const auto &M : Matchers) M->emit(Table); @@ -671,9 +670,9 @@ void RuleMatcher::defineOperand(StringRef SymbolicName, OperandMatcher &OM) { // If the operand is already defined, then we must ensure both references in // the matcher have the exact same node. RuleMatcher &RM = OM.getInstructionMatcher().getRuleMatcher(); - OM.addPredicate<SameOperandMatcher>( - OM.getSymbolicName(), getOperandMatcher(OM.getSymbolicName()).getOpIdx(), - RM.getGISelFlags()); + auto &OtherOM = getOperandMatcher(OM.getSymbolicName()); + OM.addPredicate<SameOperandMatcher>(OtherOM.getInsnVarID(), + OtherOM.getOpIdx(), RM.getGISelFlags()); } void RuleMatcher::definePhysRegOperand(const Record *Reg, OperandMatcher &OM) { @@ -758,11 +757,11 @@ void RuleMatcher::emit(MatchTable &Table) { } } - Roots.front()->emitPredicateOpcodes(Table, *this); + Roots.front()->emitPredicateOpcodes(Table); // Check if it's safe to replace registers. for (const auto &MA : Actions) - MA->emitAdditionalPredicates(Table, *this); + MA->emitAdditionalPredicates(Table); // We must also check if it's safe to fold the matched instructions. if (InsnMatchers.size() >= 2) { @@ -809,7 +808,7 @@ void RuleMatcher::emit(MatchTable &Table) { } for (const auto &PM : EpilogueMatchers) - PM->emitPredicateOpcodes(Table, *this); + PM->emitPredicateOpcodes(Table); if (!CustomCXXAction.empty()) { /// Handle combiners relying on custom C++ code instead of actions. @@ -819,7 +818,7 @@ void RuleMatcher::emit(MatchTable &Table) { return A->getKind() != MatchAction::AK_DebugComment; })); for (const auto &MA : Actions) - MA->emitActionOpcodes(Table, *this); + MA->emitActionOpcodes(Table); Table << MatchTable::Opcode("GIR_DoneWithCustomAction", -1) << MatchTable::Comment("Fn") << MatchTable::NamedValue(2, CustomCXXAction) @@ -832,21 +831,25 @@ void RuleMatcher::emit(MatchTable &Table) { // double as a GIR_Done and terminate execution of the rule. if (!Actions.empty()) { for (const auto &MA : drop_end(Actions)) - MA->emitActionOpcodes(Table, *this); + MA->emitActionOpcodes(Table); } - assert((Table.isWithCoverage() ? !Table.isCombiner() : true) && - "Combiner tables don't support coverage!"); - if (Table.isWithCoverage()) - Table << MatchTable::Opcode("GIR_Coverage") - << MatchTable::IntValue(4, RuleID) << MatchTable::LineBreak; - else if (!Table.isCombiner()) - Table << MatchTable::Comment( - ("GIR_Coverage, " + Twine(RuleID) + ",").str()) - << MatchTable::LineBreak; + // Emit coverage right before the Done opcode> + auto EmitCoverage = [&] { + assert((Table.isWithCoverage() ? !Table.isCombiner() : true) && + "Combiner tables don't support coverage!"); + if (Table.isWithCoverage()) + Table << MatchTable::Opcode("GIR_Coverage") + << MatchTable::IntValue(4, RuleID) << MatchTable::LineBreak; + else if (!Table.isCombiner()) + Table << MatchTable::Comment( + ("GIR_Coverage, " + Twine(RuleID) + ",").str()) + << MatchTable::LineBreak; + }; if (Actions.empty() || - !Actions.back()->emitActionOpcodesAndDone(Table, *this)) { + !Actions.back()->emitActionOpcodesAndDone(Table, EmitCoverage)) { + EmitCoverage(); Table << MatchTable::Opcode("GIR_Done", -1) << MatchTable::LineBreak; } } @@ -918,10 +921,7 @@ bool OperandPredicateMatcher::isHigherPriorityThan( //===- SameOperandMatcher -------------------------------------------------===// -void SameOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { - const OperandMatcher &OtherOM = Rule.getOperandMatcher(MatchingName); - unsigned OtherInsnVarID = OtherOM.getInstructionMatcher().getInsnVarID(); +void SameOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { const bool IgnoreCopies = Flags & GISF_IgnoreCopies; Table << MatchTable::Opcode(IgnoreCopies ? "GIM_CheckIsSameOperandIgnoreCopies" @@ -929,15 +929,9 @@ void SameOperandMatcher::emitPredicateOpcodes(MatchTable &Table, << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("OpIdx") << MatchTable::ULEB128Value(OpIdx) << MatchTable::Comment("OtherMI") - << MatchTable::ULEB128Value(OtherInsnVarID) + << MatchTable::ULEB128Value(OtherInsnID) << MatchTable::Comment("OtherOpIdx") - << MatchTable::ULEB128Value(OtherOM.getOpIdx()) - << MatchTable::LineBreak; -} - -bool SameOperandMatcher::canHoistOutsideOf(const Matcher &M) const { - const auto *RM = dyn_cast<RuleMatcher>(&M); - return !RM || !RM->hasOperand(MatchingName); + << MatchTable::ULEB128Value(OtherOpIdx) << MatchTable::LineBreak; } //===- LLTOperandMatcher --------------------------------------------------===// @@ -957,8 +951,7 @@ bool LLTOperandMatcher::hasValue() const { return TypeIDValues.count(Ty); } -void LLTOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void LLTOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { if (InsnVarID == 0) { Table << MatchTable::Opcode("GIM_RootCheckType"); } else { @@ -972,8 +965,7 @@ void LLTOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- PointerToAnyOperandMatcher -----------------------------------------===// -void PointerToAnyOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void PointerToAnyOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckPointerToAny") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx) @@ -983,8 +975,7 @@ void PointerToAnyOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- RecordNamedOperandMatcher ------------------------------------------===// -void RecordNamedOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void RecordNamedOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_RecordNamedOperand") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx) @@ -994,8 +985,7 @@ void RecordNamedOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- RecordRegisterType ------------------------------------------===// -void RecordRegisterType::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void RecordRegisterType::emitPredicateOpcodes(MatchTable &Table) const { assert(Idx < 0 && "Temp types always have negative indexes!"); Table << MatchTable::Opcode("GIM_RecordRegType") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Op") @@ -1006,7 +996,7 @@ void RecordRegisterType::emitPredicateOpcodes(MatchTable &Table, //===- ComplexPatternOperandMatcher ---------------------------------------===// void ComplexPatternOperandMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { unsigned ID = getAllocatedTemporariesBaseID(); Table << MatchTable::Opcode("GIM_CheckComplexPattern") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) @@ -1027,8 +1017,7 @@ bool RegisterBankOperandMatcher::isIdentical(const PredicateMatcher &B) const { RC.getDef() == cast<RegisterBankOperandMatcher>(&B)->RC.getDef(); } -void RegisterBankOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void RegisterBankOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { if (InsnVarID == 0) { Table << MatchTable::Opcode("GIM_RootCheckRegBankForClass"); } else { @@ -1044,8 +1033,7 @@ void RegisterBankOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- MBBOperandMatcher --------------------------------------------------===// -void MBBOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void MBBOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckIsMBB") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx) << MatchTable::LineBreak; @@ -1053,8 +1041,7 @@ void MBBOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- ImmOperandMatcher --------------------------------------------------===// -void ImmOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void ImmOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckIsImm") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx) << MatchTable::LineBreak; @@ -1062,8 +1049,7 @@ void ImmOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- ConstantIntOperandMatcher ------------------------------------------===// -void ConstantIntOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void ConstantIntOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { const bool IsInt8 = isInt<8>(Value); Table << MatchTable::Opcode(IsInt8 ? "GIM_CheckConstantInt8" : "GIM_CheckConstantInt") @@ -1074,8 +1060,7 @@ void ConstantIntOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- LiteralIntOperandMatcher -------------------------------------------===// -void LiteralIntOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void LiteralIntOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckLiteralInt") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx) @@ -1084,8 +1069,7 @@ void LiteralIntOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- CmpPredicateOperandMatcher -----------------------------------------===// -void CmpPredicateOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void CmpPredicateOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckCmpPredicate") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx) @@ -1096,8 +1080,7 @@ void CmpPredicateOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- IntrinsicIDOperandMatcher ------------------------------------------===// -void IntrinsicIDOperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void IntrinsicIDOperandMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckIntrinsicID") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx) @@ -1107,8 +1090,7 @@ void IntrinsicIDOperandMatcher::emitPredicateOpcodes(MatchTable &Table, //===- OperandImmPredicateMatcher -----------------------------------------===// -void OperandImmPredicateMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void OperandImmPredicateMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckImmOperandPredicate") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("MO") << MatchTable::ULEB128Value(OpIdx) @@ -1120,7 +1102,7 @@ void OperandImmPredicateMatcher::emitPredicateOpcodes(MatchTable &Table, //===- OperandLeafPredicateMatcher ----------------------------------------===// void OperandLeafPredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckLeafOperandPredicate") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("MO") << MatchTable::ULEB128Value(OpIdx) @@ -1155,8 +1137,7 @@ bool OperandMatcher::recordsOperand() const { return matchersRecordOperand(Predicates); } -void OperandMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) { +void OperandMatcher::emitPredicateOpcodes(MatchTable &Table) { if (!Optimized) { std::string Comment; raw_string_ostream CommentOS(Comment); @@ -1168,7 +1149,7 @@ void OperandMatcher::emitPredicateOpcodes(MatchTable &Table, Table << MatchTable::Comment(Comment) << MatchTable::LineBreak; } - emitPredicateListOpcodes(Table, Rule); + emitPredicateListOpcodes(Table); } bool OperandMatcher::isHigherPriorityThan(OperandMatcher &B) { @@ -1261,8 +1242,7 @@ RecordAndValue InstructionOpcodeMatcher::getValue() const { return MatchTable::NamedValue(2, I->Namespace, I->getName()); } -void InstructionOpcodeMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void InstructionOpcodeMatcher::emitPredicateOpcodes(MatchTable &Table) const { StringRef CheckType = Insts.size() == 1 ? "GIM_CheckOpcode" : "GIM_CheckOpcodeIsEither"; Table << MatchTable::Opcode(CheckType) << MatchTable::Comment("MI") @@ -1311,7 +1291,7 @@ StringRef InstructionOpcodeMatcher::getOperandType(unsigned OpIdx) const { //===- InstructionNumOperandsMatcher --------------------------------------===// void InstructionNumOperandsMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { StringRef Opc; switch (CK) { case CheckKind::Eq: @@ -1341,7 +1321,7 @@ bool InstructionImmPredicateMatcher::isIdentical( } void InstructionImmPredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { Table << MatchTable::Opcode(getMatchOpcodeForImmPredicate(Predicate)) << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("Predicate") @@ -1360,7 +1340,7 @@ bool AtomicOrderingMMOPredicateMatcher::isIdentical( } void AtomicOrderingMMOPredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { StringRef Opcode = "GIM_CheckAtomicOrdering"; if (Comparator == AO_OrStronger) @@ -1377,8 +1357,7 @@ void AtomicOrderingMMOPredicateMatcher::emitPredicateOpcodes( //===- MemorySizePredicateMatcher -----------------------------------------===// -void MemorySizePredicateMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const { +void MemorySizePredicateMatcher::emitPredicateOpcodes(MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckMemorySizeEqualTo") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("MMO") << MatchTable::ULEB128Value(MMOIdx) @@ -1397,7 +1376,7 @@ bool MemoryAddressSpacePredicateMatcher::isIdentical( } void MemoryAddressSpacePredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { assert(AddrSpaces.size() < 256); Table << MatchTable::Opcode("GIM_CheckMemoryAddressSpace") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) @@ -1423,7 +1402,7 @@ bool MemoryAlignmentPredicateMatcher::isIdentical( } void MemoryAlignmentPredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { // TODO: we could support more, just need to emit the right opcode or switch // to log alignment. assert(MinAlign < 256); @@ -1445,7 +1424,7 @@ bool MemoryVsLLTSizePredicateMatcher::isIdentical( } void MemoryVsLLTSizePredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { Table << MatchTable::Opcode( Relation == EqualTo ? "GIM_CheckMemorySizeEqualToLLT" : Relation == GreaterThan ? "GIM_CheckMemorySizeGreaterThanLLT" @@ -1459,7 +1438,7 @@ void MemoryVsLLTSizePredicateMatcher::emitPredicateOpcodes( //===- VectorSplatImmPredicateMatcher -------------------------------------===// void VectorSplatImmPredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { if (Kind == AllOnes) Table << MatchTable::Opcode("GIM_CheckIsBuildVectorAllOnes"); else @@ -1483,7 +1462,7 @@ bool GenericInstructionPredicateMatcher::isIdentical( static_cast<const GenericInstructionPredicateMatcher &>(B).EnumVal; } void GenericInstructionPredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { Table << MatchTable::Opcode("GIM_CheckCxxInsnPredicate") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::Comment("FnId") << MatchTable::NamedValue(2, EnumVal) @@ -1502,7 +1481,7 @@ bool MIFlagsInstructionPredicateMatcher::isIdentical( } void MIFlagsInstructionPredicateMatcher::emitPredicateOpcodes( - MatchTable &Table, RuleMatcher &Rule) const { + MatchTable &Table) const { Table << MatchTable::Opcode(CheckNot ? "GIM_MIFlagsNot" : "GIM_MIFlags") << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID) << MatchTable::NamedValue(4, join(Flags, " | ")) @@ -1548,28 +1527,27 @@ bool InstructionMatcher::recordsOperand() const { return matchersRecordOperand(Predicates) || matchersRecordOperand(operands()); } -void InstructionMatcher::emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) { +void InstructionMatcher::emitPredicateOpcodes(MatchTable &Table) { if (canAddNumOperandsCheck()) { InstructionNumOperandsMatcher(InsnVarID, getNumOperandMatchers()) - .emitPredicateOpcodes(Table, Rule); + .emitPredicateOpcodes(Table); } // First emit all instruction level predicates need to be verified before we // can verify operands. emitFilteredPredicateListOpcodes( [](const PredicateMatcher &P) { return !P.dependsOnRecordedOperands(); }, - Table, Rule); + Table); // Emit all operand constraints. for (const auto &Operand : Operands) - Operand->emitPredicateOpcodes(Table, Rule); + Operand->emitPredicateOpcodes(Table); // All of the tablegen defined predicates should now be matched. Now emit // a... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/200799 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
