<div dir="ltr"><div>Yes, it looks like that is what would have to be done this second,.</div><div><br></div><div>However Xin Tong has a patch that is being split up that encapsulates most of the functionality we need into a transform utility called OrderedInstructions.<br></div><div>See <a href="https://reviews.llvm.org/D32720">https://reviews.llvm.org/D32720</a> for the code.</div><div><br></div><div>If we split that off and commit it, i think we could use that (in both valuedfs_compare and the new code ) keep the comparator inline for now and i'll clean it up as part of a larger OrderedInstructions cleanup.</div><div><br></div><div><br></div><div>As for reverse-iterate, sure, once we get this done, i don't have an issue adding it to the tests.</div><div><br></div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 18, 2017 at 10:52 AM, Mandeep Singh Grang via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mgrang added a comment.<br>
<br>
Thanks @davide and @dberlin for your comments.<br>
<br>
Just wanted to confirm I understand this: In order to reuse the comparison function from ValueDFS_Compare in our case, we would have to move the localComesBefore function (or parts of it) out of ValueDFS_Compare so that both OBBMap and OpsToRename can use it as a comparator? ValueDFS_Compare constructor accepts BBs, so unless we first convert OpsToRename into ValueDFS (by calling our version of convertUsesToDFSOrdered) we cannot directly use it for sorting OpsToRename.<br>
Or would it be simpler to just keep the comparator inline as you do in your code?<br>
<br>
Also is it OK to add -reverse-iterate to the two unit tests as I have done in my patch (so that we catch any future non-determinism arising out of this piece of code)?<br>
<br>
<br>
<a href="https://reviews.llvm.org/D33265" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D33265</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>