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

Dinar Temirbulatov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 7 09:04:31 PST 2020


dtemirbulatov marked an inline comment as not done.
dtemirbulatov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4297
+        Entry->State != TreeEntry::ScatterVectorize) || Entry->Cost <= 0 ||
+        !Entry->Idx)
+      continue;
----------------
anton-afanasyev wrote:
> dtemirbulatov wrote:
> > ABataev wrote:
> > > 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.
> > I think I can check here if scutter/gather is supported via TargetrInfo and if it is not then move all nodes with TreeEntry::ScatterVectorize to  TreeEntry::Gather.
> I believe it's wrong decision to check scatter/gather target support for the reason mentioned here https://reviews.llvm.org/D92701#2435573. Why could not we just rely on costs (node cost and total one)?
I agree with @anton-afanasyev here. I am not sure what @ABataev wants here? If I exclude (operands <= 1) then we would lose have of all tests in SLP affected by throttling.  


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

https://reviews.llvm.org/D57779



More information about the llvm-commits mailing list