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.gli...@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
