[PATCH] D38586: SLPVectorizer.cpp: Ensure SLPVectorizer can visit each block by dominated order

[Daniel Berlin via llvm-commits]

I would just sort by dominator dfs numbers

On Sat, Oct 7, 2017, 11:24 PM Davide Italiano via Phabricator <
reviews at reviews.llvm.org> wrote:

> davide requested changes to this revision.
> davide added a comment.
> This revision now requires changes to proceed.
>
> hmmm, I'm still not entirely convinced, and I'd like somebody who knows
> the vectorizer to chime in.
> This is a NFC change, at least in theory, from what I understand (feel
> free to prove me wrong :)
> The code as it was was sorting the CSE blocks in dominator order (and
> visiting them in function order), now, you're visiting them in RPO and not
> sorting them.
> About the first change, I'm not sure what are the effects. Probably if the
> vectorizer is formulated as a forward data flow problem this isn't
> necessarily a bad thing (in general, visiting BBs in function order is not
> guaranteed to be optimal, that's why we have other visiting schemes).
>
> The second part is the one that worries me. Dominator tree order and RPO
> aren't strictly the same, e.g. when you have a basic block with multiple
> successors, as far as I can tell.
> If you don't need dominator ordering and you can get away with RPO, fine,
> but you should at least explain why (and add a comment).
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D38586
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171008/dcb0d98f/attachment.html>