Skip to content
This repository was archived by the owner on Apr 23, 2020. It is now read-only.

Commit 2ab9442

Browse files
committed
[gicombiner] Hoist pure C++ combine into the tablegen definition
Summary: This is just moving the existing C++ code around and will be NFC w.r.t AArch64. Renamed 'CombineBr' to something more descriptive ('ElideByByInvertingCond') at the same time. The remaining combines in AArch64PreLegalizeCombiner require features that aren't implemented at this point and will be hoisted as they are added. Depends on D68424 Reviewers: bogner, volkan Subscribers: kristof.beyls, hiraditya, Petar.Avramovic, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D68426 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@375057 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent a6de5ed commit 2ab9442

File tree

8 files changed

+368
-22
lines changed

8 files changed

+368
-22
lines changed

Diff for: include/llvm/CodeGen/GlobalISel/CombinerHelper.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,9 @@ class CombinerHelper {
8585
/// legal and the surrounding code makes it useful.
8686
bool tryCombineIndexedLoadStore(MachineInstr &MI);
8787

88-
bool matchCombineBr(MachineInstr &MI);
89-
bool tryCombineBr(MachineInstr &MI);
88+
bool matchElideBrByInvertingCond(MachineInstr &MI);
89+
void applyElideBrByInvertingCond(MachineInstr &MI);
90+
bool tryElideBrByInvertingCond(MachineInstr &MI);
9091

9192
/// Optimize memcpy intrinsics et al, e.g. constant len calls.
9293
/// /p MaxLen if non-zero specifies the max length of a mem libcall to inline.

Diff for: include/llvm/TableGen/Error.h

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
namespace llvm {
2020

21+
void PrintNote(const Twine &Msg);
2122
void PrintNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg);
2223

2324
void PrintWarning(ArrayRef<SMLoc> WarningLoc, const Twine &Msg);

Diff for: include/llvm/Target/GlobalISel/Combine.td

+84-1
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,91 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
// Common base class for GICombineRule and GICombineGroup.
14+
class GICombine {
15+
// See GICombineGroup. We only declare it here to make the tablegen pass
16+
// simpler.
17+
list<GICombine> Rules = ?;
18+
}
19+
20+
// A group of combine rules that can be added to a GICombiner or another group.
21+
class GICombineGroup<list<GICombine> rules> : GICombine {
22+
// The rules contained in this group. The rules in a group are flattened into
23+
// a single list and sorted into whatever order is most efficient. However,
24+
// they will never be re-ordered such that behaviour differs from the
25+
// specified order. It is therefore possible to use the order of rules in this
26+
// list to describe priorities.
27+
let Rules = rules;
28+
}
29+
1330
// Declares a combiner helper class
14-
class GICombinerHelper<string classname> {
31+
class GICombinerHelper<string classname, list<GICombine> rules>
32+
: GICombineGroup<rules> {
1533
// The class name to use in the generated output.
1634
string Classname = classname;
1735
}
36+
class GICombineRule<dag defs, dag match, dag apply> : GICombine {
37+
/// Defines the external interface of the match rule. This includes:
38+
/// * The names of the root nodes (requires at least one)
39+
/// See GIDefKind for details.
40+
dag Defs = defs;
41+
42+
/// Defines the things which must be true for the pattern to match
43+
/// See GIMatchKind for details.
44+
dag Match = match;
45+
46+
/// Defines the things which happen after the decision is made to apply a
47+
/// combine rule.
48+
/// See GIApplyKind for details.
49+
dag Apply = apply;
50+
}
51+
52+
/// The operator at the root of a GICombineRule.Defs dag.
53+
def defs;
54+
55+
/// All arguments of the defs operator must be subclasses of GIDefKind or
56+
/// sub-dags whose operator is GIDefKindWithArgs.
57+
class GIDefKind;
58+
class GIDefKindWithArgs;
59+
/// Declare a root node. There must be at least one of these in every combine
60+
/// rule.
61+
/// TODO: The plan is to elide `root` definitions and determine it from the DAG
62+
/// itself with an overide for situations where the usual determination
63+
/// is incorrect.
64+
def root : GIDefKind;
65+
66+
/// The operator at the root of a GICombineRule.Match dag.
67+
def match;
68+
/// All arguments of the match operator must be either:
69+
/// * A subclass of GIMatchKind
70+
/// * A subclass of GIMatchKindWithArgs
71+
/// * A MIR code block (deprecated)
72+
/// The GIMatchKind and GIMatchKindWithArgs cases are described in more detail
73+
/// in their definitions below.
74+
/// For the Instruction case, these are collected into a DAG where operand names
75+
/// that occur multiple times introduce edges.
76+
class GIMatchKind;
77+
class GIMatchKindWithArgs;
78+
79+
/// The operator at the root of a GICombineRule.Apply dag.
80+
def apply;
81+
/// All arguments of the apply operator must be subclasses of GIApplyKind, or
82+
/// sub-dags whose operator is GIApplyKindWithArgs, or an MIR block
83+
/// (deprecated).
84+
class GIApplyKind;
85+
class GIApplyKindWithArgs;
86+
87+
def copy_prop : GICombineRule<
88+
(defs root:$d),
89+
(match [{ return Helper.matchCombineCopy(${d}); }]),
90+
(apply [{ Helper.applyCombineCopy(${d}); }])>;
91+
def trivial_combines : GICombineGroup<[copy_prop]>;
92+
93+
// FIXME: Is there a reason this wasn't in tryCombine? I've left it out of
94+
// all_combines because it wasn't there.
95+
def elide_br_by_inverting_cond : GICombineRule<
96+
(defs root:$d),
97+
(match [{ return Helper.matchElideBrByInvertingCond(${d}); }]),
98+
(apply [{ Helper.applyElideBrByInvertingCond(${d}); }])>;
99+
100+
def all_combines : GICombineGroup<[trivial_combines]>;

Diff for: lib/CodeGen/GlobalISel/CombinerHelper.cpp

+11-5
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,10 @@ bool CombinerHelper::tryCombineIndexedLoadStore(MachineInstr &MI) {
561561
return true;
562562
}
563563

564-
bool CombinerHelper::matchCombineBr(MachineInstr &MI) {
565-
assert(MI.getOpcode() == TargetOpcode::G_BR && "Expected a G_BR");
564+
bool CombinerHelper::matchElideBrByInvertingCond(MachineInstr &MI) {
565+
if (MI.getOpcode() != TargetOpcode::G_BR)
566+
return false;
567+
566568
// Try to match the following:
567569
// bb1:
568570
// %c(s32) = G_ICMP pred, %a, %b
@@ -599,9 +601,14 @@ bool CombinerHelper::matchCombineBr(MachineInstr &MI) {
599601
return true;
600602
}
601603

602-
bool CombinerHelper::tryCombineBr(MachineInstr &MI) {
603-
if (!matchCombineBr(MI))
604+
bool CombinerHelper::tryElideBrByInvertingCond(MachineInstr &MI) {
605+
if (!matchElideBrByInvertingCond(MI))
604606
return false;
607+
applyElideBrByInvertingCond(MI);
608+
return true;
609+
}
610+
611+
void CombinerHelper::applyElideBrByInvertingCond(MachineInstr &MI) {
605612
MachineBasicBlock *BrTarget = MI.getOperand(0).getMBB();
606613
MachineBasicBlock::iterator BrIt(MI);
607614
MachineInstr *BrCond = &*std::prev(BrIt);
@@ -620,7 +627,6 @@ bool CombinerHelper::tryCombineBr(MachineInstr &MI) {
620627
BrCond->getOperand(1).setMBB(BrTarget);
621628
Observer.changedInstr(*BrCond);
622629
MI.eraseFromParent();
623-
return true;
624630
}
625631

626632
static bool shouldLowerMemFuncForSize(const MachineFunction &MF) {

Diff for: lib/TableGen/Error.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ static void PrintMessage(ArrayRef<SMLoc> Loc, SourceMgr::DiagKind Kind,
3939
"instantiated from multiclass");
4040
}
4141

42+
void PrintNote(const Twine &Msg) { WithColor::note() << Msg << "\n"; }
43+
4244
void PrintNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg) {
4345
PrintMessage(NoteLoc, SourceMgr::DK_Note, Msg);
4446
}

Diff for: lib/Target/AArch64/AArch64Combine.td

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@
1212
include "llvm/Target/GlobalISel/Combine.td"
1313

1414
def AArch64PreLegalizerCombinerHelper: GICombinerHelper<
15-
"AArch64GenPreLegalizerCombinerHelper">;
15+
"AArch64GenPreLegalizerCombinerHelper", [all_combines,
16+
elide_br_by_inverting_cond]>;

Diff for: lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp

+1-7
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,6 @@ bool AArch64PreLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
5858
CombinerHelper Helper(Observer, B, KB, MDT);
5959

6060
switch (MI.getOpcode()) {
61-
default:
62-
return false;
63-
case TargetOpcode::COPY:
64-
return Helper.tryCombineCopy(MI);
65-
case TargetOpcode::G_BR:
66-
return Helper.tryCombineBr(MI);
6761
case TargetOpcode::G_LOAD:
6862
case TargetOpcode::G_SEXTLOAD:
6963
case TargetOpcode::G_ZEXTLOAD: {
@@ -118,7 +112,7 @@ class AArch64PreLegalizerCombiner : public MachineFunctionPass {
118112
private:
119113
bool IsOptNone;
120114
};
121-
}
115+
} // end anonymous namespace
122116

123117
void AArch64PreLegalizerCombiner::getAnalysisUsage(AnalysisUsage &AU) const {
124118
AU.addRequired<TargetPassConfig>();

0 commit comments

Comments
 (0)