[COMMITTED] PowerPC: Fix copysignf optimization macro
diff mbox

Message ID 537BC70F.1070304@linux.vnet.ibm.com
State New
Headers show

Commit Message

Adhemerval Zanella May 20, 2014, 9:20 p.m. UTC
This patch fixes the __copysignf optimized macro meant to internal libm
usage when used with constant value.  Without the explicit cast to
float, if it is used with const double value (for instance, on
s_casinhf.c) double constants will be used and it may lead to precision
issues in some algorithms.

It fixes the following failures on PPC64/POWER7:

Failure: Test: Real part of: cacos_downward (inf + 0 i)
Result:
 is:          1.19209289550781250000e-07   0x1.00000000000000000000p-23
 should be:   0.00000000000000000000e+00   0x0.00000000000000000000p+0
Failure: Test: Real part of: cacos_downward (inf - 0 i)
Result:
 is:          1.19209289550781250000e-07   0x1.00000000000000000000p-23
 should be:   0.00000000000000000000e+00   0x0.00000000000000000000p+0
Failure: Test: Real part of: cacos_downward (inf + 0.5 i)
Result:
 is:          1.19209289550781250000e-07   0x1.00000000000000000000p-23
 should be:   0.00000000000000000000e+00   0x0.00000000000000000000p+0
Failure: Test: Real part of: cacos_downward (inf - 0.5 i)
Result:
 is:          1.19209289550781250000e-07   0x1.00000000000000000000p-23
 should be:   0.00000000000000000000e+00   0x0.00000000000000000000p+0
Failure: Test: Real part of: cacos_towardzero (inf + 0 i)
Result:
 is:          1.19209289550781250000e-07   0x1.00000000000000000000p-23
 should be:   0.00000000000000000000e+00   0x0.00000000000000000000p+0
Failure: Test: Real part of: cacos_towardzero (inf - 0 i)
Result:
 is:          1.19209289550781250000e-07   0x1.00000000000000000000p-23
 should be:   0.00000000000000000000e+00   0x0.00000000000000000000p+0
Failure: Test: Real part of: cacos_towardzero (inf + 0.5 i)
Result:
 is:          1.19209289550781250000e-07   0x1.00000000000000000000p-23
 should be:   0.00000000000000000000e+00   0x0.00000000000000000000p+0
Failure: Test: Real part of: cacos_towardzero (inf - 0.5 i)
Result:
 is:          1.19209289550781250000e-07   0x1.00000000000000000000p-23
 should be:   0.00000000000000000000e+00   0x0.00000000000000000000p+0

---

2014-05-20  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>

	* sysdeps/powerpc/fpu/math_private.h [__copysignf]: Fix copysign macro
	optimization when used with float constants.

--

Comments

Andreas Schwab May 21, 2014, 6:55 a.m. UTC | #1
Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes:

> diff --git a/sysdeps/powerpc/fpu/math_private.h b/sysdeps/powerpc/fpu/math_private.h
> index dde153d..1ec4881 100644
> --- a/sysdeps/powerpc/fpu/math_private.h
> +++ b/sysdeps/powerpc/fpu/math_private.h
> @@ -166,11 +166,13 @@ __ieee754_sqrtf (float __x)
>  # ifndef __copysignf
>  #  define __copysignf(x, y)		\
>      ({ float __z;			\
> +       float __x = x;			\
> +       float __y = y;			\
>       __asm __volatile (			\
>  	"	fcpsgn %0,%1,%2\n"	\
>  	"	frsp %0,%0\n"		\
>  		: "=f" (__z)		\
> -		: "f" (y), "f" (x));	\
> +		: "f" (__y), "f" (__x));\
>       __z; })
>  # endif

Is that definition actually needed at all?
sysdeps/generic/math_private.h defines it as __builtin_copysignf.

Andreas.
Adhemerval Zanella May 21, 2014, 11:37 a.m. UTC | #2
On 21-05-2014 03:55, Andreas Schwab wrote:
> Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes:
>
>> diff --git a/sysdeps/powerpc/fpu/math_private.h b/sysdeps/powerpc/fpu/math_private.h
>> index dde153d..1ec4881 100644
>> --- a/sysdeps/powerpc/fpu/math_private.h
>> +++ b/sysdeps/powerpc/fpu/math_private.h
>> @@ -166,11 +166,13 @@ __ieee754_sqrtf (float __x)
>>  # ifndef __copysignf
>>  #  define __copysignf(x, y)		\
>>      ({ float __z;			\
>> +       float __x = x;			\
>> +       float __y = y;			\
>>       __asm __volatile (			\
>>  	"	fcpsgn %0,%1,%2\n"	\
>>  	"	frsp %0,%0\n"		\
>>  		: "=f" (__z)		\
>> -		: "f" (y), "f" (x));	\
>> +		: "f" (__y), "f" (__x));\
>>       __z; })
>>  # endif
> Is that definition actually needed at all?
> sysdeps/generic/math_private.h defines it as __builtin_copysignf.
>
> Andreas.
>
Indeed it is not, I'll remove both version in powerpc math_private.h

Patch
diff mbox

diff --git a/sysdeps/powerpc/fpu/math_private.h b/sysdeps/powerpc/fpu/math_private.h
index dde153d..1ec4881 100644
--- a/sysdeps/powerpc/fpu/math_private.h
+++ b/sysdeps/powerpc/fpu/math_private.h
@@ -166,11 +166,13 @@  __ieee754_sqrtf (float __x)
 # ifndef __copysignf
 #  define __copysignf(x, y)		\
     ({ float __z;			\
+       float __x = x;			\
+       float __y = y;			\
      __asm __volatile (			\
 	"	fcpsgn %0,%1,%2\n"	\
 	"	frsp %0,%0\n"		\
 		: "=f" (__z)		\
-		: "f" (y), "f" (x));	\
+		: "f" (__y), "f" (__x));\
      __z; })
 # endif