[gicombiner] Add the run-time rule disable option

Summary:
Each generated helper can be configured to generate an option that disables
rules in that helper. This can be used to bisect rulesets.

The disable bits are stored in a SparseVector as this is very cheap for the
common case where nothing is disabled. It gets more expensive the more rules
are disabled but you're generally doing that for debug purposes where
performance is less of a concern.

Depends on D68426

Reviewers: volkan, bogner

Reviewed By: volkan

Subscribers: hiraditya, Petar.Avramovic, llvm-commits

Tags: #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@375067 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Target/GlobalISel/Combine.td b/include/llvm/Target/GlobalISel/Combine.td
index 6f38cca001af43667903b580b0330d604c6684e1..dcac399fd693ff8e4060eea8a86d12c673119f47 100644 (file)
--- a/include/llvm/Target/GlobalISel/Combine.td
+++ b/include/llvm/Target/GlobalISel/Combine.td
@@ -32,6 +32,9 @@ class GICombinerHelper<string classname, list<GICombine> rules>
     : GICombineGroup<rules> {
   // The class name to use in the generated output.
   string Classname = classname;
+  // The name of a run-time compiler option that will be generated to disable
+  // specific rules within this combiner.
+  string DisableRuleOption = ?;
 }
 class GICombineRule<dag defs, dag match, dag apply> : GICombine {
   /// Defines the external interface of the match rule. This includes:

diff --git a/lib/CodeGen/GlobalISel/Combiner.cpp b/lib/CodeGen/GlobalISel/Combiner.cpp
index 644ce2e604bd5ba351ff9ee617f952c215a7ff90..b4562a5c6601631b2b7dca665dc9694359acc2d6 100644 (file)
--- a/lib/CodeGen/GlobalISel/Combiner.cpp
+++ b/lib/CodeGen/GlobalISel/Combiner.cpp
@@ -27,6 +27,18 @@
 
 using namespace llvm;
 
+namespace llvm {
+cl::OptionCategory GICombinerOptionCategory(
+    "GlobalISel Combiner",
+    "Control the rules which are enabled. These options all take a comma "
+    "separated list of rules to disable and may be specified by number "
+    "or number range (e.g. 1-10)."
+#ifndef NDEBUG
+    " They may also be specified by name."
+#endif
+);
+} // end namespace llvm
+
 namespace {
 /// This class acts as the glue the joins the CombinerHelper to the overall
 /// Combine algorithm. The CombinerHelper is intended to report the

diff --git a/lib/Target/AArch64/AArch64Combine.td b/lib/Target/AArch64/AArch64Combine.td
index 89567301cad4a0638ac45b0a2ebf3c1bd1841358..bb99f2516ecf0ef9f8a8c724fc5dfd5e068d757b 100644 (file)
--- a/lib/Target/AArch64/AArch64Combine.td
+++ b/lib/Target/AArch64/AArch64Combine.td
@@ -13,4 +13,6 @@ include "llvm/Target/GlobalISel/Combine.td"
 
 def AArch64PreLegalizerCombinerHelper: GICombinerHelper<
   "AArch64GenPreLegalizerCombinerHelper", [all_combines,
-                                           elide_br_by_inverting_cond]>;
+                                           elide_br_by_inverting_cond]> {
+  let DisableRuleOption = "aarch64prelegalizercombiner-disable-rule";
+}

diff --git a/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp b/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp
index e76db5c12f6f2227ae86cebe8afbe59ea7ec77f8..4e3e8e3589c4dafee82f3065b31898178efd879f 100644 (file)
--- a/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp
+++ b/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp
@@ -47,7 +47,11 @@ public:
                                   GISelKnownBits *KB, MachineDominatorTree *MDT)
       : CombinerInfo(/*AllowIllegalOps*/ true, /*ShouldLegalizeIllegal*/ false,
                      /*LegalizerInfo*/ nullptr, EnableOpt, OptSize, MinSize),
-        KB(KB), MDT(MDT) {}
+        KB(KB), MDT(MDT) {
+    if (!Generated.parseCommandLineOption())
+      report_fatal_error("Invalid rule identifier");
+  }
+
   virtual bool combine(GISelChangeObserver &Observer, MachineInstr &MI,
                        MachineIRBuilder &B) const override;
 };

diff --git a/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir b/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir
new file mode 100644 (file)
index 0000000..bd6756a
--- /dev/null
+++ b/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir
@@ -0,0 +1,35 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - \
+# RUN:                                                                | FileCheck --check-prefix=ENABLED %s
+# RUN: llc -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - \
+# RUN:     --aarch64prelegalizercombinerhelper-disable-rule=copy_prop | FileCheck --check-prefix=DISABLED %s
+
+# REQUIRES: asserts
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64--"
+  define void @test_copy(i8* %addr) {
+  entry:
+    ret void
+  }
+...
+
+---
+name:            test_copy
+body: |
+  bb.0.entry:
+    liveins: $x0
+    ; ENABLED-LABEL: name: test_copy
+    ; ENABLED: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; ENABLED: $x0 = COPY [[COPY]](p0)
+    ; DISABLED-LABEL: name: test_copy
+    ; DISABLED: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; DISABLED: [[COPY1:%[0-9]+]]:_(p0) = COPY [[COPY]](p0)
+    ; DISABLED: [[COPY2:%[0-9]+]]:_(p0) = COPY [[COPY1]](p0)
+    ; DISABLED: $x0 = COPY [[COPY2]](p0)
+    %0:_(p0) = COPY $x0
+    %1:_(p0) = COPY %0
+    %2:_(p0) = COPY %1
+    $x0 = COPY %2
+...

diff --git a/utils/TableGen/GICombinerEmitter.cpp b/utils/TableGen/GICombinerEmitter.cpp
index 676deccaf8238df9bac4cb511b8a390c57e3e025..f2437252a243024b17f1d95acd909ed65f00199c 100644 (file)
--- a/utils/TableGen/GICombinerEmitter.cpp
+++ b/utils/TableGen/GICombinerEmitter.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/StringMatcher.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include "CodeGenTarget.h"
 #include "GlobalISel/CodeExpander.h"
@@ -75,6 +76,8 @@ public:
   bool parseDefs();
   bool parseMatcher(const CodeGenTarget &Target);
 
+  RuleID getID() const { return ID; }
+  StringRef getName() const { return TheDef.getName(); }
   const Record &getDef() const { return TheDef; }
   const CodeInit *getMatchingFixupCode() const { return MatchingFixupCode; }
   size_t getNumRoots() const { return Roots.size(); }
@@ -202,6 +205,9 @@ public:
   }
   void run(raw_ostream &OS);
 
+  /// Emit the name matcher (guarded by #ifndef NDEBUG) used to disable rules in
+  /// response to the generated cl::opt.
+  void emitNameMatcher(raw_ostream &OS) const;
   void generateCodeForRule(raw_ostream &OS, const CombineRule *Rule,
                            StringRef Indent) const;
 };
@@ -211,6 +217,32 @@ GICombinerEmitter::GICombinerEmitter(RecordKeeper &RK,
                                      StringRef Name, Record *Combiner)
     : Name(Name), Target(Target), Combiner(Combiner) {}
 
+void GICombinerEmitter::emitNameMatcher(raw_ostream &OS) const {
+  std::vector<std::pair<std::string, std::string>> Cases;
+  Cases.reserve(Rules.size());
+
+  for (const CombineRule &EnumeratedRule : make_pointee_range(Rules)) {
+    std::string Code;
+    raw_string_ostream SS(Code);
+    SS << "return " << EnumeratedRule.getID() << ";\n";
+    Cases.push_back(std::make_pair(EnumeratedRule.getName(), SS.str()));
+  }
+
+  OS << "#ifndef NDEBUG\n"
+     << "static Optional<uint64_t> getRuleIdxForIdentifier(StringRef "
+        "RuleIdentifier) {\n"
+     << "  uint64_t I;\n"
+     << "  // getAtInteger(...) returns false on success\n"
+     << "  bool Parsed = !RuleIdentifier.getAsInteger(0, I);\n"
+     << "  if (Parsed)\n"
+     << "    return I;\n\n";
+  StringMatcher Matcher("RuleIdentifier", Cases, OS);
+  Matcher.Emit();
+  OS << "  return None;\n"
+     << "}\n"
+     << "#endif // ifndef NDEBUG\n\n";
+}
+
 std::unique_ptr<CombineRule>
 GICombinerEmitter::makeCombineRule(const Record &TheDef) {
   std::unique_ptr<CombineRule> Rule =
@@ -254,7 +286,7 @@ void GICombinerEmitter::generateCodeForRule(raw_ostream &OS,
     const Record &RuleDef = Rule->getDef();
 
     OS << Indent << "// Rule: " << RuleDef.getName() << "\n"
-       << Indent << "{\n";
+       << Indent << "if (!isRuleDisabled(" << Rule->getID() << ")) {\n";
 
     CodeExpansions Expansions;
     for (const RootInfo &Root : Rule->roots()) {
@@ -309,21 +341,83 @@ void GICombinerEmitter::run(raw_ostream &OS) {
                      "Code Generation", "Time spent generating code",
                      TimeRegions);
   OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_DEPS\n"
+     << "#include \"llvm/ADT/SparseBitVector.h\"\n"
+     << "namespace llvm {\n"
+     << "extern cl::OptionCategory GICombinerOptionCategory;\n"
+     << "} // end namespace llvm\n"
      << "#endif // ifdef " << Name.upper() << "_GENCOMBINERHELPER_DEPS\n\n";
 
   OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_H\n"
      << "class " << getClassName() << " {\n"
+     << "  SparseBitVector<> DisabledRules;\n"
+     << "\n"
      << "public:\n"
+     << "  bool parseCommandLineOption();\n"
+     << "  bool isRuleDisabled(unsigned ID) const;\n"
+     << "  bool setRuleDisabled(StringRef RuleIdentifier);\n"
+     << "\n"
      << "  bool tryCombineAll(\n"
      << "    GISelChangeObserver &Observer,\n"
      << "    MachineInstr &MI,\n"
      << "    MachineIRBuilder &B) const;\n"
-     << "};\n";
+     << "};\n\n";
+
+  emitNameMatcher(OS);
+
+  OS << "bool " << getClassName()
+     << "::setRuleDisabled(StringRef RuleIdentifier) {\n"
+     << "  std::pair<StringRef, StringRef> RangePair = "
+        "RuleIdentifier.split('-');\n"
+     << "  if (!RangePair.second.empty()) {\n"
+     << "    const auto First = getRuleIdxForIdentifier(RangePair.first);\n"
+     << "    const auto Last = getRuleIdxForIdentifier(RangePair.second);\n"
+     << "    if (!First.hasValue() || !Last.hasValue())\n"
+     << "      return false;\n"
+     << "    if (First >= Last)\n"
+     << "      report_fatal_error(\"Beginning of range should be before end of "
+        "range\");\n"
+     << "    for (auto I = First.getValue(); I < Last.getValue(); ++I)\n"
+     << "      DisabledRules.set(I);\n"
+     << "    return true;\n"
+     << "  }\n"
+     << "#ifndef NDEBUG\n"
+     << "  else {\n"
+     << "    const auto I = getRuleIdxForIdentifier(RangePair.first);\n"
+     << "    if (!I.hasValue())\n"
+     << "      return false;\n"
+     << "    DisabledRules.set(I.getValue());\n"
+     << "    return true;\n"
+     << "  }\n"
+     << "#else // ifndef NDEBUG\n"
+     << "  llvm_unreachable(\"Cannot disable rules in non-asserts builds\");\n"
+     << "  return false;\n"
+     << "#endif // ifndef NDEBUG\n\n"
+     << "}\n";
+
+  OS << "bool " << getClassName()
+     << "::isRuleDisabled(unsigned RuleID) const {\n"
+     << "  return DisabledRules.test(RuleID);\n"
+     << "}\n";
   OS << "#endif // ifdef " << Name.upper() << "_GENCOMBINERHELPER_H\n\n";
 
   OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_CPP\n"
      << "\n"
-     << "bool " << getClassName() << "::tryCombineAll(\n"
+     << "cl::list<std::string> " << Name << "Option(\n"
+     << "    \"" << Name.lower() << "-disable-rule\",\n"
+     << "    cl::desc(\"Disable one or more combiner rules temporarily in "
+     << "the " << Name << " pass\"),\n"
+     << "    cl::CommaSeparated,\n"
+     << "    cl::Hidden,\n"
+     << "    cl::cat(GICombinerOptionCategory));\n"
+     << "\n"
+     << "bool " << getClassName() << "::parseCommandLineOption() {\n"
+     << "  for (const auto &Identifier : " << Name << "Option)\n"
+     << "    if (!setRuleDisabled(Identifier))\n"
+     << "      return false;\n"
+     << "  return true;\n"
+     << "}\n\n";
+
+  OS << "bool " << getClassName() << "::tryCombineAll(\n"
      << "    GISelChangeObserver &Observer,\n"
      << "    MachineInstr &MI,\n"
      << "    MachineIRBuilder &B) const {\n"