[GlobalISel][InstructionSelect] Refactoring out a getMatchTable virtual method, NFC

[rtereshin]

Summary

to share it between the Instruction Selector and the Testgen both.

Originally part of the larger https://reviews.llvm.org/D43962 ([GlobalISel][utils] Adding the init version of Instruction Select Testgen) patch, extracted as to make it smaller and more targeted.

Partially reviewed by @dsanders within the patch mentioned.

Required by https://reviews.llvm.org/D43962

[dsanders]

The NFC changes look good to me but the commit message (particularly the subject line) doesn't really match the content. LGTM with a better commit message.

[rtereshin]

Thanks

I'm thinking on the following message:

[GlobalISel][InstructionSelect] Refactoring out a getMatchTable virtual method + NFC

The main goal is to share getMatchTable between the Instruction
Selector and the Testgen.

The commit also contains some NFC only loosely related to refactoring
out the getMatchTable, but strongly related to the initial Testgen
patch (see https://reviews.llvm.org/D43962)

Reviewers: dsanders, aemerson

Reviewed By: dsanders

Subscribers: rovka, kristof.beyls, llvm-commits, dsanders

Differential Revision: https://reviews.llvm.org/D46096

[dsanders]

Thanks. I'd suggest making it a little more obvious to someone skim-reading the list, for example:

[GlobalISel][InstructionSelect] Refactoring out a getMatchTable virtual method + other small NFC's

but LGTM either way.

llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h

enum {
   GIR_AddImplicitUse,
   /// Add an register to the specified instruction
   /// - InsnID - Instruction ID to modify
   /// - RegNum - The register to add
   GIR_AddRegister,
   /// Add a temporary register to the specified instruction
   /// - InsnID - Instruction ID to modify
   /// - TempRegID - The temporary register ID to add
+  /// - TempRegFlags - The register flags to set
   GIR_AddTempRegister,
   /// Add an immediate to the specified instruction
   /// - InsnID - Instruction ID to modify
   /// - Imm - The immediate to add
   GIR_AddImm,
   /// Render complex operands to the specified instruction
   /// - InsnID - Instruction ID to modify
   /// - RendererID - The renderer to call
enum {
   GIR_MakeTempReg,
 
   /// A successful emission
   GIR_Done,
 
   /// Increment the rule coverage counter.
   /// - RuleID - The ID of the rule that was covered.
   GIR_Coverage,
+
+  GIU_NumOpcodes,
 };
 
 enum {
   /// Indicates the end of the variable-length MergeInsnID list in a
   /// GIR_MergeMemOperands opcode.
   GIU_MergeMemOperands_EndOfList = -1,
 };
 
bool executeMatchTable(
       TgtInstructionSelector &ISel, NewMIVector &OutMIs, MatcherState &State,
       const ISelInfoTy<PredicateBitset, ComplexMatcherMemFn, CustomRendererFn>
           &ISelInfo,
       const int64_t *MatchTable, const TargetInstrInfo &TII,
       MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
       const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
       CodeGenCoverage &CoverageInfo) const;
 
+  virtual const int64_t *getMatchTable() const {
+    llvm_unreachable("Should have been overridden by tablegen if used");
+  }
+
   virtual bool testImmPredicate_I64(unsigned, int64_t) const {
     llvm_unreachable("Subclasses must override this to use tablegen");
   }
   virtual bool testImmPredicate_APInt(unsigned, const APInt &) const {
     llvm_unreachable("Subclasses must override this to use tablegen");
   }
   virtual bool testImmPredicate_APFloat(unsigned, const APFloat &) const {
     llvm_unreachable("Subclasses must override this to use tablegen");

llvm/trunk/test/TableGen/GlobalISelEmitter.td

 // CHECK-NEXT:    typedef ComplexRendererFns(MyTargetInstructionSelector::*ComplexMatcherMemFn)(MachineOperand &) const;
 // CHECK-NEXT:    typedef void(MyTargetInstructionSelector::*CustomRendererFn)(MachineInstrBuilder &, const MachineInstr&) const;
 // CHECK-NEXT:    const ISelInfoTy<PredicateBitset, ComplexMatcherMemFn, CustomRendererFn> ISelInfo;
 // CHECK-NEXT:    static MyTargetInstructionSelector::ComplexMatcherMemFn ComplexPredicateFns[];
 // CHECK-NEXT:    static MyTargetInstructionSelector::CustomRendererFn CustomRenderers[];
 // CHECK-NEXT:    bool testImmPredicate_I64(unsigned PredicateID, int64_t Imm) const override;
 // CHECK-NEXT:    bool testImmPredicate_APInt(unsigned PredicateID, const APInt &Imm) const override;
 // CHECK-NEXT:    bool testImmPredicate_APFloat(unsigned PredicateID, const APFloat &Imm) const override;
+// CHECK-NEXT:    const int64_t *getMatchTable() const override;
 // CHECK-NEXT:  #endif // ifdef GET_GLOBALISEL_TEMPORARIES_DECL
 
 // CHECK-LABEL: #ifdef GET_GLOBALISEL_TEMPORARIES_INIT
 // CHECK-NEXT:    , State(2),
 // CHECK-NEXT:    ISelInfo({TypeObjects, FeatureBitsets, ComplexPredicateFns, CustomRenderers})
 // CHECK-NEXT:  #endif // ifdef GET_GLOBALISEL_TEMPORARIES_INIT
 
 // CHECK-LABEL: enum SubtargetFeatureBits : uint8_t {

 // CHECK-NEXT: MachineFunction &MF = *I.getParent()->getParent();
 // CHECK-NEXT: MachineRegisterInfo &MRI = MF.getRegInfo();
 // CHECK:      AvailableFunctionFeatures = computeAvailableFunctionFeatures(&STI, &MF);
 // CHECK-NEXT: const PredicateBitset AvailableFeatures = getAvailableFeatures();
 // CHECK-NEXT: NewMIVector OutMIs;
 // CHECK-NEXT: State.MIs.clear();
 // CHECK-NEXT: State.MIs.push_back(&I);
 
+// CHECK:      if (executeMatchTable(*this, OutMIs, State, ISelInfo, getMatchTable(), TII, MRI, TRI, RBI, AvailableFeatures, CoverageInfo)) {
+// CHECK-NEXT:   return true;
+// CHECK-NEXT: }
+
 //===- Test a pattern with multiple ComplexPatterns in multiple instrs ----===//
 //
 
-// CHECK-LABEL: MatchTable0[] = {
+// CHECK: const int64_t *
+// CHECK-LABEL: MyTargetInstructionSelector::getMatchTable() const {
+// CHECK:   MatchTable0[] = {
 // OPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[GRP_LABEL_NUM:[0-9]+]]*/ [[GRP_LABEL:[0-9]+]],
 // OPT-NEXT:    GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_SELECT,
 // CHECK-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ [[LABEL:[0-9]+]],
 // CHECK-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/4,
 // CHECK-NEXT:    GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/3, // MIs[1]
 // CHECK-NEXT:    GIM_CheckNumOperands, /*MI*/1, /*Expected*/4,
 // NOOPT-NEXT:    GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_SELECT,
 // OPT-NEXT:      // No instruction predicates

 // OPT-NEXT:  GIR_Done,
 // OPT-NEXT:  // Label [[GRP_LABEL_NUM]]: @[[GRP_LABEL]]
 
 def BR : I<(outs), (ins unknown:$target),
             [(br bb:$target)]>;
 
 // CHECK-NEXT:    GIM_Reject,
 // CHECK-NEXT:  };
-// CHECK-NEXT:  if (executeMatchTable(*this, OutMIs, State, ISelInfo, MatchTable0, TII, MRI, TRI, RBI, AvailableFeatures, CoverageInfo)) {
-// CHECK-NEXT:    return true;
-// CHECK-NEXT:  }
+// CHECK-NEXT:  return MatchTable0;

llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp

<< "InstructionSelector::*CustomRendererFn)(MachineInstrBuilder &, const "
         "const;\n"
      << "  const ISelInfoTy<PredicateBitset, ComplexMatcherMemFn, "
         "CustomRendererFn> "
         "ISelInfo;\n";
   OS << "  static " << Target.getName()
      << "InstructionSelector::ComplexMatcherMemFn ComplexPredicateFns[];\n"
      << "  static " << Target.getName()
      << "InstructionSelector::CustomRendererFn CustomRenderers[];\n"
-     << "bool testImmPredicate_I64(unsigned PredicateID, int64_t Imm) const "
+     << "  bool testImmPredicate_I64(unsigned PredicateID, int64_t Imm) const "
         "override;\n"
-     << "bool testImmPredicate_APInt(unsigned PredicateID, const APInt &Imm) "
+     << "  bool testImmPredicate_APInt(unsigned PredicateID, const APInt &Imm) "
         "const override;\n"
-     << "bool testImmPredicate_APFloat(unsigned PredicateID, const APFloat "
+     << "  bool testImmPredicate_APFloat(unsigned PredicateID, const APFloat "
         "&Imm) const override;\n"
+     << "  const int64_t *getMatchTable() const override;\n"
      << "#endif // ifdef GET_GLOBALISEL_TEMPORARIES_DECL\n\n";
 
   OS << "#ifdef GET_GLOBALISEL_TEMPORARIES_INIT\n"
      << ", State(" << MaxTemporaries << "),\n"
      << "ISelInfo({TypeObjects, FeatureBitsets, ComplexPredicateFns, "
         "CustomRenderers})\n"
      << "#endif // ifdef GET_GLOBALISEL_TEMPORARIES_INIT\n\n";
 
OS << Target.getName() << "InstructionSelector::CustomRendererFn\n"
      << Target.getName() << "InstructionSelector::CustomRenderers[] = {\n"
      << "  nullptr, // GICP_Invalid\n";
   for (const auto &Record : CustomRendererFns)
     OS << "  &" << Target.getName()
        << "InstructionSelector::" << Record->getValueAsString("RendererFn")
        << ", // " << Record->getName() << "\n";
   OS << "};\n\n";
 
-  OS << "bool " << Target.getName()
-     << "InstructionSelector::selectImpl(MachineInstr &I, CodeGenCoverage "
-        "&CoverageInfo) const {\n"
-     << "  MachineFunction &MF = *I.getParent()->getParent();\n"
-     << "  MachineRegisterInfo &MRI = MF.getRegInfo();\n"
-     << "  // FIXME: This should be computed on a per-function basis rather "
-        "than per-insn.\n"
-     << "  AvailableFunctionFeatures = computeAvailableFunctionFeatures(&STI, "
-        "&MF);\n"
-     << "  const PredicateBitset AvailableFeatures = getAvailableFeatures();\n"
-     << "  NewMIVector OutMIs;\n"
-     << "  State.MIs.clear();\n"
-     << "  State.MIs.push_back(&I);\n\n";
-
   std::stable_sort(Rules.begin(), Rules.end(), [&](const RuleMatcher &A,
                                                    const RuleMatcher &B) {
     int ScoreA = RuleMatcherScores[A.getRuleID()];
     int ScoreB = RuleMatcherScores[B.getRuleID()];
     if (ScoreA > ScoreB)
       return true;
     if (ScoreB > ScoreA)
       return false;
std::vector<Matcher *> OptRules =
       OptimizeMatchTable ? optimizeRules(InputRules, StorageGroupMatcher)
                          : InputRules;
 
   MatchTable Table(0);
   for (Matcher *Rule : OptRules)
     Rule->emit(Table);
 
   Table << MatchTable::Opcode("GIM_Reject") << MatchTable::LineBreak;
-  Table.emitDeclaration(OS);
-  OS << "  if (executeMatchTable(*this, OutMIs, State, ISelInfo, ";
-  Table.emitUse(OS);
-  OS << ", TII, MRI, TRI, RBI, AvailableFeatures, CoverageInfo)) {\n"
+
+  OS << "bool " << Target.getName()
+     << "InstructionSelector::selectImpl(MachineInstr &I, CodeGenCoverage "
+        "&CoverageInfo) const {\n"
+     << "  MachineFunction &MF = *I.getParent()->getParent();\n"
+     << "  MachineRegisterInfo &MRI = MF.getRegInfo();\n"
+     << "  // FIXME: This should be computed on a per-function basis rather "
+        "than per-insn.\n"
+     << "  AvailableFunctionFeatures = computeAvailableFunctionFeatures(&STI, "
+        "&MF);\n"
+     << "  const PredicateBitset AvailableFeatures = getAvailableFeatures();\n"
+     << "  NewMIVector OutMIs;\n"
+     << "  State.MIs.clear();\n"
+     << "  State.MIs.push_back(&I);\n\n"
+     << "  if (executeMatchTable(*this, OutMIs, State, ISelInfo"
+     << ", getMatchTable(), TII, MRI, TRI, RBI, AvailableFeatures"
+     << ", CoverageInfo)) {\n"
      << "    return true;\n"
-     << "  }\n\n";
+     << "  }\n\n"
+     << "  return false;\n"
+     << "}\n\n";
 
-  OS << "  return false;\n"
-     << "}\n"
-     << "#endif // ifdef GET_GLOBALISEL_IMPL\n";
+  OS << "const int64_t *" << Target.getName()
+     << "InstructionSelector::getMatchTable() const {\n";
+  Table.emitDeclaration(OS);
+  OS << "  return ";
+  Table.emitUse(OS);
+  OS << ";\n}\n";
+  OS << "#endif // ifdef GET_GLOBALISEL_IMPL\n";
 
   OS << "#ifdef GET_GLOBALISEL_PREDICATES_DECL\n"
      << "PredicateBitset AvailableModuleFeatures;\n"
      << "mutable PredicateBitset AvailableFunctionFeatures;\n"
      << "PredicateBitset getAvailableFeatures() const {\n"
      << "  return AvailableModuleFeatures | AvailableFunctionFeatures;\n"
      << "}\n"
      << "PredicateBitset\n"