From 329e748c8c3b0fd3942cb1dafa9e4d0bb0aea8a4 Mon Sep 17 00:00:00 2001 From: Daniel Sanders Date: Thu, 17 Oct 2019 00:37:04 +0000 Subject: [PATCH] [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 llvm-svn: 375067 --- llvm/include/llvm/Target/GlobalISel/Combine.td | 3 + llvm/lib/CodeGen/GlobalISel/Combiner.cpp | 12 +++ llvm/lib/Target/AArch64/AArch64Combine.td | 4 +- .../Target/AArch64/AArch64PreLegalizerCombiner.cpp | 6 +- .../prelegalizercombiner-copy-prop-disabled.mir | 35 ++++++++ llvm/utils/TableGen/GICombinerEmitter.cpp | 100 ++++++++++++++++++++- 6 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td index 6f38cca..dcac399 100644 --- a/llvm/include/llvm/Target/GlobalISel/Combine.td +++ b/llvm/include/llvm/Target/GlobalISel/Combine.td @@ -32,6 +32,9 @@ class GICombinerHelper rules> : GICombineGroup { // 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 : GICombine { /// Defines the external interface of the match rule. This includes: diff --git a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp index 644ce2e..b4562a5 100644 --- a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp +++ b/llvm/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/llvm/lib/Target/AArch64/AArch64Combine.td b/llvm/lib/Target/AArch64/AArch64Combine.td index 8956730..bb99f25 100644 --- a/llvm/lib/Target/AArch64/AArch64Combine.td +++ b/llvm/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/llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp index e76db5c..4e3e8e3 100644 --- a/llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp +++ b/llvm/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/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir new file mode 100644 index 0000000..bd6756a --- /dev/null +++ b/llvm/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/llvm/utils/TableGen/GICombinerEmitter.cpp b/llvm/utils/TableGen/GICombinerEmitter.cpp index 676decc..f243725 100644 --- a/llvm/utils/TableGen/GICombinerEmitter.cpp +++ b/llvm/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> 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 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 GICombinerEmitter::makeCombineRule(const Record &TheDef) { std::unique_ptr 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 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 " << 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" -- 2.7.4