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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
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())
----------------
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.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4188
+      if (It == TE.Scalars.begin())
+        return {};
+      auto *Ty = FixedVectorType::get(TE.Scalars.front()->getType(), Sz);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144689