[PATCH] D141924: [IR] Add new intrinsics interleave and deinterleave vectors

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

A couple of minor requests but otherwise the patch looks good.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:11569
+  SDValue Lo = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, OutVT, InVec,
+                           DAG.getConstant(0, DL, MVT::i64));
+  SDValue Hi = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, OutVT, InVec,
----------------
It's better to use `getVectorIdxConstant` here.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:11571
+  SDValue Hi = DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, OutVT, InVec,
+                           DAG.getConstant(OutNumElts, DL, MVT::i64));
+
----------------
As above.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1211-1212
       setOperationAction(ISD::EXTRACT_VECTOR_ELT, VT, Custom);
+      setOperationAction(ISD::VECTOR_DEINTERLEAVE, VT, Custom);
+      setOperationAction(ISD::VECTOR_INTERLEAVE, VT, Custom);
     }
----------------
No change required, the following is just an observational comment.

I just wanted to highlight there are no tests for the `MVT::nxv1i1` cases, which I'm guessing triggers an isel failure as there's as yet no support for the zip and uzp nodes for this type?  Given legalisation for these nodes will follow this patch I'm assuming there isn't a route that is crash free today.


================
Comment at: llvm/test/CodeGen/AArch64/fixed-vector-deinterleave.ll:43
+
+define {<8 x half>, <8 x half>} @vector_deinterleave_v8f16_v16f16(<16 x half> %vec) {
+; CHECK-LABEL: vector_deinterleave_v8f16_v16f16:
----------------
Can you move this function before `vector_deinterleave_v2f32_v4f32` to keep the related element types together.


================
Comment at: llvm/test/CodeGen/AArch64/sve-vector-deinterleave.ll:40
+
+define {<vscale x 8 x half>, <vscale x 8 x half>} @vector_deinterleave_nxv8f16_nxv16f16(<vscale x 16 x half> %vec) {
+; CHECK-LABEL: vector_deinterleave_nxv8f16_nxv16f16:
----------------
Can you move this function before `vector_deinterleave_nxv2f32_nxv4f32` to keep the related element types together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141924