This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Fix powerpc round, roundf spurious "inexact" (bug 19238) [committed]
- From: Steven Munroe <munroesj at linux dot vnet dot ibm dot com>
- To: Joseph Myers <joseph at codesourcery dot com>, Tulio Magno Quites Machado Filho <tuliom at linux dot vnet dot ibm dot com>, Carlos Eduardo Seo <cseo at linux dot vnet dot ibm dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Thu, 17 Dec 2015 10:24:49 -0600
- Subject: Re: Fix powerpc round, roundf spurious "inexact" (bug 19238) [committed]
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot DEB dot 2 dot 10 dot 1511121900250 dot 6391 at digraph dot polyomino dot org dot uk>
- Reply-to: munroesj at linux dot vnet dot ibm dot com
On Thu, 2015-11-12 at 19:00 +0000, Joseph Myers wrote:
> The powerpc hard-float round and roundf functions, both 32-bit and
> 64-bit, raise spurious "inexact" exceptions for integer arguments from
> adding 0.5 and rounding to integer toward zero.
>
> Since these functions already save and restore the rounding mode, it's
> natural to make them restore the full floating-point state instead to
> fix this bug, which this patch does. The save of the state is moved
> after the first floating-point operation on the input so that any
> "invalid" exceptions from signaling NaN inputs are properly
> preserved. As a consequence of this approach to the fix, "inexact"
> for noninteger arguments (disallowed by TS 18661-1 but not by C99/C11,
> see bug 15479) is also avoided for these implementations; this is
> *not* a general fix for bug 15479 since plenty of other
> implementations of various functions still raise spurious "inexact"
> for noninteger arguments.
>
> This issue and fix do not apply to builds using power5+ versions of
> round and roundf, which use the frin instruction and avoid "inexact"
> exceptions that way.
>
Joseph this gets confusing when we introduce the multiarch IFUNC
support. In a modern system (POWER6/7/8 the code below should never be
used. The code from:
./sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_round-power5+.S
./sysdeps/powerpc/powerpc64/fpu/multiarch/s_round-power5+.S
which includes:
sysdeps/powerpc/powerpc64/power5+/fpu/s_round.S
should be what is executing.
If this is not happening, there is bug in the build or in the IFUNC
resolver.
Also for glibc internal use it is important to built --with-cpu=power7
> This patch should get hard-float powerpc32 and powerpc64 (default
> function implementations) back to a state where test-float and
> test-double will pass after ulps regeneration.
>
> Tested for powerpc32 and powerpc64. Committed.
>
> 2015-11-12 Joseph Myers <joseph@codesourcery.com>
>
> [BZ #15479]
> [BZ #19238]
> * sysdeps/powerpc/powerpc32/fpu/s_round.S (__round): Save
> floating-point state after first operation on input. Restore full
> state rather than just rounding mode.
> * sysdeps/powerpc/powerpc32/fpu/s_roundf.S (__roundf): Likewise.
> * sysdeps/powerpc/powerpc64/fpu/s_round.S (__round): Likewise.
> * sysdeps/powerpc/powerpc64/fpu/s_roundf.S (__roundf): Likewise.
>
> diff --git a/sysdeps/powerpc/powerpc32/fpu/s_round.S b/sysdeps/powerpc/powerpc32/fpu/s_round.S
> index 061fbe2..18ba18a 100644
> --- a/sysdeps/powerpc/powerpc32/fpu/s_round.S
> +++ b/sysdeps/powerpc/powerpc32/fpu/s_round.S
> @@ -38,7 +38,6 @@
>
> .section ".text"
> ENTRY (__round)
> - mffs fp11 /* Save current FPU rounding mode. */
> #ifdef SHARED
> mflr r11
> cfi_register(lr,r11)
> @@ -55,6 +54,8 @@ ENTRY (__round)
> fabs fp0,fp1
> fsub fp12,fp13,fp13 /* generate 0.0 */
> fcmpu cr7,fp0,fp13 /* if (fabs(x) > TWO52) */
> + mffs fp11 /* Save current FPU rounding mode and
> + "inexact" state. */
> fcmpu cr6,fp1,fp12 /* if (x > 0.0) */
> bnllr- cr7
> mtfsfi 7,1 /* Set rounding mode toward 0. */
> @@ -70,7 +71,8 @@ ENTRY (__round)
> fsub fp1,fp1,fp13 /* x-= TWO52; */
> fabs fp1,fp1 /* if (x == 0.0) */
> /* x = 0.0; */
> - mtfsf 0x01,fp11 /* restore previous rounding mode. */
> + mtfsf 0xff,fp11 /* Restore previous rounding mode and
> + "inexact" state. */
> blr
> .L4:
> fsub fp9,fp1,fp10 /* x+= 0.5; */
> @@ -80,7 +82,8 @@ ENTRY (__round)
> fnabs fp1,fp1 /* if (x == 0.0) */
> /* x = -0.0; */
> .L9:
> - mtfsf 0x01,fp11 /* restore previous rounding mode. */
> + mtfsf 0xff,fp11 /* Restore previous rounding mode and
> + "inexact" state. */
> blr
> END (__round)
>
> diff --git a/sysdeps/powerpc/powerpc32/fpu/s_roundf.S b/sysdeps/powerpc/powerpc32/fpu/s_roundf.S
> index 414bede..e69a823 100644
> --- a/sysdeps/powerpc/powerpc32/fpu/s_roundf.S
> +++ b/sysdeps/powerpc/powerpc32/fpu/s_roundf.S
> @@ -37,7 +37,6 @@
>
> .section ".text"
> ENTRY (__roundf )
> - mffs fp11 /* Save current FPU rounding mode. */
> #ifdef SHARED
> mflr r11
> cfi_register(lr,r11)
> @@ -54,6 +53,8 @@ ENTRY (__roundf )
> fabs fp0,fp1
> fsubs fp12,fp13,fp13 /* generate 0.0 */
> fcmpu cr7,fp0,fp13 /* if (fabs(x) > TWO23) */
> + mffs fp11 /* Save current FPU rounding mode and
> + "inexact" state. */
> fcmpu cr6,fp1,fp12 /* if (x > 0.0) */
> bnllr- cr7
> mtfsfi 7,1 /* Set rounding mode toward 0. */
> @@ -68,7 +69,8 @@ ENTRY (__roundf )
> fsubs fp1,fp1,fp13 /* x-= TWO23; */
> fabs fp1,fp1 /* if (x == 0.0) */
> /* x = 0.0; */
> - mtfsf 0x01,fp11 /* restore previous rounding mode. */
> + mtfsf 0xff,fp11 /* Restore previous rounding mode and
> + "inexact" state. */
> blr
> .L4:
> fsubs fp9,fp1,fp10 /* x+= 0.5; */
> @@ -78,7 +80,8 @@ ENTRY (__roundf )
> fnabs fp1,fp1 /* if (x == 0.0) */
> /* x = -0.0; */
> .L9:
> - mtfsf 0x01,fp11 /* restore previous rounding mode. */
> + mtfsf 0xff,fp11 /* Restore previous rounding mode and
> + "inexact" state. */
> blr
> END (__roundf)
>
> diff --git a/sysdeps/powerpc/powerpc64/fpu/s_round.S b/sysdeps/powerpc/powerpc64/fpu/s_round.S
> index 2f99c04..31ede39 100644
> --- a/sysdeps/powerpc/powerpc64/fpu/s_round.S
> +++ b/sysdeps/powerpc/powerpc64/fpu/s_round.S
> @@ -38,11 +38,12 @@
>
> EALIGN (__round, 4, 0)
> CALL_MCOUNT 0
> - mffs fp11 /* Save current FPU rounding mode. */
> lfd fp13,.LC0@toc(2)
> fabs fp0,fp1
> fsub fp12,fp13,fp13 /* generate 0.0 */
> fcmpu cr7,fp0,fp13 /* if (fabs(x) > TWO52) */
> + mffs fp11 /* Save current FPU rounding mode and
> + "inexact" state. */
> fcmpu cr6,fp1,fp12 /* if (x > 0.0) */
> bnllr- cr7
> mtfsfi 7,1 /* Set rounding mode toward 0. */
> @@ -53,7 +54,8 @@ EALIGN (__round, 4, 0)
> fsub fp1,fp1,fp13 /* x-= TWO52; */
> fabs fp1,fp1 /* if (x == 0.0) */
> /* x = 0.0; */
> - mtfsf 0x01,fp11 /* restore previous rounding mode. */
> + mtfsf 0xff,fp11 /* Restore previous rounding mode and
> + "inexact" state. */
> blr
> .L4:
> fsub fp9,fp1,fp10 /* x+= 0.5; */
> @@ -63,7 +65,8 @@ EALIGN (__round, 4, 0)
> fnabs fp1,fp1 /* if (x == 0.0) */
> /* x = -0.0; */
> .L9:
> - mtfsf 0x01,fp11 /* restore previous rounding mode. */
> + mtfsf 0xff,fp11 /* Restore previous rounding mode and
> + "inexact" state. */
> blr
> END (__round)
>
> diff --git a/sysdeps/powerpc/powerpc64/fpu/s_roundf.S b/sysdeps/powerpc/powerpc64/fpu/s_roundf.S
> index babb52b..3ccf48c 100644
> --- a/sysdeps/powerpc/powerpc64/fpu/s_roundf.S
> +++ b/sysdeps/powerpc/powerpc64/fpu/s_roundf.S
> @@ -39,11 +39,12 @@
>
> EALIGN (__roundf, 4, 0)
> CALL_MCOUNT 0
> - mffs fp11 /* Save current FPU rounding mode. */
> lfs fp13,.LC0@toc(2)
> fabs fp0,fp1
> fsubs fp12,fp13,fp13 /* generate 0.0 */
> fcmpu cr7,fp0,fp13 /* if (fabs(x) > TWO23) */
> + mffs fp11 /* Save current FPU rounding mode and
> + "inexact" state. */
> fcmpu cr6,fp1,fp12 /* if (x > 0.0) */
> bnllr- cr7
> mtfsfi 7,1 /* Set rounding mode toward 0. */
> @@ -54,7 +55,8 @@ EALIGN (__roundf, 4, 0)
> fsubs fp1,fp1,fp13 /* x-= TWO23; */
> fabs fp1,fp1 /* if (x == 0.0) */
> /* x = 0.0; */
> - mtfsf 0x01,fp11 /* restore previous rounding mode. */
> + mtfsf 0xff,fp11 /* Restore previous rounding mode and
> + "inexact" state. */
> blr
> .L4:
> fsubs fp9,fp1,fp10 /* x+= 0.5; */
> @@ -64,7 +66,8 @@ EALIGN (__roundf, 4, 0)
> fnabs fp1,fp1 /* if (x == 0.0) */
> /* x = -0.0; */
> .L9:
> - mtfsf 0x01,fp11 /* restore previous rounding mode. */
> + mtfsf 0xff,fp11 /* Restore previous rounding mode and
> + "inexact" state. */
> blr
> END (__roundf)
>
>