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

[Sanjay Patel via llvm-commits]

spatel added a comment.

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

> I'm not sure what is the point of this patch. Can you describe what's broken and why this fixes it?
>
> If this is related to the other patch reverted, than, as I said there, this needs a wider discussion and I'd be very surprised if we couldn't fix whatever the problems are on ARM instead of just completely disabling it and making ARM a different path from other architectures.
>
> I really don't want us to start segregating the implementations per architecture, especially when nothing is broken yet.


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:

1. https://reviews.llvm.org/rL282259 (like https://reviews.llvm.org/D14200 / https://reviews.llvm.org/rL253269 before it) proposed to make -ffp-contract=on the default.
2. This allows FMA codegen for targets that have FMA instructions.
3. Please don't confuse this with -ffast-math; this has nothing to do with -ffast-math.
4. Obviously AArch64 has FMA instructions because r282259 caused AArch64 bots that run test-suite to fail and so it was reverted: https://reviews.llvm.org/rL282289.
5. The output checking for some tests that use FP in test-suite appears to be looking for exact bit patterns and/or does not have an appropriate FP tolerance setting.
6. This output checking is broken when we use FMA instructions because FMA instructions can produce different results than FMUL + FADD.
7. To keep the bots green, I am proposing (really just re-proposing the suggestions of Sept 23 from the post-commit thread for r282259) that they run with -ffp-contract=off in order to match the (faulty) expectations contained in test-suite.
8. I'm actually surprised that other architectures would not also fail test-suite for the same reason. Maybe it's just that there are no other FMA-capable bots running test-suite?
9. It is not wrong for bots to run with a particular -ffp-contract setting; they are all perfectly valid modes of operation after all.
10. Ideally, we would have more configs/bots to check each fp-contract mode. If we did that, then we'd also need to update the test-suite output checks to account for those different modes, but AFAIK nobody has volunteered to do that work.
11. Another solution would be to loosen the checks (increase the FP tolerance) in test-suite for tests that are sensitive to FMA codegen, so they will pass independently of the -ffp-contract setting.
12. This patch does not hinder anyone from doing the work for either #10 / #11.


https://reviews.llvm.org/D24925