[gicombiner] Add the run-time rule disable option

[dsanders]

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

[volkan]

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir

2

No need to pass -O0 and -global-isel as it runs a specific pass.

5

Could you add a test case that disables the rule using the ID?

llvm/utils/TableGen/GICombinerEmitter.cpp

400

Looks like it's not possible to disable a rule in Release mode. I thought it'd support disabling by rule id, could you double check?

[dsanders]

• Fix up nits

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir

5

It's not very practical to do so. The ID is stable for a given revision of the compiler but isn't guaranteed to be stable as more rules are added.

llvm/utils/TableGen/GICombinerEmitter.cpp

400

Oops. That's from before I changed my mind about supporting it in Release mode. Fixed it

[dsanders]

• setRuleDisabled wasn't quite right w.r.t disabling by id either

[volkan]

LGTM.

[sdmitriev]

llvm/utils/TableGen/GICombinerEmitter.cpp

371

getRuleIdxForIdentifier() is defined under #ifndef NDEBUG, but called without #ifndef NDEBUG. I assume this will cause an error if NDEBUG macro is defined.

[dsanders]

llvm/utils/TableGen/GICombinerEmitter.cpp

371

Yes, you're right. The sanitizer-x86_64-linux-fuzzer bot complained at me about that and I fixed it in r375067

Thanks

llvm/include/llvm/Target/GlobalISel/Combine.td

class GICombineGroup<list<GICombine> rules> : GICombine {
   let Rules = rules;
 }
 
 // Declares a combiner helper class
 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:
   /// * The names of the root nodes (requires at least one)
   /// See GIDefKind for details.
   dag Defs = defs;
 
   /// Defines the things which must be true for the pattern to match

llvm/lib/CodeGen/GlobalISel/Combiner.cpp

 #include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "gi-combiner"
 
 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
 /// modifications it makes to the MIR to the GISelChangeObserver and the
 /// observer subclass will act on these events. In this case, instruction
 /// erasure will cancel any future visits to the erased instruction and
 /// instruction creation will schedule that instruction for a future visit.
 /// Other Combiner implementations may require more complex behaviour from

llvm/lib/Target/AArch64/AArch64Combine.td

 //=- AArch64.td - Define AArch64 Combine Rules ---------------*- tablegen -*-=//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //
 //===----------------------------------------------------------------------===//
 
 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";
+}

llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp

 
 public:
   AArch64GenPreLegalizerCombinerHelper Generated;
 
   AArch64PreLegalizerCombinerInfo(bool EnableOpt, bool OptSize, bool MinSize,
                                   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;
 };
 
 bool AArch64PreLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
                                               MachineInstr &MI,
                                               MachineIRBuilder &B) const {
   CombinerHelper Helper(Observer, B, KB, MDT);

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -O0 -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - \
+# RUN:                                                                | FileCheck --check-prefix=ENABLED %s
+# RUN: llc -O0 -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
+...

llvm/utils/TableGen/GICombinerEmitter.cpp

 /// syntax
 ///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/Statistic.h"
 #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"
 #include "GlobalISel/CodeExpansions.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "gicombiner-emitter"
protected:
   /// FIXME: This is a temporary measure until we have actual pattern matching
   const CodeInit *Code = nullptr;
 public:
   CombineRule(const CodeGenTarget &Target, RuleID ID, const Record &R)
       : ID(ID), TheDef(R) {}
   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 *getCode() const { return Code; }
   size_t getNumRoots() const { return Roots.size(); }
 
   using const_root_iterator = std::vector<RootInfo>::const_iterator;
   const_root_iterator roots_begin() const { return Roots.begin(); }
   const_root_iterator roots_end() const { return Roots.end(); }
   iterator_range<const_root_iterator> roots() const {
explicit GICombinerEmitter(RecordKeeper &RK, const CodeGenTarget &Target,
                              StringRef Name, Record *Combiner);
   ~GICombinerEmitter() {}
 
   StringRef getClassName() const {
     return Combiner->getValueAsString("Classname");
   }
   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;
 };
 
 GICombinerEmitter::GICombinerEmitter(RecordKeeper &RK,
                                      const CodeGenTarget &Target,
                                      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 =
       std::make_unique<CombineRule>(Target, NumPatternTotal, TheDef);
 
   if (!Rule->parseDefs())
     return nullptr;
   if (!Rule->parseMatcher(Target))

 
 void GICombinerEmitter::generateCodeForRule(raw_ostream &OS,
                                             const CombineRule *Rule,
                                             StringRef Indent) const {
   {
     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()) {
       Expansions.declare(Root.getPatternSymbol(), "MI");
     }
     DagInit *Applyer = RuleDef.getValueAsDag("Apply");
     if (Applyer->getOperatorAsDef(RuleDef.getLoc())->getName() !=
         "apply") {

 }
 
 void GICombinerEmitter::run(raw_ostream &OS) {
   gatherRules(Rules, Combiner->getValueAsListOfDefs("Rules"));
   NamedRegionTimer T("Emit", "Time spent emitting the combiner",
                      "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"
+     << "#ifndef NDEBUG\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"
+     << "  } 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"
+     << "#ifndef NDEBUG\n"
+     << "  return DisabledRules.test(RuleID);\n"
+     << "#else // ifndef NDEBUG\n"
+     << "  return false;\n"
+     << "#endif // ifndef NDEBUG\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"
      << "  CombinerHelper Helper(Observer, B);\n"
      << "  MachineBasicBlock *MBB = MI.getParent();\n"
      << "  MachineFunction *MF = MBB->getParent();\n"
      << "  MachineRegisterInfo &MRI = MF->getRegInfo();\n"
      << "  (void)MBB; (void)MF; (void)MRI;\n\n";