[PATCH] D74912: [AArch64][SVE] Add SVE2 intrinsics for bit permutation & table lookup

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar added a comment.

Cheers for working on this @kmclaughlin!



================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:2035
+
+def int_aarch64_sve_bdep_x : AdvSIMD_2VectorArg_Intrinsic;
+def int_aarch64_sve_bext_x : AdvSIMD_2VectorArg_Intrinsic;
----------------
What does `_x` mean here?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:1222
+                                          unsigned Opc) {
+  SDLoc dl(N);
+  EVT VT = N->getValueType(0);
----------------
`DL` ;-)


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:3592
+      if (VT == MVT::nxv16i8) {
+        SelectTableSVE2(Node, 2, AArch64::TBL_ZZZZ_B);
+        return;
----------------
`NumVecs` seems be always 2 in this patch. Will we need this to work for other values in the future too?

[Nit] `2` is a bit of a magic number here. What about `2` -> `/*NumVecs=*/2`


================
Comment at: llvm/test/CodeGen/AArch64/sve2-intrinsics-bit-permutation.ll:1
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve2,+sve2-bitperm -asm-verbose=0 < %s | FileCheck %s
+
----------------
AFAIK, `-asm-verbose=0` is not currently needed here (and you don't use it in the other test). There are 2 options:

* Leave `-asm-verbose=0` (guarantees that there are no comments in assembly) and additionally decorate every function that you define with `nounwind` (guarantees that no CFI directives are added). This way you can safely replace every instance of `CHECK`  with `CHECK-NEXT`.
* Remove `-asm-verbose=0` and leave things as they are.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74912