[PATCH] D12887: [Machine Combiner] Refactor machine reassociation into a target-independent pass

[Sanjay Patel via llvm-commits]

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

LGTM too. Just a few inline nits.


================
Comment at: include/llvm/CodeGen/MachineCombinerPattern.h:35
@@ +34,3 @@
+
+  /// Enumeration of instruction pattern supported by machine combiner
+  MC_NONE = 4,
----------------
The comment should say that these are AArch64 patterns. It might even be worth making that explicit in the names like:
MC_AARCH64_MULADDW_OP1
but that can be a follow-on change since it will mean changing the AArch code.

================
Comment at: include/llvm/Target/TargetInstrInfo.h:736
@@ +735,3 @@
+  /// instruction of the same type define the first source operand, \P Commuted
+  /// will
+  /// be set to true.
----------------
Extra line.

================
Comment at: lib/CodeGen/TargetInstrInfo.cpp:630
@@ +629,3 @@
+  unsigned OpIdx[4][4] = {
+      {1, 1, 2, 2}, {1, 2, 2, 1}, {2, 1, 1, 2}, {2, 2, 1, 1}};
+
----------------
I'm biased of course, but I think it's easier to understand the logic when the rows of the array are actually on different rows in the source.


Repository:
  rL LLVM

http://reviews.llvm.org/D12887