[PATCH] D135596: [AArch64] Canonicalize ZERO_EXTEND to VSELECT

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

A couple of nits but it looks like the move to lowering has caused some side effects whereby we now return "Invalid cost" for predicate based llvm.experimental.vector.splice.nxv16i1. I think we're just missing some entries in `AArch64TTIImpl::getCastInstrCost` like

  { ISD::ZERO_EXTEND, MVT::nxv2i64, MVT::nxv2i1, 1 },
  { ISD::ZERO_EXTEND, MVT::nxv4i32, MVT::nxv4i1, 1 },
  { ISD::ZERO_EXTEND, MVT::nxv8i16, MVT::nxv8i1, 1 },
  { ISD::ZERO_EXTEND, MVT::nxv16i8, MVT::nxv16i1, 1 },



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5571
+  SDValue Zeros = DAG.getConstant(0, DL, VT);
+  return DAG.getNode(ISD::VSELECT, SDLoc(N), N->getValueType(0), Op, Ones,
+                     Zeros);
----------------
Can use `DL, VT` here as above.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5787-5788
+  case ISD::ZERO_EXTEND:
+    if (Op.getValueType().isFixedLengthVector())
+      return LowerFixedLengthVectorIntExtendToSVE(Op, DAG);
+    else
----------------
Please can you move these two lines into `LowerZERO_EXTEND` so all the zext lowering has the same entry point. 


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:515
   // zext(cmpeq(x, splat(0))) -> cnot(x)
-  def : Pat<(nxv16i8 (zext (nxv16i1 (AArch64setcc_z (nxv16i1 (SVEAllActive):$Pg), nxv16i8:$Op2, (SVEDup0), SETEQ)))),
+  def : Pat<(nxv16i8 (vselect (nxv16i1 (AArch64setcc_z (nxv16i1 (SVEAllActive):$Pg), nxv16i8:$Op2, (SVEDup0), SETEQ)), (nxv16i8 (splat_vector (i32 1))), (nxv16i8 (splat_vector (i32 0))))),
             (CNOT_ZPmZ_B $Op2, $Pg, $Op2)>;
----------------
Does `(SVEDup0)` work here as a shortcut?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135596