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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2458
+
+  if (ScalarizeAt == (VectorizableTree.size() - 1) || ScalarizeAt == 0)
+    return;
----------------
dtemirbulatov wrote:
> ABataev wrote:
> > Hmm, will it work with the graph? Seems to me, it will work absolutely correctly only with the list and might not be quite correct even for tree. VectorizableTree is actually a linearized graph and the index is not connected anyhow with the tree/graph depth/height.
> > You need to nalyze each particular subgraph/subtree independently, and thus, cannot rely on the single index.
> Looks like it works correct to me. It does not need any particular index, it just relies on "Entry->NeedToGather" variable  at line 1313. And it works    consequently, not at once in the end. We found chain of operations to vectorize, then we call getTreeCost() and inside at 2805 invoke ViewGraph and, at that time, NeedToGather is already set by cutTree for our chain members.
I don't think it works absolutely correctly. You could get better results if you would implement correct tree/graph throttling. YOu cannot rely on index of the TreeEntry in your case, you need follow the tree structure.


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

https://reviews.llvm.org/D57779