This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] PowerPC: fix hypot/hypof FP exceptions
- From: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- To: libc-alpha at sourceware dot org
- Date: Mon, 06 May 2013 14:19:37 -0300
- Subject: Re: [PATCH] PowerPC: fix hypot/hypof FP exceptions
- References: <517EADAF dot 8000601 at linux dot vnet dot ibm dot com> <Pine dot LNX dot 4 dot 64 dot 1304292033400 dot 15327 at digraph dot polyomino dot org dot uk> <517FDCCE dot 3040808 at linux dot vnet dot ibm dot com> <5183A15E dot 1040809 at linux dot vnet dot ibm dot com>
Ping.
On 05/03/2013 08:37 AM, Adhemerval Zanella wrote:
> On 04/30/2013 12:01 PM, Adhemerval Zanella wrote:
>> On 29-04-2013 17:35, Joseph S. Myers wrote:
>>> On Mon, 29 Apr 2013, Adhemerval Zanella wrote:
>>>
>>>> This patch fixes spurious overflow FP generated in PowerPC hypot code triggered
>>>> by recent cacos/cacosh/casin/casinh tests with argument as [+/-]0x1.fp-129 for
>>>> float and [+/-] 0x1.fp-1025 for double.
>>>>
>>>> For hypotf I removed the unneeded tests (since sqrt for double precision will be
>>>> used in the end).
>>>>
>>>> For hypot I adjusted the test to check if x/y with x>y is higher than 2^60 by
>>>> checking if first y * 2^60 will overflow. If so, the test used is x > (y*2^60).
>>>>
>>>> With this patch and the previous ULPs update , I'm not seeing any more issues
>>>> on float and double testcases for PowerPC. Tested on PPC32 and PPC64.
>>>>
>>>> Any tips, comments, advices?
>>> In a case such as this I advise:
>>>
>>> * File a bug in Bugzilla for the user-visible hypot issues, and reference
>>> the bug number as [BZ #N] in the ChangeLog entry for the fix.
>>>
>>> * As part of the patch fixing the bug, add testcases that directly test
>>> those hypot issues, rather than relying on the tests for another function
>>> to cover this bug.
>>>
>> Thanks for reply Joseph. I have create the bugzillas and update the patch with specific
>> tests.
>>
>> ---
>>
>>
> I updated the patch with NEWS additions of fixed bugs. Ok to commit?
>
>
> ---
>
> 2013-05-03 Adhemerval Zanella <azanella@linux.vnet.ibm.com>
>
> [BZ #15418]
> [BZ #15419]
> * sysdeps/powerpc/fpu/e_hypot.c: Fix spurious FP exception generated by
> internal tests.
> * sysdeps/powerpc/fpu/e_hypotf.c: Likewise.
>
> --
>
> diff --git a/NEWS b/NEWS
> index ce654c1..95a4e4f 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -16,7 +16,7 @@ Version 2.18
> 15084, 15085, 15086, 15160, 15214, 15221, 15232, 15234, 15283, 15285,
> 15287, 15304, 15305, 15307, 15309, 15327, 15330, 15335, 15336, 15337,
> 15342, 15346, 15361, 15366, 15380, 15394, 15405, 15406, 15409, 15416,
> - 15423.
> + 15418, 15419, 15423.
>
> * CVE-2013-0242 Buffer overrun in regexp matcher has been fixed (Bugzilla
> #15078).
> diff --git a/math/libm-test.inc b/math/libm-test.inc
> index 8feb0a6..fad302c 100644
> --- a/math/libm-test.inc
> +++ b/math/libm-test.inc
> @@ -9159,6 +9159,7 @@ hypot_test (void)
> TEST_ff_f (hypot, 0.75L, 1.25L, 1.45773797371132511771853821938639577L);
>
> TEST_ff_f (hypot, 1.0L, 0x1p-61L, 1.0L);
> + TEST_ff_f (hypot, 0x1p+0L, 0x1.fp-129L, 0x1p+0L);
> #if defined TEST_LDOUBLE && LDBL_MANT_DIG >= 106
> TEST_ff_f (hypot, 0x1.23456789abcdef0123456789ab8p-500L, 0x1.23456789abcdef0123456789ab8p-500L, 4.9155782399407039128612180934736799735113e-151L);
> #endif
> @@ -9170,6 +9171,7 @@ hypot_test (void)
>
> #if !defined TEST_FLOAT && !(defined TEST_DOUBLE && defined TEST_INLINE)
> TEST_ff_f (hypot, 0x3p1021L, 0x4p1021L, 0x5p1021L);
> + TEST_ff_f (hypot, 0x1p+0L, 0x0.3ep-1022L, 0x1p+0L);
> #endif
>
> #if defined TEST_LDOUBLE && LDBL_MAX_EXP >= 16384 && !defined TEST_INLINE
> diff --git a/sysdeps/powerpc/fpu/e_hypot.c b/sysdeps/powerpc/fpu/e_hypot.c
> index f236339..d8568d6 100644
> --- a/sysdeps/powerpc/fpu/e_hypot.c
> +++ b/sysdeps/powerpc/fpu/e_hypot.c
> @@ -26,6 +26,7 @@ static const double two600 = 4.149515568880993e+180;
> static const double two1022 = 4.49423283715579e+307;
> static const double twoM500 = 3.054936363499605e-151;
> static const double twoM600 = 2.4099198651028841e-181;
> +static const double two60factor = 1.5592502418239997e+290;
> static const double pdnum = 2.225073858507201e-308;
>
> /* __ieee754_hypot(x,y)
> @@ -87,9 +88,20 @@ __ieee754_hypot (double x, double y)
> x = y;
> y = t;
> }
> - if (y == 0.0 || (x / y) > two60)
> + if (y == 0.0)
> + return x;
> + /* if y is higher enough, y * 2^60 might overflow. The tests if
> + y >= 1.7976931348623157e+308/2^60 (two60factor) and uses the
> + appropriate check to avoid the overflow exception generation. */
> + if (y > two60factor)
> {
> - return x + y;
> + if ((x / y) > two60)
> + return x + y;
> + }
> + else
> + {
> + if (x > (y * two60))
> + return x + y;
> }
> if (x > two500)
> {
> diff --git a/sysdeps/powerpc/fpu/e_hypotf.c b/sysdeps/powerpc/fpu/e_hypotf.c
> index e97f0c3..93055af 100644
> --- a/sysdeps/powerpc/fpu/e_hypotf.c
> +++ b/sysdeps/powerpc/fpu/e_hypotf.c
> @@ -69,22 +69,8 @@ static const float two30 = 1.0737418e09;
> float
> __ieee754_hypotf (float x, float y)
> {
> - x = fabsf (x);
> - y = fabsf (y);
> -
> TEST_INF_NAN (x, y);
>
> - if (y > x)
> - {
> - float t = y;
> - y = x;
> - x = t;
> - }
> - if (y == 0.0 || (x / y) > two30)
> - {
> - return x + y;
> - }
> -
> return __ieee754_sqrt ((double) x * x + (double) y * y);
> }
> strong_alias (__ieee754_hypotf, __hypotf_finite)