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

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

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)
-- 
1.7.1