[PATCH] D49928: [SLP] Fix PR38339: Instruction does not dominate all uses!

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp:3116
+            Builder.SetInsertPoint(I->getParent(),
+                                   std::next(I->getIterator()));
+          else
----------------
efriedma wrote:
> ABataev wrote:
> > efriedma wrote:
> > > I don't think `std::next(I->getIterator())` is a valid insertion point in all cases.  V could be a phi, or an invoke, or some exception-handling instruction.
> > What if something like this:
> > ```
> > auto It = std::next(I->getIterator());
> > for (auto E = I->getParent()->end(); It != E; ++It) {
> >   if (!isa<PHINode>(&*It)) {
> >     if (It->isEHPad())
> >       ++It;
> >     break; 
> >   }
> > }
> > Builder.SetInsertPoint(I->getParent(), It);
> > ```
> > ?
> That doesn't really help... among other things, there are some blocks which have no legal insertion points.
> 
> Maybe there's some constraint on SLP vectorization which would help choose a better insertion point?  Why can't we just use VL0 as the insertion point?
Yes, maybe that's the best option here. Will fix this. 


Repository:
  rL LLVM

https://reviews.llvm.org/D49928