[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:527
   /// A negative number means that this is profitable.
-  int getTreeCost();
+  int getTreeCost(bool ReduceTree = false);
 
----------------
vporpo wrote:
> Could you use a cl::opt flag instead of this ReduceTree function argument?
ok, true by default.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2683
+      // values for canceled elements use.
+      if (InternalTreeUses.find(EU.Scalar) != InternalTreeUses.end())
+        for (ExternalUser &IU : InternalTreeUses[EU.Scalar])
----------------
vporpo wrote:
> As far as I understand, you are calculating the additional cost for the EU.Scalars that are no longer vectorized (because of reduceTree()).
> So you are getting all its in-tree uses and you are calculating the extract operation cost from for the data transfer from EU.Scalar->vector use.
> 1. Isn't this an 'insert' operation and not an 'extract' ?
> 2. Isn't this just the gather cost that has already been considered in line 2756 ?
> 3. Even if you have multiple in-tree vectorized uses, we should only count the gather cost once, as the vector can then be reused for the rest of the uses with no extra cost.
Consider this example :
We considering to vectorize full tree height:
....
a0 = x*a;
a1 = x*b;
a2 = x+c;
a3 = x+d;
....
Then, later we decided to remove "a2", "a3" out of proposed to vectorized operations. And "x" variable is not available in a scalar form and in order to obtain "x" value we should extract "x" to be used in a scalar form. And while extracting "x" we should calculate the cost for such operation. It might be not profitable to vectorize such tree cut if we have to produce such an extract operation.
And for 2756 I have another example:
a0 = x*a;
a1 = x*b;
a2 = a0+c;
a3 = a1+d;
suppose that we would like to start vectorizaton from "a2", "a3" and "a0" and "a1" should be in scalar form we have calculated "c" and "d" as a gather operations, but since we are interrupting full-tree we have to consider calculating gather cost for "a0", "a1" because full intruppted full vectorization chain and "a0", "a1" are scalars now.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2748
+
+    if (ReduceTree || I == (E - 1)) {
+      Entry->ScalarizeAtCost = CostSum;
----------------
vporpo wrote:
> If ReduceTree is false, is the cost calculation correct (no  extract cost?) ? 
> 
> In any case if ReduceTree == false, I think it is better if we compute the cost as in the original code, without having to compute extractCost() etc. for every single 'cut' of the tree.
> 
> Also consider adding some of the costs as member variables in TreeEntry: like ExtractCost, SpillCost, GatherCost  etc.
> 
>If ReduceTree is false, is the cost calculation correct (no extract cost?) ?
yes, I think in this case we calculate the extract cost, for the uncut version with "I == (E - 1)" for full tree hight.


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

https://reviews.llvm.org/D57779