Responding to two emails at the same time ;-)
On 7/21/23 13:47, Richard Biener wrote:
On Fri, 21 Jul 2023, Matthew Malcomson wrote:
On some AArch64 bootstrapped builds, we were getting a flaky test
because the floating point operations in `get_time` were being fused
with the floating point operations in `timevar_accumulate`.
This meant that the rounding behaviour of our multiplication with
`ticks_to_msec` was different when used in `timer::start` and when
performed in `timer::stop`. These extra inaccuracies led to the
testcase `g++.dg/ext/timevar1.C` being flaky on some hardware.
This change ensures those operations are not fused and hence stops the test
being flaky on that particular machine. There is no expected change in the
generated code.
Bootstrap & regtest on AArch64 passes with no regressions.
I think this is undesriable. With fused you mean we use FMA?
I think you could use -ffp-contract=off for the TU instead.
Yeah -- we used fused multiply subtract because we combined the multiply
in `get_time` with the subtract in `timevar_accumulate`.
Note you can't use __attribute__((noinline)) literally since the
host compiler might not support this.
Richard.
On 7/21/23 13:49, Xi Ruoyao wrote:
...
I don't think it's correct. It will break bootstrapping GCC from other
ISO C++11 compilers, you need to at least guard it with #ifdef __GNUC__.
And IMO it's just hiding the real problem.
We need more info of the "particular machine". Is this a hardware bug
(i.e. the machine violates the AArch64 spec) or a GCC code generation
issue? Or should we generally use -ffp-contract=off in BOOT_CFLAGS?
My understanding is that this is not a hardware bug and that it's
specified that rounding does not happen on the multiply "sub-part" in
`FNMSUB`, but rounding happens on the `FMUL` that generates some input
to it.
I was given to understand from discussions with others that this codegen
is allowed -- though I honestly didn't confirm the line of reasoning
through all the relevant standards.
------------------------
W.r.t. both:
Thanks for pointing out bootstrapping from other ISO C++ compilers --
(didn't realise that was a concern).
I can look into `-ffp-contract=off` as you both have recommended.
One question -- if we have concerns that the host compiler may not be
able to handle `attribute((noinline))` would we also be concerned that
this flag may not be supported?
(Or is the severity of lack of support sufficiently different in the two
cases that this is fine -- i.e. not compile vs may trigger floating
point rounding inaccuracies?)
