[PATCH] D45858: [DivRemPairs] Fix non-determinism in use list order.

[Mandeep Singh Grang via Phabricator via llvm-commits]

mgrang added a comment.

In https://reviews.llvm.org/D45858#1076816, @spatel wrote:

> In https://reviews.llvm.org/D45858#1075653, @gberry wrote:
>
> > In https://reviews.llvm.org/D45858#1073565, @spatel wrote:
> >
> > > That looks ok, but I'm curious;
> > >
> > > 1. How do you discover this problem?
> >
> >
> > I ran into this when testing a totally unrelated change that I expected to have no effect on final code generation.  When it generated different code for one llvm-test-suite test case, I investigated why, and it led me here.
> >
> > > 2. Is it possible to make the problem visible in a regression test (if so, please add a test)?
> >
> > I don't think so, but maybe.  The problem is that before this change, the DivRemPairs pass will generate non-deterministic use-list orderings.  I could write a test that dumps the use-list info and check that, but that seems pretty fragile.
>
>
> The answer to #1 suggests that we could reduce a codegen test based on whatever wiggled in test-suite? I don't know how much effort that requires, so if it doesn't seem worthwhile, I won't hold up this patch. I just want to know what I should do if I run into a similar problem. Also, @mgrang is the answer about the reverse-iteration-bot sufficient? (Again, I don't know anything about it, so trying to understand better.)


A quick background on the reverse-iteration bot: https://llvm.org/docs/CodingStandards.html#beware-of-non-determinism-due-to-ordering-of-pointers
Unordered containers like SmallPtrSet and DenseMap do not guarantee the order of iteration of elements. So iterating them can result in non-deterministic codegen. However, not all instances of iterating an unordered container result in non-determinism. Given this, it becomes difficult to identify which iteration instances can cause this behavior. The reverse iteration bot iterates such containers in reverse, by default, to weed out such cases.

However, the bot is only useful only if there are tests which stress/depend on iteration order. And while it may be possible to write such tests I am not sure if it's worth the effort. My experience also has been that I have randomly run into non-deterministic behavior and debugging led me to problems in iteration order.

So I think this patch LGTM.

> I just want to know what I should do if I run into a similar problem.

You can build your toolchain with LLVM_REVERSE_ITERATION:ON and see if your test still fails. If it does then it suggests a problem in iteration order of either SmallPtrSet or DenseMap.
>From http://llvm.org/docs/CMake.html?highlight=cmake:

  LLVM_REVERSE_ITERATION:BOOL
  If enabled, all supported unordered llvm containers would be iterated in reverse order. This is useful for uncovering non-determinism caused by iteration of unordered containers.


Repository:
  rL LLVM

https://reviews.llvm.org/D45858