[PATCH] D39245: [ADT] Shuffle containers before sorting to uncover non-deterministic behavior

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

As with Davide, my argument was mostly "use an existing bucket" not "it should be in bucket X". =]

I'm happy with this being under NDEBUG if this doesn't break ABI. As long as shuffle is O(N), adding an O(N) check to an O(N * log(N)) algorithm seems totally fine for NDEBUG, so I'd rather not stuff it behind the "expensive checks" macro.

In https://reviews.llvm.org/D39245#908745, @mgrang wrote:

> I guess +asserts and -asserts builds should be comparable in terms of ninja check failures. I would prefer not to add these under ABI_BREAKING. How about we add these either under EXPENSIVE_CHECKS or rename LLVM_REVERSE_ITERATION as LLVM_NON_DETERMINISM_CHECKS and add these under that?
>
> Btw .. I ran shuffle+sort with all targets enabled and see 39 ninja check failures (previously I had built only ARM/AArch64 targets).


The suggestion is that we should fix these failures before we land this unless there is some reason that will be infeasible to do. That way we don't have new failures?


Repository:
  rL LLVM

https://reviews.llvm.org/D39245