This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix cos computation for multiple precision fallback (bz #20357)
- From: "Paul E. Murphy" <murphyp at linux dot vnet dot ibm dot com>
- To: Siddhesh Poyarekar <siddhesh at sourceware dot org>, libc-alpha at sourceware dot org
- Cc: mclay at lycos dot com
- Date: Mon, 18 Jul 2016 09:47:43 -0500
- Subject: Re: [PATCH] Fix cos computation for multiple precision fallback (bz #20357)
- Authentication-results: sourceware.org; auth=none
- References: <1468313523-18716-1-git-send-email-siddhesh@sourceware.org>
On 07/12/2016 03:52 AM, Siddhesh Poyarekar wrote:
> During the sincos consolidation I made two mistakes, one was a logical
> error due to which cos(0x1.8475e5afd4481p+0) returned
> sin(0x1.8475e5afd4481p+0) instead.
>
> The second issue was an error in negating inputs for the correct
> quadrants for sine. I could not find a suitable test case for this
> despite running a program to search for such an input for a couple of
> hours.
>
> Following patch fixes both issues. Tested on x86_64. Thanks to Matt
> (Clay?) for identifying the issue.
>
> [BZ #20357]
> * sysdeps/ieee754/dbl-64/s_sin.c (sloww): Fix up condition
> to call __mpsin/__mpcos and to negate values.
> * math/auto-libm-test-in: Add test.
> * math/auto-libm-test-out: Regenerate.
> ---
> math/auto-libm-test-in | 3 +
> math/auto-libm-test-out | 207 +++++++++++++++++++++++++++++++++++++++++
> sysdeps/ieee754/dbl-64/s_sin.c | 4 +-
> 3 files changed, 212 insertions(+), 2 deletions(-)
>
> diff --git a/math/auto-libm-test-in b/math/auto-libm-test-in
> index ffcc40a..51c152c 100644
> --- a/math/auto-libm-test-in
> +++ b/math/auto-libm-test-in
> @@ -1144,6 +1144,7 @@ cos 0x4.7857dp+68
> cos -0x1.02e34cp+0
> cos 0xf.f0274p+4
> cos 0x3.042d88p+0
> +cos 0x1.8475e5afd4481p+0
>
> cosh 0
> cosh -0
> @@ -3818,6 +3819,7 @@ sin min
> sin -min
> sin min_subnorm
> sin -min_subnorm
> +sin 0x1.8475e5afd4481p+0
>
> sincos 0
> sincos -0
> @@ -3852,6 +3854,7 @@ sincos min
> sincos -min
> sincos min_subnorm
> sincos -min_subnorm
> +sincos 0x1.8475e5afd4481p+0
>
> sinh 0
> sinh -0
Ok.
> diff --git a/sysdeps/ieee754/dbl-64/s_sin.c b/sysdeps/ieee754/dbl-64/s_sin.c
> index ca2532f..7c9a079 100644
> --- a/sysdeps/ieee754/dbl-64/s_sin.c
> +++ b/sysdeps/ieee754/dbl-64/s_sin.c
> @@ -803,7 +803,7 @@ sloww (double x, double dx, double orig, int k)
> a = t - y;
> da = ((t - a) - y) + da;
>
> - if (n == 2 || n == 1)
> + if (n & 2)
That looks ok. I had to do double check the unification. It wasn't
immediately obvious this was correct.
> {
> a = -a;
> da = -da;
> @@ -817,7 +817,7 @@ sloww (double x, double dx, double orig, int k)
> if (w[0] == w[0] + cor)
> return (a > 0) ? w[0] : -w[0];
>
> - return (n & 1) ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
> + return k ? __mpcos (orig, 0, true) : __mpsin (orig, 0, true);
OK, that fix looks fairly obvious after reviewing commit
b300455644e2945da05eb49d12d3a037f1408be1.