[PATCH] D94444: [RFC][Scalable] Add scalable shuffle intrinsic to extract evens from a pair of vectors

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added a comment.

In D94444#2492146 <https://reviews.llvm.org/D94444#2492146>, @sdesmalen wrote:

> Thanks for creating this patch!
>
>> I chose to extract the even elements from a pair of vectors (full vector result), rather than a single vector (1/2 width vector result). This is in line with existing fixed shuffle vectors. And can be extended to accept an undef argument if needed. The motivation behind this decision was that we'd want the result vector to be a full vector for performance reasons. It would also map well to SVE's LD2 and UZP1.
>
> Are you also planning to add intrinsics for interleaving?

I am, plus some others for Complex vectorization. I just wanted to work out the kinks with this first example.

>> How do we feel about the intrinsic name: llvm.experimental.vector.extract.evens(...)?
>
> I quite like the `odd/even` terminology, but would prefer to drop the `s`, as in: "extracting the even elements".

I like that too. I also like David's suggestion about deinterleave.even. I don't really have a strong opinion on either though. Does anyone feel strongly either way?

>> How do we feel about the ISDNode name: ISD::EXTRACT_EVENS_VECTOR? It could be argued that this set of nodes should have SCALABLE in their names, unless we plan to also allow fixed width arguments as well. Currently the fixed width intrinsics are canonicalized to the existing shuffle vector implementation, so they never reach this ISDNode.
>
> I would favour not to add SCALABLE to that name, because there is nothing limiting these nodes from being used for fixed-width vectors.

Ok, that's fair. Right now I have an assert in ISelLowering to ensure only scalable types. That could really be removed though, since the UZP1 lowering would also work for fixed types. It might take a little work to clean up, but I don't foresee any problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94444