[PATCH] D148185: Add more efficient bitwise vector reductions on AArch64

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Is it worth splitting this into i1 and non-i1 parts? There are quite a few changes in the test.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13188
-                      Op.getOpcode() == ISD::VECREDUCE_OR ||
-                      Op.getOpcode() == ISD::VECREDUCE_XOR ||
                       Op.getOpcode() == ISD::VECREDUCE_FADD ||
----------------
If SVE is available then the orv/eorv/etc should be preferred.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13216
+
+  // special case for boolean reductions
+  if (ElemVT == MVT::i1) {
----------------
special -> Special


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13218
+  if (ElemVT == MVT::i1) {
+    // split large vectors into smaller ones
+    if (NumElems > 16) {
----------------
split -> Split


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13222
+      std::tie(Lo, Hi) = DAG.SplitVector(Vec, DL);
+      return getVectorBitwiseReduce(Opcode, DAG.getNode(ScalarOpcode, DL, Lo.getValueType(), Lo, Hi), VT, DL, DAG);
+    }
----------------
Formatting - the line is a bot long here.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13248
+
+    // Iteratively apply the bitwise operator to the first and second half
+    // of the vector until only one element remains.
----------------
Would it be possible for vector <= 64bits to use the 64bit type sizes? It won't matter in a lot of cases but some cpu's have a higher throughput for 64bit vectors.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148185/new/

https://reviews.llvm.org/D148185