[PATCH] D119623: [SLP] Simplify indices processing for insertelements

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D119623#3317456 <https://reviews.llvm.org/D119623#3317456>, @anton-afanasyev wrote:

> In D119623#3316827 <https://reviews.llvm.org/D119623#3316827>, @ABataev wrote:
>
>> What kind of bugs do you see with these test cases and current version of the function? I would try to keep it, better to generate UndefMaskElem if possible, rather than cut of the vectorization early.
>
> All these test cases are crashing without patch. There are several issues with this code. First of all, see lines 4028 and 4053 -- we use already dereferenced `Optional<int> Idx = *getInsertIndex(V);` (it should be without `*`). Also `MinIdx` is computed but not used at all.
>
> But the main problem is that `UndefMaskElem` generally makes no sense for index: the function `getInsertIndex()` returns _index_ rather than element, if it is undef/poison/out of range then whole vector is poison. Also this return value `UndefMaskElem` isn't used now anywhere, if somehow (how?) it could be useful in future, it can be added.

Ok, got it. Could you also add a check to the buildvector detection function for such kind of insertelement (+ possibly insertvalue) instructions? They should not be treated as a part of buildvector, need to treat them as an initial/terminating instruction and exclude from possibly vectorizable buildvector instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119623