[PATCH] D133441: [SLP] Look ahead for mutual horizontal reductions.

Mon Sep 12 12:25:14 PDT 2022

vporpo added a comment.

In D133441#3782167 <https://reviews.llvm.org/D133441#3782167>, @labrinea wrote:

>>> One solution could be to collect the seeds that resulted in failed trees due to external users (and perhaps filter them more by checking if the users are actually seeds), and revisit the seeds once again. I think this is more or less what Alexey is describing.
>>
>> Yep, exactly! At least looks so :)
>
> I am not sure how would I revisit the seeds though without changing the code. Also can you clarify what do you mean by seeds in this context? The root instruction of the reduction, the list of reduced values passed to buildTree, something else?

This should require code changes in at least a couple of places:

1. Checking that a tree failed to vectorize because of external uses (changes mainly in`getTreeCost()`).
2. Collecting the roots that failed to vectorize because of external uses (these are the "seeds" that I am referring to). This should require changes after the invocations of `getTreeCost()` and the checks against `SLPCostThreshold`.
3. Adding some logic to retry vectorizing the missed sseds. This is probably the trickiest part because we don't really know when it is best to retry vectorizing these trees (unless perhaps we also keep track of when the external uses get vectorized?) . I was having a similar issue with the other patch: the vectorizer would succeed vectorizing the code 2-wide, before I would retry vectorizing from the missed 4-wide seed, which resulted in worse than optimal code. But anyway, retrying to vectorize such missed opportunities is definitely better than not retrying at all.

What you don't need to change is the cost calculation for external uses, as in this way the vectorizer is already calculating the costs correctly for you, as this time the uses will have already been vectorized. I think this is what Alexey meant with his comment (line 11778).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133441