[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

llvm-svn: 375057

llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h

@@ -85,8 +85,9 @@ class CombinerHelper {
   /// legal and the surrounding code makes it useful.
   bool tryCombineIndexedLoadStore(MachineInstr &MI);
 
-  bool matchCombineBr(MachineInstr &MI);
-  bool tryCombineBr(MachineInstr &MI);
+  bool matchElideBrByInvertingCond(MachineInstr &MI);
+  void applyElideBrByInvertingCond(MachineInstr &MI);
+  bool tryElideBrByInvertingCond(MachineInstr &MI);
 
   /// Optimize memcpy intrinsics et al, e.g. constant len calls.
   /// /p MaxLen if non-zero specifies the max length of a mem libcall to inline.

llvm/include/llvm/TableGen/Error.h

@@ -18,6 +18,7 @@
 
 namespace llvm {
 
+void PrintNote(const Twine &Msg);
 void PrintNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg);
 
 void PrintWarning(ArrayRef<SMLoc> WarningLoc, const Twine &Msg);

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

@@ -10,8 +10,91 @@
 //
 //===----------------------------------------------------------------------===//
 
+// Common base class for GICombineRule and GICombineGroup.
+class GICombine {
+  // See GICombineGroup. We only declare it here to make the tablegen pass
+  // simpler.
+  list<GICombine> Rules = ?;
+}
+
+// A group of combine rules that can be added to a GICombiner or another group.
+class GICombineGroup<list<GICombine> rules> : GICombine {
+  // The rules contained in this group. The rules in a group are flattened into
+  // a single list and sorted into whatever order is most efficient. However,
+  // they will never be re-ordered such that behaviour differs from the
+  // specified order. It is therefore possible to use the order of rules in this
+  // list to describe priorities.
+  let Rules = rules;
+}
+
 // Declares a combiner helper class
-class GICombinerHelper<string classname> {
+class GICombinerHelper<string classname, list<GICombine> rules>
+    : GICombineGroup<rules> {
   // The class name to use in the generated output.
   string Classname = classname;
 }
+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
+  /// See GIMatchKind for details.
+  dag Match = match;
+
+  /// Defines the things which happen after the decision is made to apply a
+  /// combine rule.
+  /// See GIApplyKind for details.
+  dag Apply = apply;
+}
+
+/// The operator at the root of a GICombineRule.Defs dag.
+def defs;
+
+/// All arguments of the defs operator must be subclasses of GIDefKind or
+/// sub-dags whose operator is GIDefKindWithArgs.
+class GIDefKind;
+class GIDefKindWithArgs;
+/// Declare a root node. There must be at least one of these in every combine
+/// rule.
+/// TODO: The plan is to elide `root` definitions and determine it from the DAG
+///       itself with an overide for situations where the usual determination
+///       is incorrect.
+def root : GIDefKind;
+
+/// The operator at the root of a GICombineRule.Match dag.
+def match;
+/// All arguments of the match operator must be either:
+/// * A subclass of GIMatchKind
+/// * A subclass of GIMatchKindWithArgs
+/// * A MIR code block (deprecated)
+/// The GIMatchKind and GIMatchKindWithArgs cases are described in more detail
+/// in their definitions below.
+/// For the Instruction case, these are collected into a DAG where operand names
+/// that occur multiple times introduce edges.
+class GIMatchKind;
+class GIMatchKindWithArgs;
+
+/// The operator at the root of a GICombineRule.Apply dag.
+def apply;
+/// All arguments of the apply operator must be subclasses of GIApplyKind, or
+/// sub-dags whose operator is GIApplyKindWithArgs, or an MIR block
+/// (deprecated).
+class GIApplyKind;
+class GIApplyKindWithArgs;
+
+def copy_prop : GICombineRule<
+  (defs root:$d),
+  (match [{ return Helper.matchCombineCopy(${d}); }]),
+  (apply [{ Helper.applyCombineCopy(${d}); }])>;
+def trivial_combines : GICombineGroup<[copy_prop]>;
+
+// FIXME: Is there a reason this wasn't in tryCombine? I've left it out of
+//        all_combines because it wasn't there.
+def elide_br_by_inverting_cond : GICombineRule<
+  (defs root:$d),
+  (match [{ return Helper.matchElideBrByInvertingCond(${d}); }]),
+  (apply [{ Helper.applyElideBrByInvertingCond(${d}); }])>;
+
+def all_combines : GICombineGroup<[trivial_combines]>;

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

@@ -561,8 +561,10 @@ bool CombinerHelper::tryCombineIndexedLoadStore(MachineInstr &MI) {
   return true;
 }
 
-bool CombinerHelper::matchCombineBr(MachineInstr &MI) {
-  assert(MI.getOpcode() == TargetOpcode::G_BR && "Expected a G_BR");
+bool CombinerHelper::matchElideBrByInvertingCond(MachineInstr &MI) {
+  if (MI.getOpcode() != TargetOpcode::G_BR)
+    return false;
+
   // Try to match the following:
   // bb1:
   //   %c(s32) = G_ICMP pred, %a, %b
@@ -599,9 +601,14 @@ bool CombinerHelper::matchCombineBr(MachineInstr &MI) {
   return true;
 }
 
-bool CombinerHelper::tryCombineBr(MachineInstr &MI) {
-  if (!matchCombineBr(MI))
+bool CombinerHelper::tryElideBrByInvertingCond(MachineInstr &MI) {
+  if (!matchElideBrByInvertingCond(MI))
     return false;
+  applyElideBrByInvertingCond(MI);
+  return true;
+}
+
+void CombinerHelper::applyElideBrByInvertingCond(MachineInstr &MI) {
   MachineBasicBlock *BrTarget = MI.getOperand(0).getMBB();
   MachineBasicBlock::iterator BrIt(MI);
   MachineInstr *BrCond = &*std::prev(BrIt);
@@ -620,7 +627,6 @@ bool CombinerHelper::tryCombineBr(MachineInstr &MI) {
   BrCond->getOperand(1).setMBB(BrTarget);
   Observer.changedInstr(*BrCond);
   MI.eraseFromParent();
-  return true;
 }
 
 static bool shouldLowerMemFuncForSize(const MachineFunction &MF) {

llvm/lib/TableGen/Error.cpp

@@ -39,6 +39,8 @@ static void PrintMessage(ArrayRef<SMLoc> Loc, SourceMgr::DiagKind Kind,
                         "instantiated from multiclass");
 }
 
+void PrintNote(const Twine &Msg) { WithColor::note() << Msg << "\n"; }
+
 void PrintNote(ArrayRef<SMLoc> NoteLoc, const Twine &Msg) {
   PrintMessage(NoteLoc, SourceMgr::DK_Note, Msg);
 }

llvm/lib/Target/AArch64/AArch64Combine.td

@@ -12,4 +12,5 @@
 include "llvm/Target/GlobalISel/Combine.td"
 
 def AArch64PreLegalizerCombinerHelper: GICombinerHelper<
-  "AArch64GenPreLegalizerCombinerHelper">;
+  "AArch64GenPreLegalizerCombinerHelper", [all_combines,
+                                           elide_br_by_inverting_cond]>;

llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp

@@ -58,12 +58,6 @@ bool AArch64PreLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
   CombinerHelper Helper(Observer, B, KB, MDT);
 
   switch (MI.getOpcode()) {
-  default:
-    return false;
-  case TargetOpcode::COPY:
-    return Helper.tryCombineCopy(MI);
-  case TargetOpcode::G_BR:
-    return Helper.tryCombineBr(MI);
   case TargetOpcode::G_LOAD:
   case TargetOpcode::G_SEXTLOAD:
   case TargetOpcode::G_ZEXTLOAD: {
@@ -118,7 +112,7 @@ class AArch64PreLegalizerCombiner : public MachineFunctionPass {
 private:
   bool IsOptNone;
 };
-}
+} // end anonymous namespace
 
 void AArch64PreLegalizerCombiner::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<TargetPassConfig>();