Re: sinl/cosl for ldbl-96

["Carlos O'Donell" <carlos at systemhalted dot org>]

On Thu, Mar 15, 2012 at 8:52 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> This patch fixes the problems with range reduction for sinl / cosl /
> sincosl (ldbl-96, bug 13851) by providing a C implementation and using
> it instead of the x87 instructions.
>
> In general the C implementation is based on Jakub's ldbl-128
> implementation, and I did not try to optimize the polynomials used for
> the lower precision of ldbl-96 (it's probably possible to use fewer
> terms without any significant effect on accuracy); I used plain
> floating point in ldbl-96 in place of various bit-manipulations of the
> floating-point representation done in ldbl-128. ?I did regenerate the
> table of precomputed values (t_sincosl.c) with the attached program
> (having verified that a variant of it for 113-bit precision appeared
> to replicate the values in ldbl-128/t_sincosl.c).
>
> It turned out that the general range reduction code, not previously
> used for x86, needed a __FLT_EVAL_METHOD__ != 0 fix in the case prec
> == 2 (the value to be used for 64-bit mantissa) similar to that
> already present for prec == 3 (113-bit mantissa).
>
> In the course of preparing this I found what appears to be a bug in
> the ldbl-128 and ldbl-128ibm implementations of k_sinl.c, for which
> I'll send a patch separately.
>
> Tested x86 and x86_64 and ULPs updated based on that testing.
>
> 2012-03-16 ?Joseph Myers ?<joseph@codesourcery.com>
>
> ? ? ? ?[BZ #13851]
> ? ? ? ?* sysdeps/ieee754/ldbl-96/e_rem_pio2l.c: New file.
> ? ? ? ?* sysdeps/ieee754/ldbl-96/k_cosl.c: Likewise.
> ? ? ? ?* sysdeps/ieee754/ldbl-96/k_sinl.c: Likewise.
> ? ? ? ?* sysdeps/ieee754/ldbl-96/t_sincosl.c: Likewise.
> ? ? ? ?* sysdeps/ieee754/ldbl-96/s_cosl.c (__cosl): Correct test for
> ? ? ? ?infinite argument.
> ? ? ? ?* sysdeps/ieee754/ldbl-96/s_sinl.c (__sinl): Likewise.
> ? ? ? ?* sysdeps/ieee754/dbl-64/k_rem_pio2.c: Handle __FLT_EVAL_METHOD__
> ? ? ? ?!= 0 for prec == 2.
> ? ? ? ?* sysdeps/i386/fpu/e_rem_pio2l.c: Remove.
> ? ? ? ?* sysdeps/i386/fpu/k_rem_pio2.c: Likewise.
> ? ? ? ?* sysdeps/i386/fpu/s_cosl.S: Likewise.
> ? ? ? ?* sysdeps/i386/fpu/s_sincosl.S: Likewise.
> ? ? ? ?* sysdeps/i386/fpu/s_sinl.S: Likewise.
> ? ? ? ?* sysdeps/x86_64/fpu/e_rem_pio2l.c: Likewise.
> ? ? ? ?* sysdeps/x86_64/fpu/k_cosl.c: Likewise.
> ? ? ? ?* sysdeps/x86_64/fpu/k_sinl.c: Likewise.
> ? ? ? ?* sysdeps/x86_64/fpu/s_cosl.S: Likewise.
> ? ? ? ?* sysdeps/x86_64/fpu/s_sincosl.S: Likewise.
> ? ? ? ?* sysdeps/x86_64/fpu/s_sinl.S: Likewise.
> ? ? ? ?* math/libm-test.inc (cos_test): Add more tests and enable more
> ? ? ? ?tests for long double.
> ? ? ? ?(sin_test): Likewise.
> ? ? ? ?(sincos_test): Likewise.
> ? ? ? ?* sysdeps/i386/fpu/libm-test-ulps: Update.
> ? ? ? ?* sysdeps/x86_64/fpu/libm-test-ulps: Likewise.

I have not gone through and reviewed this code, but I have one comment.

Could you please include the entire contents of compute-fsincosl.c in
the comments above the table that it precomputes?

There is value in keeping these "proglets" as Jakub calls them since
they document how the constants were computed (which can in and of
itself have bugs).

Even better would be to include these programs somehow into the build
since bug fixes in mpfr could result in changes to the table of
constants.

Cheers,
Carlos.