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

[Dinar Temirbulatov via Phabricator via llvm-commits]

dtemirbulatov marked 7 inline comments as done.
dtemirbulatov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3286-3295
+  for (std::unique_ptr<TreeEntry> &TEPtr : Tree->VectorizableTree) {
+    TreeEntry *Entry = TEPtr.get();
+    if (Entry->State == TreeEntry::Vectorize)
+      VecNodes.push_back(Entry);
+  }
+  // Canceling unprofitable elements.
+  for (std::unique_ptr<TreeEntry> &TEPtr : Tree->VectorizableTree) {
----------------
ABataev wrote:
> These two loops can be merged, no? And use `switch` instead of `if`, if possible, after merging
Thanks.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6419
       int Cost = R.getTreeCost();
+      unsigned UserCost = 0;
       CandidateFound = true;
----------------
ABataev wrote:
> Do you really need this new var here? I don't see where it is used except as an argument of `R.findSubTree(UserCost)` call
I think we need to compensate the ExctractCost with that cost of the insert sequence as in case of full-vectorization.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4108-4111
+    int i = 0;
+    for (auto It = Vec.begin(), E = Vec.end(); It != E; ++It, i++)
+      if (i>MaxCostsRecalculations)
+        Vec.erase(It);
----------------
ABataev wrote:
> dtemirbulatov wrote:
> > ABataev wrote:
> > > Just `Vec.erase(Vec.rbegin(), Vec.rbegin() + (Vec.size() - MaxCostsRecalculations)`?
> > No, We could not use "Vec.rbegin() + " with std::set.
> Then just `Vec.erase(Vec.begin() + MaxCostsRecalculations, Vec.end());`.
eh, no it is std::_Rb_tree_const_iterator<llvm::slpvectorizer::BoUpSLP::TreeEntry*>.


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

https://reviews.llvm.org/D57779