[PATCH] D113376: [AArch64][SVE] Lower shuffles to permute instructions: zip1/2, uzp1/2, trn1/2

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

Just a passing review I'm afraid.  I'll take a proper look tomorrow.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19567
+        DAG, VT,
+        DAG.getNode(AArch64ISD::ZIP1, DL, Op1.getValueType(), Op1, Op2));
+
----------------
I think you can use `ContainerVT`  here? It might help with the formatting.

Same goes for the other places where you use `Op1.getValueType()`.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19609-19618
+    if (isZIP_v_undef_Mask(ShuffleMask, VT, WhichResult) && WhichResult != 0)
+      return convertFromScalableVector(
+          DAG, VT,
+          DAG.getNode(AArch64ISD::ZIP2, DL, Op1.getValueType(), Op1, Op1));
+
+    if (isUZP_v_undef_Mask(ShuffleMask, VT, WhichResult)) {
+      unsigned Opc = (WhichResult == 0) ? AArch64ISD::UZP1 : AArch64ISD::UZP2;
----------------
I've not thought about it deeply so might be wrong but given these cases only use `Op1` I'm wondering if they're always safe and thus don't need to be part of the `MinSVESize == MaxSVESize == VT.getSizeInBits()` restricted set?


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

https://reviews.llvm.org/D113376