[PATCH] D144689: [SLP]Improve handling gathers/buildvectors with undefs.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie accepted this revision.
vdmitrie added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1449
         // Undefs are always profitable for extractelements.
         if (isa<UndefValue>(V2))
+          return (isa<PoisonValue>(V2) || isUndefVector(EV1).all())
----------------
ABataev wrote:
> vdmitrie wrote:
> > This needs a bit of explanation (a comment). 
> We can easily combine `poison and extractelement <non-poison>` or `undef and extractelement <poison>`. But combining `undef + extractelement <non-poison-but-my-produce-poison>` requires some extra operations and it is not very effective to combine such elements (to preserve the difference between undefs and poison), rather than extractelement from the same EV1, even in reversed order.
Thanks for explanation. I just meant to put it into the code as a comment.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4188
+      if (It == TE.Scalars.begin())
+        return {};
+      auto *Ty = FixedVectorType::get(TE.Scalars.front()->getType(), Sz);
----------------
ABataev wrote:
> vdmitrie wrote:
> > Could you please clarify the difference between returning empty container vs std::nullopt?
> > The description comment for getReorderingData method does not mention this distinction.
> std::nullopt means that the ordering is not important for the node, empty - prefer identity order. I'll add it to the description of the function.
Thanks. That is what I thought but wasn't absolutely sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144689