[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:18226
+    EVT OrigVT = LHS->getOperand(0).getValueType();
+    if (LHS->getValueType(0).isScalarInteger() && OrigVT.isVector() &&
+        OrigVT.getVectorElementType() == MVT::i1) {
----------------
This can be just `VT.isScalar()` and made part of the first if statement since there's no point querying the setcc operands until after we know they're scalar.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18236
+      auto Ext = DAG.getNode(ExtendCode, DL, CCVT, SetCC);
+      return DAG.getNode(ISD::VECREDUCE_OR, DL, VT, Ext);
+    }
----------------
You've misunderstood my previous `getBooleanContents` comment but that doesn't matter because I think the transformation is not correct anyway.  The input (i.e. `setcc (iN (bitcast (vNi1 X))), 0, eq`) is saying "return true if and only if all bits in `X` are false", where as `vecreduce_or (setcc X, 0, eq)` will return true if any bit in `X` is false".

I think the transformation you want is: `setcc (iN (bitcast (vNi1 X))), 0, eq` => `setcc (iN zext(i1 vecreduce_or (vNi1 X))), 0, eq`? which is to say you just need to change LHS to replace the vector->scalar bitcast with a reduction.


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

https://reviews.llvm.org/D130163