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

Anton Afanasyev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 11 10:00:31 PDT 2021


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


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2883
+
+      if (IsConsecutive) {
+        TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx);
----------------
ABataev wrote:
> 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.
Hmm, `buildTree_rec()` here (with completely filled Operands) is intended: if we skip vectorization of non-consecutive inserts, we still try to vectorize starting from `Operands` (as it were before this patch). I think it's rare case when such operands could be successful seed for vectorizable tree, but why not to try?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98714



More information about the llvm-commits mailing list