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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to