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

[Davide Italiano via Phabricator via llvm-commits]

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