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

[Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>]

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.

---

2013-04-30  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/math/libm-test.inc b/math/libm-test.inc
index d131ffa..f50f920 100644
--- a/math/libm-test.inc
+++ b/math/libm-test.inc
@@ -8787,6 +8787,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
@@ -8798,6 +8799,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)
-- 
1.7.1