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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev 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;
----------------
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.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6481
+      } else if (SLPThrottling && R.findSubTree(UserCost))
+        R.saveTree();
     }
----------------
Better to enclose this substatement in braces to make the code uniform


================
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);
----------------
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?


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

https://reviews.llvm.org/D57779