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

[Anton Afanasyev via Phabricator via llvm-commits]

anton-afanasyev marked 3 inline comments as done.
anton-afanasyev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:9603
+        getInsertIndex(LastInsertInst, OperandOffset);
     if (!OperandIndex)
       return false;
----------------
ABataev wrote:
> anton-afanasyev wrote:
> > ABataev wrote:
> > > anton-afanasyev wrote:
> > > > @ABataev Do you mean this check?
> > > Yes, but I thought about returning `true` here.
> > Hmm, but `true` means that this insertelement is a part of buildvector. Do you want this?
> True means that we have a buildvector, but do not include this insertelement into the list of the vectorisable insertelements, i.e. treat it as an initial vector of the buildvector. Could you try this?
Ok, I see.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:9609
                                   BuildVectorOpds, InsertElts, *OperandIndex))
         return false;
     } else {
----------------
ABataev wrote:
> I mean, maybe return true here too, if we have at least 2 insertelements in the InsertElts vector?
I've simplified this a bit more, removed boolean return type of function at all, since "at least 2 insertelements" condition has already been checking at the non-recursive wrapper `findBuildAggregate()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119623