[PATCH] D94883: [CodeGen][SelectionDAG]Add new intrinsic experimental.vector.reverse

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D94883#2518090 <https://reviews.llvm.org/D94883#2518090>, @paulwalker-arm wrote:

> In D94883#2515136 <https://reviews.llvm.org/D94883#2515136>, @fhahn wrote:
>
>> Taking a step back, are there plans to use the intrinsics for fixed vectors?
>
> I'd like to say yes but at this stage it is too early to say.  What we definitely need is a unified interface so that transforms can be written without needing to worry about the type of vector.  If a pass uses an IRBuilder then great, we can hide the "do I create a shufflevector or intrinsic" code behind suitably names functions (e.g. craeteVectorSplice()...).  For passes that want to create a node directly then we'd recommend just creating an intrinsic call with the expectation that those working with fixed length vectors are transformed to shufflevector sufficiently early to maintain existing code quality.

Hm, making things slightly easier for passes not using `IRBuilder` doesn't seem like the strongest motivation to me, especially if it also comes with new pass ordering constraints (doing the conversion during instruction selection seems like it would mean we potentially miss existing folds in `InstCombine` & co). Also, even if passes not using `IRBuilder`, creating an intrinsic call without it is probably more work than just instantiating `IRBuilder` and using it directly?

It seems to me like only allowing them for scalable vectors initially would define away a couple of potential problems (like introducing new pass-ordering constraints, uncertainty whether to use `shuffle vector` or the intrinsics). Personally I'd prefer  one  way of doing things  rather than having to chose whether to use an intrinsic or a shuffle for fixed vectors. If it is uncertain whether the intrinsic will actually be used for fixed vectors, having it first restricted to the actual use case (scalable vectors) and extend it to fixed vectors once it is needed seems like a slightly more incremental/conservative approach.

But if others think having them for fixed with vectors is useful, that's fine by me, especially because they are still experimental and we can remove fixed vector support again if it is not used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94883