[PATCH] D147243: [X86] MatchVectorAllZeroTest - add support for icmp(bitcast(icmp_ne(X,Y)),0) vector reduction patterns

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:24429
+                                  SelectionDAG &DAG, X86::CondCode &X86CC) {
+  SDValue Z = DAG.getConstant(0, DL, V.getValueType());
+  return LowerVectorAllEqual(DL, V, Z, CC, Mask, Subtarget, DAG, X86CC);
----------------
goldstein.w.n wrote:
> Is there an issue creating a node that may potentially unused?
There shouldn't be - it's just a constant node so has no dependencies to the rest of the DAG, and will get cleaned up later after the combine call. Although there's the remote chance that we reuse an existing node and spoil a hasOneUse check before cleanup - but on constants that's very unlikely.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:24503
+  // TODO: Expand to icmp(bitcast(icmp_eq(X,Y)),-1) patterns.
+  if (Op.getOpcode() == ISD::BITCAST && Mask.isAllOnes()) {
+    SDValue Src = Op.getOperand(0);
----------------
goldstein.w.n wrote:
> Maybe just `PeekThroughBitCast(Op).getOpcode() == ISD::SETCC`?
We'd need to repeat the PeekThroughBitCast call - is that clearer or not? Very minor tbh, I'm happy to do either.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:24509
+      ISD::CondCode SrcCC = cast<CondCodeSDNode>(Src.getOperand(2))->get();
+      if (SrcCC == ISD::SETNE) {
+        SDValue LHS = Src.getOperand(0);
----------------
goldstein.w.n wrote:
> Shouldnt there be a `Src.getOperand(1).isZeroConstant()`?
No, the inner comparison doesn't need to be a match against zero - its just the outer comparison that needs to match zero to form the reduction pattern - that's why its a LowerVectorAllEqual call and not LowerVectorAllZero like the existing patterns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147243