[PATCH] D33265: [PredicateInfo ] Fix non-determinism in codegen uncovered by reverse iterating SmallPtrSet

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Thu May 18 11:24:09 PDT 2017


Yes, it looks like that is what would have to be done this second,.

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.
See https://reviews.llvm.org/D32720 for the code.

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.


As for reverse-iterate, sure, once we get this done, i don't have an issue
adding it to the tests.





On Thu, May 18, 2017 at 10:52 AM, Mandeep Singh Grang via Phabricator <
reviews at reviews.llvm.org> wrote:

> mgrang added a comment.
>
> Thanks @davide and @dberlin for your comments.
>
> 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.
> Or would it be simpler to just keep the comparator inline as you do in
> your code?
>
> 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)?
>
>
> https://reviews.llvm.org/D33265
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170518/ceb72d51/attachment.html>


More information about the llvm-commits mailing list