[PATCH] D98714: [SLP] Add insertelement instructions to vectorizable tree

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2883
+
+      if (IsConsecutive) {
+        TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx);
----------------
anton-afanasyev wrote:
> ABataev wrote:
> > ABataev wrote:
> > > anton-afanasyev wrote:
> > > > ABataev wrote:
> > > > > anton-afanasyev wrote:
> > > > > > ABataev wrote:
> > > > > > > If not consecutive, we can include the cost of single permute and still vectorize it, no?
> > > > > > Inserts here are coming from `findBuildAggregate()`, being already sorted by index and therefore consecutive, the only exclusion is that they can have gaps, so we check it here. I don't think we should process this rare case within this patch.
> > > > > Can we check for the gaps during the analysis of the build vector sequence instead?
> > > > I've decided to check it here since we can get other sources of incoming insert bundle in future.
> > > The no need to iterate through the whole list, use early exit out of loop with cancelled scheduling and gather nodes.
> > Not done, you can early exit out of the loop if the non consecutive insert is found:
> > ```
> >       int Offset = *getInsertIndex(VL[0], 0);
> >       ValueList Operands(VL.size());
> >       Operands[0] = cast<Instruction>(VL[0])->getOperand(1);
> >       for (unsigned I = 1, E = VL.size(); I < E; ++I) {
> >         if (I != *getInsertIndex(VL[I], 0) - Offset) {
> >           LLVM_DEBUG(dbgs() << "SLP: skipping non-consecutive inserts.\n");
> >           BS.cancelScheduling(VL, VL0);
> >           buildTree_rec(Operands, Depth, UserTreeIdx);
> >           return;
> >         }
> >         Operands[I] = cast<Instruction>(VL[I])->getOperand(1);
> >       }
> >       TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx);
> >       LLVM_DEBUG(dbgs() << "SLP: added inserts bundle.\n");
> >       TE->setOperand(0, Operands);
> > 
> >       ValueList VectorOperands;
> >       for (Value *V : VL)
> >         VectorOperands.push_back(cast<Instruction>(V)->getOperand(0));
> > 
> >       TE->setOperand(1, VectorOperands);
> > 
> >       buildTree_rec(Operands, Depth + 1, {TE, 0});
> >       return;
> > ```
> This code is not what intended, since we early exit building tree with uncompletely filled `Operands`.
I meant `newTreeEntry(VL, None /*not vectorized*/, S, UserTreeIdx);`, just like we're doing for other nodes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98714