[PATCH] D102834: [SLPVectorizer] WIP Implement initial memory versioning (WIP!)

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D102834#2802402 <https://reviews.llvm.org/D102834#2802402>, @fhahn wrote:

> In D102834#2771053 <https://reviews.llvm.org/D102834#2771053>, @ABataev wrote:
>
>> You can easily get the first instruction - `VectorizableTree.front()`. Stores can be only in the front of the vectorizable tree. As to the last instruction, I think we need to scan all tree entries and find all gather nodes with memaccess which may alias with stores. The main problem that the versioning better to perform before we try to build the tree, otherwise we may decide to gather some instructions instead of trying vectorizing them because of possible aliasing. And it may not be profitable.
>
> I updated the patch to only collect possible bounds for versioning first and queue basic blocks for which we found bounds for re-processing.  When re-processing those blocks, we create a versioned block with the appropriate `!noalias` metadata and re-run vectorization (for now just seeded by stores). As as, this should be correct, but may not be optimal, because either:
> a) there were issues preventing vectorization other than aliasing, which may cause us to not vectorize anything in the versioned block
> b) the runtime checks are too expensive and offset the gain from additional vectorization.
>
> We should be able to solve both by comparing the cost of the versioned & non-versioned BB after vectorization. If versioning is not profitable, we can remove the conditional branch again. What do you think of this direction?

Yeah, looks like this is the only way for now. Should be much easier to do with VPlan. Also, I would look at the instruction scheduling. Most probably, need to schedule the instructions more aggressively to improve locality and avoid regressions.

> In D102834#2793483 <https://reviews.llvm.org/D102834#2793483>, @SjoerdMeijer wrote:
>
>> Hi Florian, thanks for putting this up for review and starting the discussion. If you don't mind me asking, how high is this on your todo-list? The reason I am asking is that this well help x264 where we are behind a lot (and it more in general it solves this long outstanding issue that we have). Fixing x264 is high on our list, which is why I put up D102748 <https://reviews.llvm.org/D102748> to start this discussion. If, for example, you don't see time to work on this, we could consider looking at it.
>
> It's fairly high on my todo list, as we have quite a number of of such cases reported by our internal users. But I'm not planning to rush and I think we should also start collecting micro-benchmarks for the missed opportunities, so we can thoroughly evaluate the impact  and do not introduce regressions in the future.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102834