[llvm] r333017 - [GlobalISel][InstructionSelect] Switching MatchTable over opcodes, perf patch 4

Roman Tereshin via llvm-commits llvm-commits at lists.llvm.org
Tue May 22 12:37:59 PDT 2018


Author: rtereshin
Date: Tue May 22 12:37:59 2018
New Revision: 333017

URL: http://llvm.org/viewvc/llvm-project?rev=333017&view=rev
Log:
[GlobalISel][InstructionSelect] Switching MatchTable over opcodes, perf patch 4

This patch continues a series of patches started by r332907 (reapplied
as r332917)

In this commit we introduce a new matching opcode GIM_SwitchOpcode
that implements a jump table over opcodes and start emitting them for
root instructions.

This is expected to decrease time GlobalISel spends in its
InstructionSelect pass by roughly 20% for an -O0 build as measured on
sqlite3-amalgamation (http://sqlite.org/download.html) targeting
AArch64.

To some degree, we assume here that the opcodes form a dense set,
which is true at the moment for all upstream targets given the
limitations of our rule importing mechanism.

It might not be true for out of tree targets, specifically due to
pseudo's. If so, we might noticeably increase the size of the
MatchTable with this patch due to padding zeros. This will be
addressed later.

Reviewers: qcolombet, dsanders, bogner, aemerson, javed.absar

Reviewed By: qcolombet

Subscribers: rovka, llvm-commits, kristof.beyls

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

Modified:
    llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
    llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
    llvm/trunk/test/TableGen/GlobalISelEmitter.td
    llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h?rev=333017&r1=333016&r2=333017&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h Tue May 22 12:37:59 2018
@@ -81,6 +81,14 @@ enum {
   ///        failed match.
   GIM_Try,
 
+  /// Switch over the opcode on the specified instruction
+  /// - InsnID - Instruction ID
+  /// - LowerBound - numerically minimum opcode supported
+  /// - UpperBound - numerically maximum + 1 opcode supported
+  /// - Default - failure jump target
+  /// - JumpTable... - (UpperBound - LowerBound) (at least 2) jump targets
+  GIM_SwitchOpcode,
+
   /// Record the specified instruction
   /// - NewInsnID - Instruction ID to define
   /// - InsnID - Instruction ID

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h?rev=333017&r1=333016&r2=333017&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h Tue May 22 12:37:59 2018
@@ -153,6 +153,33 @@ bool InstructionSelector::executeMatchTa
       break;
     }
 
+    case GIM_SwitchOpcode: {
+      int64_t InsnID = MatchTable[CurrentIdx++];
+      int64_t LowerBound = MatchTable[CurrentIdx++];
+      int64_t UpperBound = MatchTable[CurrentIdx++];
+      int64_t Default = MatchTable[CurrentIdx++];
+
+      assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
+      const int64_t Opcode = State.MIs[InsnID]->getOpcode();
+
+      DEBUG_WITH_TYPE(TgtInstructionSelector::getName(), {
+        dbgs() << CurrentIdx << ": GIM_SwitchOpcode(MIs[" << InsnID << "], ["
+               << LowerBound << ", " << UpperBound << "), Default=" << Default
+               << ", JumpTable...) // Got=" << Opcode << "\n";
+      });
+      if (Opcode < LowerBound || UpperBound <= Opcode) {
+        CurrentIdx = Default;
+        break;
+      }
+      CurrentIdx = MatchTable[CurrentIdx + (Opcode - LowerBound)];
+      if (!CurrentIdx) {
+        CurrentIdx = Default;
+	break;
+      }
+      OnFailResumeAt.push_back(Default);
+      break;
+    }
+
     case GIM_CheckNumOperands: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t Expected = MatchTable[CurrentIdx++];

Modified: llvm/trunk/test/TableGen/GlobalISelEmitter.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/GlobalISelEmitter.td?rev=333017&r1=333016&r2=333017&view=diff
==============================================================================
--- llvm/trunk/test/TableGen/GlobalISelEmitter.td (original)
+++ llvm/trunk/test/TableGen/GlobalISelEmitter.td Tue May 22 12:37:59 2018
@@ -239,8 +239,11 @@ def HasC : Predicate<"Subtarget->hasC()"
 
 //===- Test a pattern with multiple ComplexPatterns in multiple instrs ----===//
 //
-// R19O-NEXT:  GIM_Try, /*On fail goto*//*Label [[GROUP_NUM:[0-9]+]]*/ [[GROUP:[0-9]+]],
-// R19O-NEXT:    GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_SELECT,
+// R19O-NEXT:  GIM_SwitchOpcode, /*MI*/0, /*[*/{{[0-9]+}}, {{[0-9]+}}, /*)*//*default:*//*Label [[DEFAULT_NUM:[0-9]+]]*/ [[DEFAULT:[0-9]+]],
+// R19O-NEXT:  /*TargetOpcode::G_ADD*//*Label [[CASE_ADD_NUM:[0-9]+]]*/ [[CASE_ADD:[0-9]+]],
+// R19O:       /*TargetOpcode::G_SELECT*//*Label [[CASE_SELECT_NUM:[0-9]+]]*/ [[CASE_SELECT:[0-9]+]],
+// R19O:       // Label [[CASE_ADD_NUM]]: @[[CASE_ADD]]
+// R19O:       // Label [[CASE_SELECT_NUM]]: @[[CASE_SELECT]]
 //
 // R19C-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ [[LABEL:[0-9]+]],
 // R19N-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/4,
@@ -298,8 +301,9 @@ def HasC : Predicate<"Subtarget->hasC()"
 // R19C-NEXT:    GIR_Done,
 // R19C-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 //
-// R19O:         GIM_Reject,
-// R19O-NEXT:  // Label [[GROUP_NUM]]: @[[GROUP]]
+// R19O:       // Label [[DEFAULT_NUM]]: @[[DEFAULT]]
+// R19O-NEXT:  GIM_Reject,
+// R19O-NEXT:  };
 
 def INSN3 : I<(outs GPR32:$dst),
               (ins GPR32Op:$src1, GPR32:$src2a, GPR32:$src2b, GPR32:$scr), []>;
@@ -313,8 +317,11 @@ def : Pat<(select GPR32:$src1, (complex_
                  (INSN4 GPR32:$src3, complex:$src4, i32imm:$src5a,
                         i32imm:$src5b))>;
 
-// R21O-NEXT:  GIM_Try, /*On fail goto*//*Label [[GROUP_NUM:[0-9]+]]*/ [[GROUP:[0-9]+]],
-// R21O-NEXT:    GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_SELECT,
+// R21O-NEXT:  GIM_SwitchOpcode, /*MI*/0, /*[*/{{[0-9]+}}, {{[0-9]+}}, /*)*//*default:*//*Label [[DEFAULT_NUM:[0-9]+]]*/ [[DEFAULT:[0-9]+]],
+// R21O-NEXT:  /*TargetOpcode::G_ADD*//*Label [[CASE_ADD_NUM:[0-9]+]]*/ [[CASE_ADD:[0-9]+]],
+// R21O:       /*TargetOpcode::G_SELECT*//*Label [[CASE_SELECT_NUM:[0-9]+]]*/ [[CASE_SELECT:[0-9]+]],
+// R21O:       // Label [[CASE_ADD_NUM]]: @[[CASE_ADD]]
+// R21O:       // Label [[CASE_SELECT_NUM]]: @[[CASE_SELECT]]
 //
 // R21C-NEXT:  GIM_Try, /*On fail goto*//*Label [[PREV_NUM:[0-9]+]]*/ [[PREV:[0-9]+]], // Rule ID 19 //
 // R21C-NOT:     GIR_Done,
@@ -353,8 +360,10 @@ def : Pat<(select GPR32:$src1, (complex_
 // R21C-NEXT:    GIR_Done,
 // R21C-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 //
-// R21O-NEXT:    GIM_Reject,
-// R21O-NEXT:  // Label [[GROUP_NUM]]: @[[GROUP]]
+// R21O-NEXT:  GIM_Reject,
+// R21O:       // Label [[DEFAULT_NUM]]: @[[DEFAULT]]
+// R21O-NEXT:  GIM_Reject,
+// R21O-NEXT:  };
 
 //===- Test a pattern with ComplexPattern operands. -----------------------===//
 //

Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=333017&r1=333016&r2=333017&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
+++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Tue May 22 12:37:59 2018
@@ -705,6 +705,60 @@ private:
   bool candidateConditionMatches(const PredicateMatcher &Predicate) const;
 };
 
+class SwitchMatcher : public Matcher {
+  /// All the nested matchers, representing distinct switch-cases. The first
+  /// conditions (as Matcher::getFirstCondition() reports) of all the nested
+  /// matchers must share the same type and path to a value they check, in other
+  /// words, be isIdenticalDownToValue, but have different values they check
+  /// against.
+  std::vector<Matcher *> Matchers;
+
+  /// The representative condition, with a type and a path (InsnVarID and OpIdx
+  /// in most cases)  shared by all the matchers contained.
+  std::unique_ptr<PredicateMatcher> Condition = nullptr;
+
+  /// Temporary set used to check that the case values don't repeat within the
+  /// same switch.
+  std::set<MatchTableRecord> Values;
+
+  /// An owning collection for any auxiliary matchers created while optimizing
+  /// nested matchers contained.
+  std::vector<std::unique_ptr<Matcher>> MatcherStorage;
+
+public:
+  bool addMatcher(Matcher &Candidate);
+
+  void finalize();
+  void emit(MatchTable &Table) override;
+
+  iterator_range<std::vector<Matcher *>::iterator> matchers() {
+    return make_range(Matchers.begin(), Matchers.end());
+  }
+  size_t size() const { return Matchers.size(); }
+  bool empty() const { return Matchers.empty(); }
+
+  std::unique_ptr<PredicateMatcher> popFirstCondition() override {
+    // SwitchMatcher doesn't have a common first condition for its cases, as all
+    // the cases only share a kind of a value (a type and a path to it) they
+    // match, but deliberately differ in the actual value they match.
+    llvm_unreachable("Trying to pop a condition from a condition-less group");
+  }
+  const PredicateMatcher &getFirstCondition() const override {
+    llvm_unreachable("Trying to pop a condition from a condition-less group");
+  }
+  bool hasFirstCondition() const override { return false; }
+
+private:
+  /// See if the predicate type has a Switch-implementation for it.
+  static bool isSupportedPredicateType(const PredicateMatcher &Predicate);
+
+  bool candidateConditionMatches(const PredicateMatcher &Predicate) const;
+
+  /// emit()-helper
+  static void emitPredicateSpecificOpcodes(const PredicateMatcher &P,
+                                           MatchTable &Table);
+};
+
 /// Generates code to check that a match rule matches.
 class RuleMatcher : public Matcher {
 public:
@@ -4045,6 +4099,8 @@ GlobalISelEmitter::buildMatchTable(Mutab
   for (Matcher *Rule : OptRules)
     Rule->optimize();
 
+  OptRules = optimizeRules<SwitchMatcher>(OptRules, MatcherStorage);
+
   return MatchTable::buildTable(OptRules, WithCoverage);
 }
 
@@ -4526,6 +4582,128 @@ void GroupMatcher::emit(MatchTable &Tabl
           << MatchTable::Label(LabelID);
 }
 
+bool SwitchMatcher::isSupportedPredicateType(const PredicateMatcher &P) {
+  return isa<InstructionOpcodeMatcher>(P);
+}
+
+bool SwitchMatcher::candidateConditionMatches(
+    const PredicateMatcher &Predicate) const {
+
+  if (empty()) {
+    // Sharing predicates for nested instructions is not supported yet as we
+    // currently don't hoist the GIM_RecordInsn's properly, therefore we can
+    // only work on the original root instruction (InsnVarID == 0):
+    if (Predicate.getInsnVarID() != 0)
+      return false;
+    // ... while an attempt to add even a root matcher to an empty SwitchMatcher
+    // could fail as not all the types of conditions are supported:
+    if (!isSupportedPredicateType(Predicate))
+      return false;
+    // ... or the condition might not have a proper implementation of
+    // getValue() / isIdenticalDownToValue() yet:
+    if (!Predicate.hasValue())
+      return false;
+    // ... otherwise an empty Switch can accomodate the condition with no
+    // further requirements:
+    return true;
+  }
+
+  const Matcher &CaseRepresentative = **Matchers.begin();
+  const auto &RepresentativeCondition = CaseRepresentative.getFirstCondition();
+  // Switch-cases must share the same kind of condition and path to the value it
+  // checks:
+  if (!Predicate.isIdenticalDownToValue(RepresentativeCondition))
+    return false;
+
+  const auto Value = Predicate.getValue();
+  // ... but be unique with respect to the actual value they check:
+  return Values.count(Value) == 0;
+}
+
+bool SwitchMatcher::addMatcher(Matcher &Candidate) {
+  if (!Candidate.hasFirstCondition())
+    return false;
+
+  const PredicateMatcher &Predicate = Candidate.getFirstCondition();
+  if (!candidateConditionMatches(Predicate))
+    return false;
+  const auto Value = Predicate.getValue();
+  Values.insert(Value);
+
+  Matchers.push_back(&Candidate);
+  return true;
+}
+
+void SwitchMatcher::finalize() {
+  assert(Condition == nullptr && "Already finalized");
+  assert(Values.size() == Matchers.size() && "Broken SwitchMatcher");
+  if (empty())
+    return;
+
+  std::stable_sort(Matchers.begin(), Matchers.end(),
+                   [](const Matcher *L, const Matcher *R) {
+                     return L->getFirstCondition().getValue() <
+                            R->getFirstCondition().getValue();
+                   });
+  Condition = Matchers[0]->popFirstCondition();
+  for (unsigned I = 1, E = Values.size(); I < E; ++I)
+    Matchers[I]->popFirstCondition();
+}
+
+void SwitchMatcher::emitPredicateSpecificOpcodes(const PredicateMatcher &P,
+                                                 MatchTable &Table) {
+  assert(isSupportedPredicateType(P) && "Predicate type is not supported");
+
+  if (const auto *Condition = dyn_cast<InstructionOpcodeMatcher>(&P)) {
+    Table << MatchTable::Opcode("GIM_SwitchOpcode") << MatchTable::Comment("MI")
+          << MatchTable::IntValue(Condition->getInsnVarID());
+    return;
+  }
+
+  llvm_unreachable("emitPredicateSpecificOpcodes is broken: can not handle a "
+                   "predicate type that is claimed to be supported");
+}
+
+void SwitchMatcher::emit(MatchTable &Table) {
+  assert(Values.size() == Matchers.size() && "Broken SwitchMatcher");
+  if (empty())
+    return;
+  assert(Condition != nullptr &&
+         "Broken SwitchMatcher, hasn't been finalized?");
+
+  std::vector<unsigned> LabelIDs(Values.size());
+  std::generate(LabelIDs.begin(), LabelIDs.end(),
+                [&Table]() { return Table.allocateLabelID(); });
+  const unsigned Default = Table.allocateLabelID();
+
+  const int64_t LowerBound = Values.begin()->getRawValue();
+  const int64_t UpperBound = Values.rbegin()->getRawValue() + 1;
+
+  emitPredicateSpecificOpcodes(*Condition, Table);
+
+  Table << MatchTable::Comment("[") << MatchTable::IntValue(LowerBound)
+        << MatchTable::IntValue(UpperBound) << MatchTable::Comment(")")
+        << MatchTable::Comment("default:") << MatchTable::JumpTarget(Default);
+
+  int64_t J = LowerBound;
+  auto VI = Values.begin();
+  for (unsigned I = 0, E = Values.size(); I < E; ++I) {
+    auto V = *VI++;
+    while (J++ < V.getRawValue())
+      Table << MatchTable::IntValue(0);
+    V.turnIntoComment();
+    Table << MatchTable::LineBreak << V << MatchTable::JumpTarget(LabelIDs[I]);
+  }
+  Table << MatchTable::LineBreak;
+
+  for (unsigned I = 0, E = Values.size(); I < E; ++I) {
+    Table << MatchTable::Label(LabelIDs[I]);
+    Matchers[I]->emit(Table);
+    Table << MatchTable::Opcode("GIM_Reject") << MatchTable::LineBreak;
+  }
+  Table << MatchTable::Label(Default);
+}
+
 unsigned OperandMatcher::getInsnVarID() const { return Insn.getInsnVarID(); }
 
 } // end anonymous namespace




More information about the llvm-commits mailing list