[PATCH] D27779: [ARM] More aggressive matching for vpadd and vpaddl.

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Generally, looks good. I have some comments on the format of the code, which could be a bit cleaner on the depths of the `if`s and commoning up the checks.

cheers,
--renato



================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9219
+  if ((N0.getOpcode() == ARMISD::VUZP ||
+      (N0.getOpcode() == ARMISD::VTRN && N0.getValueType() == MVT::v2i32)) &&
+      N0.getNode() == N1.getNode() &&
----------------
Why `v2i32` only? I can see from your tests that this also matches `i8`, so I'm suspecting this is already extended by the selection dag before getting here?

Also, `VPADD` works with `float`s, too.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9221
+      N0.getNode() == N1.getNode() &&
+      N0 != N1 && N->getValueType(0).is64BitVector()) {
+    // An add where the inputs both come from the same vuzp forms a vpadd.
----------------
Better to negate this `if` and move `return SDValue()` here.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9252
+    SDValue N10 = N1.getOperand(0);
+    if ((N00.getOpcode() == ARMISD::VUZP ||
+        (N00.getOpcode() == ARMISD::VTRN &&
----------------
This `if` is almost identical to the one above. I'd common them up in a local static function.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9256
+        N00.getNode() == N10.getNode() &&
+        N00 != N10 && N00.getValueType().is64BitVector() &&
+        N0.getValueType().is128BitVector()) {
----------------
`VPADDL` works on D and Q regs, not Q only.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9257
+        N00 != N10 && N00.getValueType().is64BitVector() &&
+        N0.getValueType().is128BitVector()) {
+      // An extended add where the inputs both come from the same vuzp forms
----------------
Same comment here, better to negate this `if` and `return SDValue()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D27779