diff mbox

[1/4] alpha: fix ceil on sNaN input [committed/2.23]

Message ID 20161208185149.22707-1-vapier@gentoo.org
State New
Headers show

Commit Message

Mike Frysinger Dec. 8, 2016, 6:51 p.m. UTC
From: Aurelien Jarno <aurelien@aurel32.net>

The alpha version of ceil wrongly return sNaN for sNaN input. Fix that
by checking for NaN and by returning the input value added with itself
in that case.

Finally remove the code to handle inexact exception, ceil should never
generate such an exception.

Changelog:
	* sysdeps/alpha/fpu/s_ceil.c (__ceil): Add argument with itself
	when it is a NaN.
	[_IEEE_FP_INEXACT] Remove.
	* sysdeps/alpha/fpu/s_ceilf.c (__ceilf): Likewise.

(cherry picked from commit 062e53c195b4a87754632c7d51254867247698b4)
---
 ChangeLog                   | 7 +++++++
 sysdeps/alpha/fpu/s_ceil.c  | 7 +++----
 sysdeps/alpha/fpu/s_ceilf.c | 7 +++----
 3 files changed, 13 insertions(+), 8 deletions(-)

Comments

Richard Henderson Dec. 9, 2016, 4:01 a.m. UTC | #1
On 12/08/2016 10:51 AM, Mike Frysinger wrote:
>  __ceil (double x)
>  {
> +  if (isnan (x))
> +    return x + x;
> +
>    if (isless (fabs (x), 9007199254740992.0))	/* 1 << DBL_MANT_DIG */
>      {
>        double tmp1, new_x;

Probably better to test for nan in the else of this if, since the isless is 
surely more likely than the nan.

That goes for all of the changes.


r~
Mike Frysinger Dec. 9, 2016, 4:11 a.m. UTC | #2
On 08 Dec 2016 20:01, Richard Henderson wrote:
> On 12/08/2016 10:51 AM, Mike Frysinger wrote:
> >  __ceil (double x)
> >  {
> > +  if (isnan (x))
> > +    return x + x;
> > +
> >    if (isless (fabs (x), 9007199254740992.0))	/* 1 << DBL_MANT_DIG */
> >      {
> >        double tmp1, new_x;
> 
> Probably better to test for nan in the else of this if, since the isless is 
> surely more likely than the nan.
> 
> That goes for all of the changes.

i guess fabs(NaN) and isless(NaN, ...) isn't a problem ?

would it be equiv to do ?
	if (__glibc_unlikely (isnan (x)))
-mike
Richard Henderson Dec. 9, 2016, 4:14 a.m. UTC | #3
On 12/08/2016 08:11 PM, Mike Frysinger wrote:
> On 08 Dec 2016 20:01, Richard Henderson wrote:
>> On 12/08/2016 10:51 AM, Mike Frysinger wrote:
>>>  __ceil (double x)
>>>  {
>>> +  if (isnan (x))
>>> +    return x + x;
>>> +
>>>    if (isless (fabs (x), 9007199254740992.0))	/* 1 << DBL_MANT_DIG */
>>>      {
>>>        double tmp1, new_x;
>>
>> Probably better to test for nan in the else of this if, since the isless is
>> surely more likely than the nan.
>>
>> That goes for all of the changes.
>
> i guess fabs(NaN) and isless(NaN, ...) isn't a problem ?

No, that's fine.

> would it be equiv to do ?
> 	if (__glibc_unlikely (isnan (x)))

No, since that test would still happen first.  I think that

   if (isless(...))
     ...
   else if (isnan(x))
     return x + x;

is better for the average case.


r~
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index 24ce4491dc80..ad8921af8858 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@ 
+2016-08-02  Aurelien Jarno  <aurelien@aurel32.net>
+
+	* sysdeps/alpha/fpu/s_ceil.c (__ceil): Add argument with itself
+	when it is a NaN.
+	[_IEEE_FP_INEXACT] Remove.
+	* sysdeps/alpha/fpu/s_ceilf.c (__ceilf): Likewise.
+
 2016-11-23  Matthew Fortune  <Matthew.Fortune@imgtec.com>
 	    Maciej W. Rozycki  <macro@imgtec.com>
 
diff --git a/sysdeps/alpha/fpu/s_ceil.c b/sysdeps/alpha/fpu/s_ceil.c
index c1ff864d4b86..e9c350af1cc0 100644
--- a/sysdeps/alpha/fpu/s_ceil.c
+++ b/sysdeps/alpha/fpu/s_ceil.c
@@ -26,17 +26,16 @@ 
 double
 __ceil (double x)
 {
+  if (isnan (x))
+    return x + x;
+
   if (isless (fabs (x), 9007199254740992.0))	/* 1 << DBL_MANT_DIG */
     {
       double tmp1, new_x;
 
       new_x = -x;
       __asm (
-#ifdef _IEEE_FP_INEXACT
-	     "cvttq/svim %2,%1\n\t"
-#else
 	     "cvttq/svm %2,%1\n\t"
-#endif
 	     "cvtqt/m %1,%0\n\t"
 	     : "=f"(new_x), "=&f"(tmp1)
 	     : "f"(new_x));
diff --git a/sysdeps/alpha/fpu/s_ceilf.c b/sysdeps/alpha/fpu/s_ceilf.c
index 7e63a6fe94e7..77e01a99f743 100644
--- a/sysdeps/alpha/fpu/s_ceilf.c
+++ b/sysdeps/alpha/fpu/s_ceilf.c
@@ -25,6 +25,9 @@ 
 float
 __ceilf (float x)
 {
+  if (isnanf (x))
+    return x + x;
+
   if (isless (fabsf (x), 16777216.0f))	/* 1 << FLT_MANT_DIG */
     {
       /* Note that Alpha S_Floating is stored in registers in a
@@ -36,11 +39,7 @@  __ceilf (float x)
 
       new_x = -x;
       __asm ("cvtst/s %3,%2\n\t"
-#ifdef _IEEE_FP_INEXACT
-	     "cvttq/svim %2,%1\n\t"
-#else
 	     "cvttq/svm %2,%1\n\t"
-#endif
 	     "cvtqt/m %1,%0\n\t"
 	     : "=f"(new_x), "=&f"(tmp1), "=&f"(tmp2)
 	     : "f"(new_x));