[PATCH] D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9042-9045
+  ElementCount EC = VT.getVectorElementCount();
+  unsigned IntElSize = AArch64::SVEBitsPerBlock / EC.getKnownMinValue();
+  MVT IntElType = MVT::getIntegerVT(IntElSize);
+  MVT VecVT = MVT::getVectorVT(IntElType, EC);
----------------
Is it possible to use getPromotedVTForPredicate here?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9050
+  MVT IndexVecType = IntElSize == 64 ? MVT::i64 : MVT::i32;
+  StepVal = DAG.getZExtOrTrunc(StepVal, dl, IndexVecType);
+  SDValue Zero = DAG.getConstant(0, dl, IndexVecType);
----------------
I can imagine a route where this promotion is also required for the `ElemVT != MVT::i1` case.  Related to this I suspect we'll need an implementation of `PromoteIntOp_STEP_VECTOR` but currently don't because there's nothing that can exercise the `Step != 1` case.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10215
     SDValue And = DAG.getNode(ISD::AND, dl, OpVT, Op.getOperand(0), One);
+    if (VT.isScalableVector()) {
+      SDValue Pg = getPTrue(DAG, dl, VT, AArch64SVEPredPattern::all);
----------------
Is this test needed? I'm guessing this code was added as part of SVE support (v#i1 being an illegal type for NEON).


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

https://reviews.llvm.org/D97299