[PATCH] D130163: [AArch64] Combine setcc (iN (bitcast (vNi1 X))) with vecreduce_or

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18199-18200
+      LHS->getOpcode() == ISD::BITCAST &&
+      LHS->getSimpleValueType(0) == MVT::i16 &&
+      LHS->getOperand(0)->getSimpleValueType(0) == MVT::v16i1) {
+    MVT OrigVT = LHS->getOperand(0).getSimpleValueType();
----------------
Do you need to be restrictive here? Would the result for larger vectors (e.g. v64i1) still be better than what we get today?

You might need to be careful about creating illegal operations.  Given the context of this combine, can it be restricted to before type legalisation?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18204
+                          DAG.getConstant(0, DL, OrigVT), N->getOperand(2));
+    return DAG.getNode(ISD::VECREDUCE_OR, DL, MVT::i1, SetCC);
+  }
----------------
The result type of `SETCC` doesn't have to be `i1` so you either need to extend the result based on `getBooleanContents()` or restrict the combine to cases where `VT==i1`. My preference is the former.


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

https://reviews.llvm.org/D130163