Skip to content

Commit 329e748

Browse files
dsandersllvmdsandersllvm
committedOct 17, 2019
[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
1 parent c319afc commit 329e748

File tree

6 files changed

+155
-5
lines changed

6 files changed

+155
-5
lines changed
 

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ class GICombinerHelper<string classname, list<GICombine> rules>
3232
: GICombineGroup<rules> {
3333
// The class name to use in the generated output.
3434
string Classname = classname;
35+
// The name of a run-time compiler option that will be generated to disable
36+
// specific rules within this combiner.
37+
string DisableRuleOption = ?;
3538
}
3639
class GICombineRule<dag defs, dag match, dag apply> : GICombine {
3740
/// Defines the external interface of the match rule. This includes:

‎llvm/lib/CodeGen/GlobalISel/Combiner.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,18 @@
2727

2828
using namespace llvm;
2929

30+
namespace llvm {
31+
cl::OptionCategory GICombinerOptionCategory(
32+
"GlobalISel Combiner",
33+
"Control the rules which are enabled. These options all take a comma "
34+
"separated list of rules to disable and may be specified by number "
35+
"or number range (e.g. 1-10)."
36+
#ifndef NDEBUG
37+
" They may also be specified by name."
38+
#endif
39+
);
40+
} // end namespace llvm
41+
3042
namespace {
3143
/// This class acts as the glue the joins the CombinerHelper to the overall
3244
/// Combine algorithm. The CombinerHelper is intended to report the

‎llvm/lib/Target/AArch64/AArch64Combine.td

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,6 @@ include "llvm/Target/GlobalISel/Combine.td"
1313

1414
def AArch64PreLegalizerCombinerHelper: GICombinerHelper<
1515
"AArch64GenPreLegalizerCombinerHelper", [all_combines,
16-
elide_br_by_inverting_cond]>;
16+
elide_br_by_inverting_cond]> {
17+
let DisableRuleOption = "aarch64prelegalizercombiner-disable-rule";
18+
}

‎llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,11 @@ class AArch64PreLegalizerCombinerInfo : public CombinerInfo {
4747
GISelKnownBits *KB, MachineDominatorTree *MDT)
4848
: CombinerInfo(/*AllowIllegalOps*/ true, /*ShouldLegalizeIllegal*/ false,
4949
/*LegalizerInfo*/ nullptr, EnableOpt, OptSize, MinSize),
50-
KB(KB), MDT(MDT) {}
50+
KB(KB), MDT(MDT) {
51+
if (!Generated.parseCommandLineOption())
52+
report_fatal_error("Invalid rule identifier");
53+
}
54+
5155
virtual bool combine(GISelChangeObserver &Observer, MachineInstr &MI,
5256
MachineIRBuilder &B) const override;
5357
};

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2+
# RUN: llc -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - \
3+
# RUN: | FileCheck --check-prefix=ENABLED %s
4+
# RUN: llc -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - \
5+
# RUN: --aarch64prelegalizercombinerhelper-disable-rule=copy_prop | FileCheck --check-prefix=DISABLED %s
6+
7+
# REQUIRES: asserts
8+
9+
--- |
10+
target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
11+
target triple = "aarch64--"
12+
define void @test_copy(i8* %addr) {
13+
entry:
14+
ret void
15+
}
16+
...
17+
18+
---
19+
name: test_copy
20+
body: |
21+
bb.0.entry:
22+
liveins: $x0
23+
; ENABLED-LABEL: name: test_copy
24+
; ENABLED: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
25+
; ENABLED: $x0 = COPY [[COPY]](p0)
26+
; DISABLED-LABEL: name: test_copy
27+
; DISABLED: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
28+
; DISABLED: [[COPY1:%[0-9]+]]:_(p0) = COPY [[COPY]](p0)
29+
; DISABLED: [[COPY2:%[0-9]+]]:_(p0) = COPY [[COPY1]](p0)
30+
; DISABLED: $x0 = COPY [[COPY2]](p0)
31+
%0:_(p0) = COPY $x0
32+
%1:_(p0) = COPY %0
33+
%2:_(p0) = COPY %1
34+
$x0 = COPY %2
35+
...

‎llvm/utils/TableGen/GICombinerEmitter.cpp

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "llvm/Support/CommandLine.h"
1616
#include "llvm/Support/Timer.h"
1717
#include "llvm/TableGen/Error.h"
18+
#include "llvm/TableGen/StringMatcher.h"
1819
#include "llvm/TableGen/TableGenBackend.h"
1920
#include "CodeGenTarget.h"
2021
#include "GlobalISel/CodeExpander.h"
@@ -75,6 +76,8 @@ class CombineRule {
7576
bool parseDefs();
7677
bool parseMatcher(const CodeGenTarget &Target);
7778

79+
RuleID getID() const { return ID; }
80+
StringRef getName() const { return TheDef.getName(); }
7881
const Record &getDef() const { return TheDef; }
7982
const CodeInit *getMatchingFixupCode() const { return MatchingFixupCode; }
8083
size_t getNumRoots() const { return Roots.size(); }
@@ -202,6 +205,9 @@ class GICombinerEmitter {
202205
}
203206
void run(raw_ostream &OS);
204207

208+
/// Emit the name matcher (guarded by #ifndef NDEBUG) used to disable rules in
209+
/// response to the generated cl::opt.
210+
void emitNameMatcher(raw_ostream &OS) const;
205211
void generateCodeForRule(raw_ostream &OS, const CombineRule *Rule,
206212
StringRef Indent) const;
207213
};
@@ -211,6 +217,32 @@ GICombinerEmitter::GICombinerEmitter(RecordKeeper &RK,
211217
StringRef Name, Record *Combiner)
212218
: Name(Name), Target(Target), Combiner(Combiner) {}
213219

220+
void GICombinerEmitter::emitNameMatcher(raw_ostream &OS) const {
221+
std::vector<std::pair<std::string, std::string>> Cases;
222+
Cases.reserve(Rules.size());
223+
224+
for (const CombineRule &EnumeratedRule : make_pointee_range(Rules)) {
225+
std::string Code;
226+
raw_string_ostream SS(Code);
227+
SS << "return " << EnumeratedRule.getID() << ";\n";
228+
Cases.push_back(std::make_pair(EnumeratedRule.getName(), SS.str()));
229+
}
230+
231+
OS << "#ifndef NDEBUG\n"
232+
<< "static Optional<uint64_t> getRuleIdxForIdentifier(StringRef "
233+
"RuleIdentifier) {\n"
234+
<< " uint64_t I;\n"
235+
<< " // getAtInteger(...) returns false on success\n"
236+
<< " bool Parsed = !RuleIdentifier.getAsInteger(0, I);\n"
237+
<< " if (Parsed)\n"
238+
<< " return I;\n\n";
239+
StringMatcher Matcher("RuleIdentifier", Cases, OS);
240+
Matcher.Emit();
241+
OS << " return None;\n"
242+
<< "}\n"
243+
<< "#endif // ifndef NDEBUG\n\n";
244+
}
245+
214246
std::unique_ptr<CombineRule>
215247
GICombinerEmitter::makeCombineRule(const Record &TheDef) {
216248
std::unique_ptr<CombineRule> Rule =
@@ -254,7 +286,7 @@ void GICombinerEmitter::generateCodeForRule(raw_ostream &OS,
254286
const Record &RuleDef = Rule->getDef();
255287

256288
OS << Indent << "// Rule: " << RuleDef.getName() << "\n"
257-
<< Indent << "{\n";
289+
<< Indent << "if (!isRuleDisabled(" << Rule->getID() << ")) {\n";
258290

259291
CodeExpansions Expansions;
260292
for (const RootInfo &Root : Rule->roots()) {
@@ -309,21 +341,83 @@ void GICombinerEmitter::run(raw_ostream &OS) {
309341
"Code Generation", "Time spent generating code",
310342
TimeRegions);
311343
OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_DEPS\n"
344+
<< "#include \"llvm/ADT/SparseBitVector.h\"\n"
345+
<< "namespace llvm {\n"
346+
<< "extern cl::OptionCategory GICombinerOptionCategory;\n"
347+
<< "} // end namespace llvm\n"
312348
<< "#endif // ifdef " << Name.upper() << "_GENCOMBINERHELPER_DEPS\n\n";
313349

314350
OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_H\n"
315351
<< "class " << getClassName() << " {\n"
352+
<< " SparseBitVector<> DisabledRules;\n"
353+
<< "\n"
316354
<< "public:\n"
355+
<< " bool parseCommandLineOption();\n"
356+
<< " bool isRuleDisabled(unsigned ID) const;\n"
357+
<< " bool setRuleDisabled(StringRef RuleIdentifier);\n"
358+
<< "\n"
317359
<< " bool tryCombineAll(\n"
318360
<< " GISelChangeObserver &Observer,\n"
319361
<< " MachineInstr &MI,\n"
320362
<< " MachineIRBuilder &B) const;\n"
321-
<< "};\n";
363+
<< "};\n\n";
364+
365+
emitNameMatcher(OS);
366+
367+
OS << "bool " << getClassName()
368+
<< "::setRuleDisabled(StringRef RuleIdentifier) {\n"
369+
<< " std::pair<StringRef, StringRef> RangePair = "
370+
"RuleIdentifier.split('-');\n"
371+
<< " if (!RangePair.second.empty()) {\n"
372+
<< " const auto First = getRuleIdxForIdentifier(RangePair.first);\n"
373+
<< " const auto Last = getRuleIdxForIdentifier(RangePair.second);\n"
374+
<< " if (!First.hasValue() || !Last.hasValue())\n"
375+
<< " return false;\n"
376+
<< " if (First >= Last)\n"
377+
<< " report_fatal_error(\"Beginning of range should be before end of "
378+
"range\");\n"
379+
<< " for (auto I = First.getValue(); I < Last.getValue(); ++I)\n"
380+
<< " DisabledRules.set(I);\n"
381+
<< " return true;\n"
382+
<< " }\n"
383+
<< "#ifndef NDEBUG\n"
384+
<< " else {\n"
385+
<< " const auto I = getRuleIdxForIdentifier(RangePair.first);\n"
386+
<< " if (!I.hasValue())\n"
387+
<< " return false;\n"
388+
<< " DisabledRules.set(I.getValue());\n"
389+
<< " return true;\n"
390+
<< " }\n"
391+
<< "#else // ifndef NDEBUG\n"
392+
<< " llvm_unreachable(\"Cannot disable rules in non-asserts builds\");\n"
393+
<< " return false;\n"
394+
<< "#endif // ifndef NDEBUG\n\n"
395+
<< "}\n";
396+
397+
OS << "bool " << getClassName()
398+
<< "::isRuleDisabled(unsigned RuleID) const {\n"
399+
<< " return DisabledRules.test(RuleID);\n"
400+
<< "}\n";
322401
OS << "#endif // ifdef " << Name.upper() << "_GENCOMBINERHELPER_H\n\n";
323402

324403
OS << "#ifdef " << Name.upper() << "_GENCOMBINERHELPER_CPP\n"
325404
<< "\n"
326-
<< "bool " << getClassName() << "::tryCombineAll(\n"
405+
<< "cl::list<std::string> " << Name << "Option(\n"
406+
<< " \"" << Name.lower() << "-disable-rule\",\n"
407+
<< " cl::desc(\"Disable one or more combiner rules temporarily in "
408+
<< "the " << Name << " pass\"),\n"
409+
<< " cl::CommaSeparated,\n"
410+
<< " cl::Hidden,\n"
411+
<< " cl::cat(GICombinerOptionCategory));\n"
412+
<< "\n"
413+
<< "bool " << getClassName() << "::parseCommandLineOption() {\n"
414+
<< " for (const auto &Identifier : " << Name << "Option)\n"
415+
<< " if (!setRuleDisabled(Identifier))\n"
416+
<< " return false;\n"
417+
<< " return true;\n"
418+
<< "}\n\n";
419+
420+
OS << "bool " << getClassName() << "::tryCombineAll(\n"
327421
<< " GISelChangeObserver &Observer,\n"
328422
<< " MachineInstr &MI,\n"
329423
<< " MachineIRBuilder &B) const {\n"

0 commit comments

Comments
 (0)