[PATCH] D119536: [SLP] Extract intermediate insertelement for external use

[Anton Afanasyev via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:722
+  auto *LastInsert = cast<Instruction>(*find_if(Inserts, [Inserts](Value *V) {
+    return llvm::all_of(Inserts, [V](Value *VV) {
+      return V != cast<Instruction>(VV)->getOperand(0);
----------------
ABataev wrote:
> Drop `llvm:`
Done


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:730
+    SmallVector<int> Mask(NumElts);
+    std::iota(Mask.begin(), std::next(Mask.begin(), NumElts), 0);
+
----------------
ABataev wrote:
> ABataev wrote:
> > `std::next(Mask.begin(), NumElts)` -> `Mask.end()`
> Would be good to fill unused elements with UndefMaskElem instead, which may help to improve cost in the future for large vectors.
> std::next(Mask.begin(), NumElts) -> Mask.end()
Sure, thanks.

> Would be good to fill unused elements with UndefMaskElem instead, which may help to improve cost in the future for large vectors.
What do you mean by unused elements? We have to copy whole vector (with several replaced elements).


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:734
+    while (CurrentInsert != Insert) {
+      Optional<int> InsertIdx = getInsertIndex(CurrentInsert);
+      Mask[*InsertIdx] = *InsertIdx + NumElts;
----------------
ABataev wrote:
> What if `InsertIdx` is `UndefMaskElem`?
I forgot about it, thanks. But it may be reasonable to get rid of non-constant and undef indices at all, at `buildTree()` stage: https://reviews.llvm.org/D119623


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119536