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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp:3116
+            Builder.SetInsertPoint(I->getParent(),
+                                   std::next(I->getIterator()));
+          else
----------------
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?


Repository:
  rL LLVM

https://reviews.llvm.org/D49928