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

[Dinar Temirbulatov via Phabricator via llvm-commits]

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


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2776
+
+    buildTree(S);
+
----------------
vporpo wrote:
> buildTree() is an expensive operation because of scheduling. We should not build a new tree if we can reuse the existing one.
> 
I think it is the dependency analysis is expensive, when certain seeds were just ignored, I could get the numbers. But here, we have already correct seed from the dependency analysis point of view.  hmm, We could turn off the dependancy check for our collected seeds.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3530
+
+  // Walk across all leaves in order to reduce their paths. If we encounter
+  // another branch on the way from our leaf then consider the path from given
----------------
vporpo wrote:
> Collecting all paths is expensive, and I think we should not be doing this here just for removing some nodes from the tree. Also, you will end up visiting the same nodes over and over again (the ones close to the root). As I had described in a previous comment, I think we could get a good-enough tree cutting with a single top-down traversal of the tree. Did you find any examples where that would not work?
>Also, you will end up visiting the same nodes over and over again (the ones close to the root).
Hmm, It should not visit, multi-times the same node. I will check again.
>Did you find any examples where that would not work?
yes, Many, it is definitely better with tree traverse.


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

https://reviews.llvm.org/D57779