<p dir="ltr">I would just sort by dominator dfs numbers</p>
<br><div class="gmail_quote"><div dir="ltr">On Sat, Oct 7, 2017, 11:24 PM Davide Italiano via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">davide requested changes to this revision.<br>
davide added a comment.<br>
This revision now requires changes to proceed.<br>
<br>
hmmm, I'm still not entirely convinced, and I'd like somebody who knows the vectorizer to chime in.<br>
This is a NFC change, at least in theory, from what I understand (feel free to prove me wrong :)<br>
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.<br>
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).<br>
<br>
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.<br>
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).<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D38586" rel="noreferrer" target="_blank">https://reviews.llvm.org/D38586</a><br>
<br>
<br>
<br>
</blockquote></div>