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

[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:2555
+    if (Entry->State == TreeEntry::MayBeRemoved) {
+      assert(UserIgnoreList.size() == 0 &&
+             "UserIgnoreList should be empty when roots are inserts");
----------------
ABataev wrote:
> `UserIgnoreList.empty()`
Ok, thanks.


================
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;
----------------
ABataev wrote:
> Isn't it the first instruction in the list?
Not necessary. `findBuildAggregate()` sorts instructions by insert index, but main insert could have any one. Main insert is the only insert among others which has external user.
I'm to add comment here to be more clear.


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


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98714