[PATCH] D57779: [SLP] Add support for throttling.
Alexey Bataev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 4 05:20:22 PST 2020
ABataev added inline comments.
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4290
+ auto Cmp = [](const TreeEntry *LHS, const TreeEntry *RHS) {
+ return LHS->Cost > RHS->Cost;
+ };
----------------
dtemirbulatov wrote:
> ABataev wrote:
> > Not sure that this is the best criterion. I think you also need to include the distance from the head of the tree to the entry, because some big costs can be compensated by the vectorizable nodes in the tree.
> > What I would do here is just some kind of level ordering search (BFS) starting from the deepest level.
> Hmm, implemented, but I don't see any benefit from that, plus we have to do BFS search. And we are going to throw away any non-vectorizable nodes at 4295.
It may trigger for targets like silvermont or in future for vectorized functions.
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4297
+ Entry->State != TreeEntry::ScatterVectorize) || Entry->Cost <= 0 ||
+ !Entry->Idx)
+ continue;
----------------
dtemirbulatov wrote:
> ABataev wrote:
> > I think you can also exclude entries with the number of operands <= 1.
> But why? The only thing that matters here is the cost.
Because the main idea is to drop gathers and drop one gather in favor of another one will not be profitable for sure. But it may improve compile time and the list of candidates, The only case you need to check for is the latest masked gather case, it may be profitable to convert it to gathers for some targets.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57779/new/
https://reviews.llvm.org/D57779
More information about the llvm-commits
mailing list