[PATCH] D57779: [SLP] Add support for throttling.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

Will it work if the same treeentry is used several times in the tree? For example, in diamond merge? If the treeentry, used several times is not profitable?



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2949
+void BoUpSLP::recordSeeds(ArrayRef<Value *> Ops) {
+  Seeds.push_back(SmallVector<Value *, 8>(Ops.begin(), Ops.end()));
+}
----------------
dtemirbulatov wrote:
> ABataev wrote:
> > `emplace_back(Ops.begin(), Ops.end())`?
> Hmm, it does not work out of ArrayRef without specifying the constructor.
And just `emplace_back(Ops)`?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3764-3765
+    if (!TE.NeedToGather) {
+      for (Value *V : TE.Scalars)
+        ScalarsToVec.insert(V);
+    }
----------------
dtemirbulatov wrote:
> ABataev wrote:
> > Use `llvm::copy`
> I could not use llvm::copy here since Scalars is SmallVector and ScalarsToVec declared as SmallPtrSet type.
Then just `ScalarsToVec.insert(TE.Scalars.begin(), TE.Scalars.end());`?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2912-2913
+        ExternalUses.erase(
+            std::remove_if(ExternalUses.begin(), ExternalUses.end(),
+                           [&V](ExternalUser &EU) { return EU.Scalar == V; }),
+            ExternalUses.end());
----------------
Use `llvm::remove_if`


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3772
     if (TE.NeedToGather &&
-        std::any_of(
-            std::next(VectorizableTree.begin(), I + 1), VectorizableTree.end(),
-            [TE](const std::unique_ptr<TreeEntry> &EntryPtr) {
-              return EntryPtr->NeedToGather && EntryPtr->isSame(TE.Scalars);
-            }))
+        std::any_of(std::next(VectorizableTree.begin(), TE.Idx + 1),
+                    VectorizableTree.end(),
----------------
Use `llvm::any_of(llvm::drop_begin(VectorizableTree, TE.Idx + 1), ...)`


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

https://reviews.llvm.org/D57779