[PATCH] D98714: [SLP] Add insertelement instructions to vectorizable tree

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2555
+    if (Entry->State == TreeEntry::MayBeRemoved) {
+      assert(UserIgnoreList.size() == 0 &&
+             "UserIgnoreList should be empty when roots are inserts");
----------------
`UserIgnoreList.empty()`


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2627
   if (S.OpValue->getType()->isVectorTy()) {
-    LLVM_DEBUG(dbgs() << "SLP: Gathering due to vector type.\n");
-    newTreeEntry(VL, None /*not vectorized*/, S, UserTreeIdx);
-    return;
+    if (isa<InsertElementInst>(VL[0])) {
+      ValueList Operands;
----------------
What if some of the instructions are not insertelements?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2634-2644
+        bool FoundMainInsert = false;
+        for (auto *Insert : VL)
+          if (llvm::any_of(Insert->users(), [&](Value *U) {
+                return !llvm::any_of(VL, [&](Value *S) { return U == S; });
+              })) {
+            if (FoundMainInsert) { // Main insert should be unique
+              FoundMainInsert = false;
----------------
Isn't it the first instruction in the list?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2654
+      }
+      buildTree_rec(Operands, Depth, UserTreeIdx);
+      return;
----------------
Why do we ignore user vector instructions here? Also, is this supposed only for extractelements operands?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2656
+      return;
+    } else {
+      LLVM_DEBUG(dbgs() << "SLP: Gathering due to vector type.\n");
----------------
No need for `else` here


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5094
+      Value *MainInsert = Entry->VectorizedValue;
+      Value *Vec = getTreeEntry(Entry->getOperand(0)[0])->VectorizedValue;
+      MainInsert->replaceAllUsesWith(Vec);
----------------
assert for `Vec`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98714