[PATCH] D28489: [CodeGen] Move MacroFusion to the target

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

LGTM with nitpicks addressed:



================
Comment at: llvm/lib/Target/AArch64/AArch64MacroFusion.cpp:10
+//
+// This file contains the AArch64 implementation of the DAG scheduling mutation
+// to pair instructions back to back.
----------------
Should be `/// \file This file ...`


================
Comment at: llvm/lib/Target/AArch64/AArch64MacroFusion.cpp:191
+
+std::unique_ptr<ScheduleDAGMutation> createAArch64MacroFusionDAGMutation () {
+  return EnableMacroFusion ? make_unique<AArch64MacroFusion>() : nullptr;
----------------
No space before `()`.


================
Comment at: llvm/lib/Target/AArch64/AArch64MacroFusion.cpp:195
+
+} // llvm
----------------
Should be `// end namespace llvm` according to coding standards.


================
Comment at: llvm/lib/Target/AArch64/AArch64MacroFusion.h:10
+//
+// This file contains the AArch64 definition of the DAG scheduling mutation
+// to pair instructions back to back.
----------------
Should be `/// \file This file ...`


================
Comment at: llvm/lib/Target/AArch64/AArch64MacroFusion.h:24-31
+/// \brief Post-process the DAG to create cluster edges between instructions
+/// that may be fused by the processor into a single operation.
+class AArch64MacroFusion : public ScheduleDAGMutation {
+public:
+  AArch64MacroFusion() {}
+
+  void apply(ScheduleDAGInstrs *DAGInstrs) override;
----------------
Please move the class declaration into the AArch64MacroFusion.cpp file and into an anonymous namespace.


================
Comment at: llvm/lib/Target/X86/X86MacroFusion.cpp:10
+//
+// This file contains the X86 implementation of the DAG scheduling mutation to
+// pair instructions back to back.
----------------
Should be `/// \file ...`


================
Comment at: llvm/lib/Target/X86/X86MacroFusion.cpp:245
+std::unique_ptr<ScheduleDAGMutation>
+createX86MacroFusionDAGMutation () {
+  return EnableMacroFusion ? make_unique<X86MacroFusion>() : nullptr;
----------------
No space before `()`


================
Comment at: llvm/lib/Target/X86/X86MacroFusion.cpp:249
+
+} // llvm
----------------
Should be `// end namespace llvm`.


================
Comment at: llvm/lib/Target/X86/X86MacroFusion.h:10
+//
+// This file contains the X86 definition of the DAG scheduling mutation to pair
+// instructions back to back.
----------------
Should be `/// \file This file ...`


================
Comment at: llvm/lib/Target/X86/X86MacroFusion.h:24-31
+/// \brief Post-process the DAG to create cluster edges between instructions
+/// that may be fused by the processor into a single operation.
+class X86MacroFusion : public ScheduleDAGMutation {
+public:
+  X86MacroFusion() {}
+
+  void apply(ScheduleDAGInstrs *DAGInstrs) override;
----------------
Please move the class declaration into the X86MacroFusion.cpp file and into an anonymous namespace.


Repository:
  rL LLVM

https://reviews.llvm.org/D28489