[PATCH] D67841: [SLP] avoid reduction transform on patterns that the backend can load-combine

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 16 04:49:59 PDT 2019


spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:1698
+  ///       may not be necessary.
+  llvm::Optional<int> getLoadCombineCost(Value *V) {
+    using namespace llvm::PatternMatch;
----------------
ABataev wrote:
> Shall we just terminate the reduction of this special construct unconditionally? Do we need this cost calculation just to prevent the reduction all the time we see this pattern? If yes, then, probably, we don't need to calculate the cost.
> There is a function `isTreeTinyAndNotFullyVectorizable()`. Can you put pattern matching analysis for this particular construct in this function without any additional cost analysis?
I'm not opposed to ignoring costs for this patch. It's makes the patch simpler (although less flexible since targets can't then override via cost model).

I don't think we should put this directly into isTreeTinyAndNotFullyVectorizable() though. In this case, the tree may not be tiny, and it may be fully vectorizable, so that would be wrong on both counts. :)

I think the first revision of the patch was already close in spirit to this suggestion - it modified SLP alone rather than the cost model. Let me rework that, and we'll see if that looks better.


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

https://reviews.llvm.org/D67841





More information about the llvm-commits mailing list