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

[Dinar Temirbulatov via Phabricator via llvm-commits]

dtemirbulatov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3316
+          LLVM_DEBUG(dbgs() << "SLP: Checking user:" << *U << ".\n");
+          if (Tree->ScalarToTreeEntry.find(U) != Tree->ScalarToTreeEntry.end())
+            continue;
----------------
ABataev wrote:
> Looks like the `Scalar` should be extracted only if its user is vectorized and it remains to be scalar in the vectorized tree. Or it is not going to be vectorized.
Thanks, good catch. I think I need first to populate  ExternalUser as TreeEntires are mark with ProposedToGather and then change them to a NeedToGather node in the separate loop.


================
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:
> > > 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*>.
> Why is this a const iterator?
std::set iterators are bidirectional, not random-access. we have to use std::advance


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

https://reviews.llvm.org/D57779