[PATCH] D146212: [AArch64] Use NEON's tbl1 for 16xi8 build vector with mask.

[Lawrence Benson via Phabricator via llvm-commits]

lawben added a comment.





================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10218
+  // TBL1.
+  if (VT.getSimpleVT() != MVT::v16i8)
+    return SDValue();
----------------
fhahn wrote:
> Why limit this it v16i8?
Good point. I've added v8i8 too. I only had the v16i8 use case in mind when writing this. This also works for other vector sizes, but that requires some additional logic to multiply and and byte offsets, as TBL only supports v16i8 and v8i8 directly. I've seen some of that around the code and I think x86 instruction selection also does this. If you think this is valuable and the right place to do it, I'd be happy to implement that in a follow-up patch.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10219
+  if (VT.getSimpleVT() != MVT::v16i8)
+    return SDValue();
+
----------------
fhahn wrote:
> It looks like we are missing test coverage for most (all?) exits. It would be great if you could extend the test coverage.
I've added a few tests to cover the exits. Please let me know if you think something is missing. I also noticed that some of my assumptions were too pessimistic in the process, so I removed them. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146212