[llvm] 778db88 - [gicombiner] Allow disable-rule option to disable all-except-...

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 16 16:57:30 PDT 2020


Author: Daniel Sanders
Date: 2020-06-16T16:57:16-07:00
New Revision: 778db88723d77e5ec1d55a57d0ea979ead988cf4

URL: https://github.com/llvm/llvm-project/commit/778db88723d77e5ec1d55a57d0ea979ead988cf4
DIFF: https://github.com/llvm/llvm-project/commit/778db88723d77e5ec1d55a57d0ea979ead988cf4.diff

LOG: [gicombiner] Allow disable-rule option to disable all-except-...

Summary:
Adds two features to the generated rule disable option:
- '*' - Disable all rules
- '!<foo>' - Re-enable rule(s)
  - '!foo' - Enable rule named 'foo'
  - '!5' - Enable rule five
  - '!4-9' - Enable rule four to nine
  - '!foo-bar' - Enable rules from 'foo' to (and including) 'bar'
(the '!' is available to the generated disable option but is not part of the underlying and determines whether to call setRuleDisabled() or setRuleEnabled())

This is intended to support unit testing of combine rules so
that you can do:
  GeneratedCfg.setRuleDisabled("*")
  GeneratedCfg.setRuleEnabled("foo")
to ensure only a specific rule is in effect. The rule is still
required to be included in a combiner though

Also added --...-only-enable-rule=X,Y which is effectively an
alias for --...-disable-rule=*,!X,!Y and as such interacts
properly with disable-rule.

Reviewers: aditya_nandakumar, bogner, volkan, aemerson, paquette, arsenm

Subscribers: wdng, llvm-commits

Tags: #llvm

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

Added: 
    

Modified: 
    llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir
    llvm/utils/TableGen/GICombinerEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir
index bd6756aa40a7..5604bc81022f 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir
@@ -3,6 +3,14 @@
 # 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
+# RUN: llc -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - \
+# RUN:     --aarch64prelegalizercombinerhelper-disable-rule="*" | FileCheck --check-prefix=DISABLED %s
+# RUN: llc -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - \
+# RUN:     --aarch64prelegalizercombinerhelper-disable-rule="*,!copy_prop" \
+# RUN:    | FileCheck --check-prefix=ENABLED %s
+# RUN: llc -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - \
+# RUN:     --aarch64prelegalizercombinerhelper-only-enable-rule="copy_prop" \
+# RUN:    | FileCheck --check-prefix=ENABLED %s
 
 # REQUIRES: asserts
 

diff  --git a/llvm/utils/TableGen/GICombinerEmitter.cpp b/llvm/utils/TableGen/GICombinerEmitter.cpp
index 151b18dbe034..e2a670070ae7 100644
--- a/llvm/utils/TableGen/GICombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GICombinerEmitter.cpp
@@ -904,6 +904,7 @@ void GICombinerEmitter::run(raw_ostream &OS) {
      << "public:\n"
      << "  bool parseCommandLineOption();\n"
      << "  bool isRuleDisabled(unsigned ID) const;\n"
+     << "  bool setRuleEnabled(StringRef RuleIdentifier);\n"
      << "  bool setRuleDisabled(StringRef RuleIdentifier);\n"
      << "\n"
      << "};\n"
@@ -932,30 +933,43 @@ void GICombinerEmitter::run(raw_ostream &OS) {
 
   emitNameMatcher(OS);
 
-  OS << "bool " << getClassName()
-     << "RuleConfig::setRuleDisabled(StringRef RuleIdentifier) {\n"
+  OS << "static Optional<std::pair<uint64_t, uint64_t>> "
+        "getRuleRangeForIdentifier(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"
+     << "    const auto First = "
+        "getRuleIdxForIdentifier(RangePair.first);\n"
+     << "    const auto Last = "
+        "getRuleIdxForIdentifier(RangePair.second);\n"
      << "    if (!First.hasValue() || !Last.hasValue())\n"
-     << "      return false;\n"
+     << "      return None;\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"
+     << "      report_fatal_error(\"Beginning of range should be before "
+        "end of range\");\n"
+     << "    return {{ *First, *Last + 1 }};\n"
+     << "  } else if (RangePair.first == \"*\") {\n"
+     << "    return {{ 0, " << Rules.size() << " }};\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"
+     << "      return None;\n"
+     << "    return {{*I, *I + 1}};\n"
      << "  }\n"
-     << "  return false;\n"
-     << "}\n";
+     << "  return None;\n"
+     << "}\n\n";
+
+  for (bool Enabled : {true, false}) {
+    OS << "bool " << getClassName() << "RuleConfig::setRule"
+       << (Enabled ? "Enabled" : "Disabled") << "(StringRef RuleIdentifier) {\n"
+       << "  auto MaybeRange = getRuleRangeForIdentifier(RuleIdentifier);\n"
+       << "  if(!MaybeRange.hasValue())\n"
+       << "    return false;\n"
+       << "  for (auto I = MaybeRange->first; I < MaybeRange->second; ++I)\n"
+       << "    DisabledRules." << (Enabled ? "reset" : "set") << "(I);\n"
+       << "  return true;\n"
+       << "}\n\n";
+  }
 
   OS << "bool " << getClassName()
      << "RuleConfig::isRuleDisabled(unsigned RuleID) const {\n"
@@ -965,18 +979,41 @@ void GICombinerEmitter::run(raw_ostream &OS) {
 
   OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_CPP\n"
      << "\n"
-     << "cl::list<std::string> " << Name << "Option(\n"
+     << "std::vector<std::string> " << Name << "Option;\n"
+     << "cl::list<std::string> " << Name << "DisableOption(\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"
+     << "    cl::cat(GICombinerOptionCategory),\n"
+     << "    cl::callback([](const std::string &Str) {\n"
+     << "      " << Name << "Option.push_back(Str);\n"
+     << "    }));\n"
+     << "cl::list<std::string> " << Name << "OnlyEnableOption(\n"
+     << "    \"" << Name.lower() << "-only-enable-rule\",\n"
+     << "    cl::desc(\"Disable all rules in the " << Name
+     << " pass then re-enable the specified ones\"),\n"
+     << "    cl::Hidden,\n"
+     << "    cl::cat(GICombinerOptionCategory),\n"
+     << "    cl::callback([](const std::string &CommaSeparatedArg) {\n"
+     << "      StringRef Str = CommaSeparatedArg;\n"
+     << "      " << Name << "Option.push_back(\"*\");\n"
+     << "      do {\n"
+     << "        auto X = Str.split(\",\");\n"
+     << "        " << Name << "Option.push_back((\"!\" + X.first).str());\n"
+     << "        Str = X.second;\n"
+     << "      } while (!Str.empty());\n"
+     << "    }));\n"
      << "\n"
      << "bool " << getClassName() << "RuleConfig::parseCommandLineOption() {\n"
-     << "  for (const auto &Identifier : " << Name << "Option)\n"
-     << "    if (!setRuleDisabled(Identifier))\n"
+     << "  for (StringRef Identifier : " << Name << "Option) {\n"
+     << "    bool Enabled = Identifier.consume_front(\"!\");\n"
+     << "    if (Enabled && !setRuleEnabled(Identifier))\n"
      << "      return false;\n"
+     << "    if (!Enabled && !setRuleDisabled(Identifier))\n"
+     << "      return false;\n"
+     << "  }\n"
      << "  return true;\n"
      << "}\n\n";
 


        


More information about the llvm-commits mailing list