diff mbox

Fix powerpc round, roundf spurious "inexact" (bug 19238) [committed]

Message ID alpine.DEB.2.10.1511121900250.6391@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers Nov. 12, 2015, 7 p.m. UTC
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.

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.

Comments

Steven Munroe Dec. 17, 2015, 4:24 p.m. UTC | #1
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)
> 
>
Joseph Myers Dec. 17, 2015, 4:55 p.m. UTC | #2
On Thu, 17 Dec 2015, Steven Munroe wrote:

> > 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:

Yes, I already said this code isn't used when the power5+ versions are 
used.  My 64-bit powerpc testing is on a POWER4 system and my 32-bit 
testing is on a 476 system; although I now have access to a little-endian 
POWER8 system I haven't tried glibc testing there.
Adhemerval Zanella Netto Dec. 17, 2015, 5:22 p.m. UTC | #3
On 17-12-2015 14:55, Joseph Myers wrote:
> On Thu, 17 Dec 2015, Steven Munroe wrote:
> 
>>> 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:
> 
> Yes, I already said this code isn't used when the power5+ versions are 
> used.  My 64-bit powerpc testing is on a POWER4 system and my 32-bit 
> testing is on a 476 system; although I now have access to a little-endian 
> POWER8 system I haven't tried glibc testing there.
> 

You can also trigger this kind of failures with --disable-multi-arch --with-cpu=power4
(for instance) on newer chips.
diff mbox

Patch

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)