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

Kerry McLaughlin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 21 06:51:38 PST 2020


kmclaughlin marked 4 inline comments as done.
kmclaughlin added a comment.

Thanks for reviewing this, @andwar!



================
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;
----------------
andwar wrote:
> What does `_x` mean here?
_x indicates that this is an unpredicated intrinsic.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:3592
+      if (VT == MVT::nxv16i8) {
+        SelectTableSVE2(Node, 2, AArch64::TBL_ZZZZ_B);
+        return;
----------------
andwar wrote:
> `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`
I agree that it's not very clear what 2 is used for here. As NumVecs will always be the same value for the tbl2 intrinsic and SelectTableSVE2 is unlikely to be used for anything else, I've removed it from the list of parameters & added a comment there to explain the value used.


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

https://reviews.llvm.org/D74912





More information about the cfe-commits mailing list