Re: [PATCH] PowerPC: fix hypot/hypof FP exceptions

[Adhemerval Zanella <azanella 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)