[PATCH] D71401: [AArch64][SVE] Add permutation and selection intrinsics

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:782
                  LLVMMatchType<0>,
                  llvm_i32_ty],
                 [IntrNoMem]>;
----------------
efriedma wrote:
> I guess this is sort of orthogonal to your patch, but this is missing ImmArg marking
Using `ImmArg` for this class opens up a can of worms, I know Kerry ran into similar issues in a separate patch. From what I understand it requires using `TImmLeaf` instead of `ImmLeaf` for the imm args in instruction definitions and some of those operands are used all over the AArch64 backend.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1182
 def int_aarch64_sve_sunpklo   : AdvSIMD_SVE_Unpack_Intrinsic;
-
+def int_aarch64_sve_tbl       : AdvSIMD_SVE_TBL_Intrinsic;
+def int_aarch64_sve_trn1      : AdvSIMD_2VectorArg_Intrinsic;
----------------
efriedma wrote:
> Any thoughts on naming tbl for one-register and two-register forms?
By two-register I assume you're referring to the SVE2 form which has two source registers? Downstream we name this `int_aarch64_sve_tbl2` and it's implemented with the same class as TBX.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:5040
+  def : SVE_3_Op_Pat<nxv4f32, op, nxv4i1, nxv4f32, nxv4f32, !cast<Instruction>(NAME # _S)>;
+  def : SVE_3_Op_Pat<nxv2f64, op, nxv2i1, nxv2f64, nxv2f64, !cast<Instruction>(NAME # _D)>;
 }
----------------
efriedma wrote:
> This unconditionally maps the "splice" intrinsic to the destructive form?  I guess that's fine.
Correct, SVE only defines a destructive form for splice, SVE2 introduced a constructive form which is implemented with a separate class but the ACLE defines no intrinsic for it.


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

https://reviews.llvm.org/D71401