This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Optimize sin(atan(x)), take 2
- From: Giuliano Augusto Faulin Belinassi <giuliano dot belinassi at usp dot br>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Jeff Law <law at redhat dot com>, Richard Biener <richard dot guenther at gmail dot com>, marc dot glisse at inria dot fr
- Date: Fri, 5 Oct 2018 09:09:03 -0300
- Subject: Re: [PATCH] Optimize sin(atan(x)), take 2
- References: <CAEFO=4CzZzMDfFRa7_G4LUnAD0+NRqe9KhYQ9h7GPjkk8Uqcaw@mail.gmail.com> <38bddfc3-8c0a-e07a-f7ee-f379ffd40fc3@redhat.com> <alpine.DEB.2.21.1810041347180.14369@stedding.saclay.inria.fr>
Thank you for the review. I will address all these issues :-).
> Imagine a pause here while I lookup isolation of radicals.... It's been
> a long time... OK. Sure. I see what you're doing here...
Sorry, but I really did not understand your comment. Should I write a
shorter comment for that function?
> Not sure what you mean for safety reasons. The calculations to produce
> "c" then convert it into a REAL_VALUE_TYPE all make sense. Just not
> sure what this line is really meant to do.
Imagine the following case:
Let "c" be the real constant such that it is certain that for every x
> "c", 1/sqrt(x*x + 1) = 1.
Suppose now that our calculation leads us to a c' < "c" due to a minor
imprecision.
The logic here is that 10 * c' > "c" and everything will work, thus it is safer.
Note however that I cannot prove that 10 * c' > "c", but I would be
really surprised
if this does not holds.
> A related remark would be: with the precision of double, for x>=cst (about
> 2^53), atan(x) is constant, within .5 ulp of pi/2 if the math library is
> super precise (which it probably isn't). Returning 0 for its cos (what
> happens if x*x gives +Inf) is thus completely fine unless you are using
> crlibm, but then you wouldn't use flag_unsafe_math_optimizations. The main
> issue is that if we have -ffast-math, we have -ffinite-math-only, and we
> are possibly introducing an infinity as intermediate result...
Thank you. This clarifies the need for a similar constant for the cos(atan(x)).
On Thu, Oct 4, 2018 at 9:36 AM Marc Glisse <marc.glisse@inria.fr> wrote:
>
> On Wed, 3 Oct 2018, Jeff Law wrote:
>
> >> +/* Simplify cos(atan(x)) -> 1 / sqrt(x*x + 1). */
> >> + (for coss (COS)
> >> + atans (ATAN)
> >> + sqrts (SQRT)
> >> + (simplify
> >> + (coss (atans:s @0))
> >> + (rdiv {build_one_cst (type);}
> >> + (sqrts (plus (mult @0 @0) {build_one_cst (type);})))))
> > Don't we have the same kinds of issues with the x*x in here? As X gets
> > large it will overflow, but the result is going to be approaching zero.
> > So we might be able to use a similar trick here.
>
> (I have not read the patch)
>
> The similar trick would say that for X large, this is the same as 1/abs(X)
> I guess. Note that it may be simpler to generate a call to hypot (C99).
>
> A related remark would be: with the precision of double, for x>=cst (about
> 2^53), atan(x) is constant, within .5 ulp of pi/2 if the math library is
> super precise (which it probably isn't). Returning 0 for its cos (what
> happens if x*x gives +Inf) is thus completely fine unless you are using
> crlibm, but then you wouldn't use flag_unsafe_math_optimizations. The main
> issue is that if we have -ffast-math, we have -ffinite-math-only, and we
> are possibly introducing an infinity as intermediate result...
>
> --
> Marc Glisse