Skip to content

Commit d2b74d7

Browse files
authored
[TableGen] Handle duplicate rules in combiners (#69296)
We would crash when a rule was accidentally added twice to a combiner. This patch adds a warning instead to skip the already-processed rules.
1 parent 0841955 commit d2b74d7

File tree

2 files changed

+62
-18
lines changed

2 files changed

+62
-18
lines changed
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// RUN: llvm-tblgen -I %p/../../../../include -gen-global-isel-combiner \
2+
// RUN: -combiners=Combiner %s 2>&1 | FileCheck %s
3+
4+
include "llvm/Target/Target.td"
5+
include "llvm/Target/GlobalISel/Combine.td"
6+
7+
// Check we don't crash if a combine is present twice in the list.
8+
9+
def MyTargetISA : InstrInfo;
10+
def MyTarget : Target { let InstructionSet = MyTargetISA; }
11+
12+
def dummy;
13+
14+
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: warning: skipping rule 'Foo' because it has already been processed
15+
def Foo : GICombineRule<
16+
(defs root:$root),
17+
(match (G_ZEXT $root, $x)),
18+
(apply (G_TRUNC $root, $x))>;
19+
20+
def Bar : GICombineRule<
21+
(defs root:$root),
22+
(match (G_TRUNC $root, $x)),
23+
(apply (G_ZEXT $root, $x))>;
24+
25+
def FooBar : GICombineGroup<[ Foo, Bar ]>;
26+
27+
def Combiner: GICombiner<"GenMyCombiner", [
28+
FooBar,
29+
Foo
30+
]>;

llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3307,6 +3307,10 @@ class GICombinerEmitter final : public GlobalISelMatchTableExecutorEmitter {
33073307
// combine rule used to disable/enable it.
33083308
std::vector<std::pair<unsigned, std::string>> AllCombineRules;
33093309

3310+
// Keep track of all rules we've seen so far to ensure we don't process
3311+
// the same rule twice.
3312+
StringSet<> RulesSeen;
3313+
33103314
MatchTable buildMatchTable(MutableArrayRef<RuleMatcher> Rules);
33113315

33123316
void emitRuleConfigImpl(raw_ostream &OS);
@@ -3624,27 +3628,37 @@ void GICombinerEmitter::gatherRules(
36243628
std::vector<RuleMatcher> &ActiveRules,
36253629
const std::vector<Record *> &&RulesAndGroups) {
36263630
for (Record *Rec : RulesAndGroups) {
3627-
if (Rec->isValueUnset("Rules")) {
3628-
AllCombineRules.emplace_back(NextRuleID, Rec->getName().str());
3629-
CombineRuleBuilder CRB(Target, SubtargetFeatures, *Rec, NextRuleID++,
3630-
ActiveRules);
3631+
if (!Rec->isValueUnset("Rules")) {
3632+
gatherRules(ActiveRules, Rec->getValueAsListOfDefs("Rules"));
3633+
continue;
3634+
}
36313635

3632-
if (!CRB.parseAll()) {
3633-
assert(ErrorsPrinted && "Parsing failed without errors!");
3634-
continue;
3635-
}
3636+
StringRef RuleName = Rec->getName();
3637+
if (!RulesSeen.insert(RuleName).second) {
3638+
PrintWarning(Rec->getLoc(),
3639+
"skipping rule '" + Rec->getName() +
3640+
"' because it has already been processed");
3641+
continue;
3642+
}
36363643

3637-
if (StopAfterParse) {
3638-
CRB.print(outs());
3639-
continue;
3640-
}
3644+
AllCombineRules.emplace_back(NextRuleID, Rec->getName().str());
3645+
CombineRuleBuilder CRB(Target, SubtargetFeatures, *Rec, NextRuleID++,
3646+
ActiveRules);
36413647

3642-
if (!CRB.emitRuleMatchers()) {
3643-
assert(ErrorsPrinted && "Emission failed without errors!");
3644-
continue;
3645-
}
3646-
} else
3647-
gatherRules(ActiveRules, Rec->getValueAsListOfDefs("Rules"));
3648+
if (!CRB.parseAll()) {
3649+
assert(ErrorsPrinted && "Parsing failed without errors!");
3650+
continue;
3651+
}
3652+
3653+
if (StopAfterParse) {
3654+
CRB.print(outs());
3655+
continue;
3656+
}
3657+
3658+
if (!CRB.emitRuleMatchers()) {
3659+
assert(ErrorsPrinted && "Emission failed without errors!");
3660+
continue;
3661+
}
36483662
}
36493663
}
36503664

0 commit comments

Comments
 (0)