[PATCH] D24925: [testsuite] turn fp-contract off for ARM because output checking is not flexible enough

[Hal Finkel via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D24925#552702, @rengolin wrote:

> In https://reviews.llvm.org/D24925#552646, @spatel wrote:
>
> > I don't know what "segregating the implementations per architecture" means, and this patch does not disable any testing (just makes it test differently). Let me try to explain some more:
>
>
> This is exactly what it means: testing different things for different architectures. If it used to work, and it's not working, disabling / redirecting the tests is *not* a solution.


Hold on... both the new and the old state are working states, they're just different. When you use FMAs, as the standard allows, the answer will often be different compared to not using them. However, making this change is actually the most-useful thing to do (I'll explain why I think this below).

> 

> 

> > 12. This patch does not hinder anyone from doing the work for either #10 / #11.

> 

> 

> It relaxes tests when we should be fixing tests. The first step of making tests irrelevant is disabling them for random reasons instead of fixing them. If that's a route we're willing to take, than we might just as well disable all buildbots.


I understand you're being sarcastic, but this is unfair. This is not a random reason, we understand exactly why this change produces changes in program output, and I also believe that testing using -ffp-contract=off is the most-useful test configuration to have (although having others in addition would be great too). Why is it the most useful? Because it is the testing configuration for which we can have the most-precise known-good answer for comparison. As you point out, having strict thresholds for these tests has caught important miscompilation bugs in the past. When you start using FMAs, one issue is that there are lots of places where you now might or might not actually use them, and so the space of potential correct answers is large. We can come up with loser tolerances to capture this freedom, but even doing this precisely is potentially degenerate with bugs for some inputs, and so only having the loser version is actually worse than just testing with FMAs disabled. I would love to have both, but in the mean time, this is the right thing to do.

FWIW, the reason we do this on PPC/LInux, and have for a long time, is exactly because GCC on PPC/Linux defaults to using FMAs, and so GCC needs -ffp-contract=off to pass our test suite (independent of anything else). This is just a general statement about our reference outputs: they're all generated without FMAs. Our flags should encode that dependency so that we don't see spurious failures (with Clang or any other compiler).


https://reviews.llvm.org/D24925