Re: patch to bug #86829

[Jeff Law <law at redhat dot com>]

On 08/04/2018 07:22 AM, Giuliano Augusto Faulin Belinassi wrote:
> Closes bug #86829
> 
> Description: Adds substitution rules for both sin(atan(x)) and
> cos(atan(x)). These formulas are replaced by x / sqrt(x*x + 1) and 1 /
> sqrt(x*x + 1) respectively, providing up to 10x speedup. This identity
> can be proved mathematically.
> 
> Changelog:
> 
> 2018-08-03  Giuliano Belinassi <giuliano.belinassi@usp.br>
> 
>     * match.pd: add simplification rules to sin(atan(x)) and cos(atan(x)).
> 
> Bootstrap and Testing:
> There were no unexpected failures in a proper testing in GCC 8.1.0
> under a x86_64 running Ubuntu 18.04.
I understand these are mathematical identities.  But floating point
arthmetic in a compiler isn't nearly that clean :-)  We have to worry
about overflows, underflows, rounding, and the simple fact that many
floating point numbers can't be exactly represented.

Just as an example, compare the results for
x = 0x1.fffffffffffffp1023

I think sin(atan (x)) is well defined in that case.  But the x*x isn't
because it overflows.

So  I think this has to be somewhere under the -ffast-math umbrella.
And the testing requirements for that are painful -- you have to verify
it doesn't break the spec benchmark.

I know Richi acked in the PR, but that might have been premature.

jeff