[PATCH] D58385: [tools] Rewrite tests for symbol remapping to FileCheck

[Jonas Hahnfeld via Phabricator via llvm-commits]

Hahnfeld added a comment.

In D58385#1410546 <https://reviews.llvm.org/D58385#1410546>, @lebedev.ri wrote:

> In D58385#1410534 <https://reviews.llvm.org/D58385#1410534>, @Hahnfeld wrote:
>
> > In D58385#1410520 <https://reviews.llvm.org/D58385#1410520>, @lebedev.ri wrote:
> >
> > > In D58385#1410516 <https://reviews.llvm.org/D58385#1410516>, @Hahnfeld wrote:
> > >
> > > > Ping, that's now the only test that fails the `reverse-iteration` bot...
> > >
> > >
> > > This seems to only shuffle the tests around.
> > >  Does this now pass in reverse-iteration config?
> >
> >
> > Yes, I've tested this locally. The idea for `instr-remap.test` is to invoke `FileCheck` three times, checking each function separately.
>
>
> So then i think i'm missing the point..
>
> If it currently passes without reverse iteration, and does not pass with reverse iteration, 
>  then that means there is some nondeterminism, correct?


>From my understanding there is no predefined ordering for the output of functions. So with `reverse-iteration` they are shuffled, compared to what the test expects. However, their properties are the same in both configuration, they are just output in a different order which is why the approach with multiple `FileCheck` invocations works.

Another approach would be to sort the output and make it independent from `reverse-iteration`. See D58631 <https://reviews.llvm.org/D58631> for how this was done for ThinLTO. I'm fine with whatever unbreaks the tests, but I'm not familiar with the code so "fixing" the tests and getting review on this was easier for me.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58385/new/

https://reviews.llvm.org/D58385