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

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes marked 2 inline comments as done.
c-rhodes added inline comments.


================
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);
+    }
----------------
paulwalker-arm wrote:
> 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.
> 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.

Ah, well spotted. Replacing the inner bitcast with reduction was the original idea but I incorrectly figured reducing vector setcc was semantically equivalent and had slightly better codegen.



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

https://reviews.llvm.org/D130163