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

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 11 14:36:08 PST 2021


fhahn accepted this revision.
fhahn added a comment.

In D94883#2553696 <https://reviews.llvm.org/D94883#2553696>, @sdesmalen wrote:

> In D94883#2524665 <https://reviews.llvm.org/D94883#2524665>, @fhahn wrote:
>
>> 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.
>
> While these intrinsics are marked `experimental` they probably shouldn't be created for fixed-width vectors by passes or front-ends until we have a better understanding what direction to take for shuffles. At the same time, the name doesn't suggest the intrinsic is limited to scalable-vectors only, and it is quite trivial to make this work for fixed-width vectors, so I don't see a reason not to support this. Perhaps the LangRef description can mention this consideration?

Making it explicit in the langref sounds good to me, thanks!

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94883



More information about the llvm-commits mailing list