[PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[Aurelien Jarno]

On ARM soft-float, the float to double conversion doesn't convert a sNaN
to qNaN as the IEEE Std 754 standard mandates:

"Under default exception handling, any operation signaling an invalid
operation exception and for which a floating-point result is to be
delivered shall deliver a quiet NaN."

Given the soft float ARM code ignores exceptions and always provides a
result, a float to double conversion of a signaling NaN should return a
quiet NaN. Fix this in extendsfdf2.


2014-05-18  Aurelien Jarno  <aurelien@aurel32.net>
       
	PR target/61219
	* config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN.


Index: libgcc/config/arm/ieee754-df.S
===================================================================
--- libgcc/config/arm/ieee754-df.S	(revision 210588)
+++ libgcc/config/arm/ieee754-df.S	(working copy)
@@ -473,11 +473,15 @@
 	eorne	xh, xh, #0x38000000	@ fixup exponent otherwise.
 	RETc(ne)			@ and return it.
 
-	teq	r2, #0			@ if actually 0
-	do_it	ne, e
-	teqne	r3, #0xff000000		@ or INF or NAN
+	bics	r2, r2, #0xff000000	@ isolate mantissa
+	do_it	eq			@ if 0, that is ZERO or INF,
 	RETc(eq)			@ we are done already.
 
+	teq	r3, #0xff000000		@ check for NAN
+	do_it	eq, t
+	orreq	xh, xh, #0x00080000	@ change to quiet NAN
+	RETc(eq)			@ and return it.
+
 	@ value was denormalized.  We can normalize it now.
 	do_push	{r4, r5, lr}
 	mov	r4, #0x380		@ setup corresponding exponent

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[Joey Ye]

If f2d need fix, then please fix d2f too as current implementation for
both behave similarly.

- Joey

On Mon, May 19, 2014 at 5:23 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On ARM soft-float, the float to double conversion doesn't convert a sNaN
> to qNaN as the IEEE Std 754 standard mandates:
>
> "Under default exception handling, any operation signaling an invalid
> operation exception and for which a floating-point result is to be
> delivered shall deliver a quiet NaN."
>
> Given the soft float ARM code ignores exceptions and always provides a
> result, a float to double conversion of a signaling NaN should return a
> quiet NaN. Fix this in extendsfdf2.
>
>
> 2014-05-18  Aurelien Jarno  <aurelien@aurel32.net>
>
>         PR target/61219
>         * config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN.
>
>
> Index: libgcc/config/arm/ieee754-df.S
> ===================================================================
> --- libgcc/config/arm/ieee754-df.S      (revision 210588)
> +++ libgcc/config/arm/ieee754-df.S      (working copy)
> @@ -473,11 +473,15 @@
>         eorne   xh, xh, #0x38000000     @ fixup exponent otherwise.
>         RETc(ne)                        @ and return it.
>
> -       teq     r2, #0                  @ if actually 0
> -       do_it   ne, e
> -       teqne   r3, #0xff000000         @ or INF or NAN
> +       bics    r2, r2, #0xff000000     @ isolate mantissa
> +       do_it   eq                      @ if 0, that is ZERO or INF,
>         RETc(eq)                        @ we are done already.
>
> +       teq     r3, #0xff000000         @ check for NAN
> +       do_it   eq, t
> +       orreq   xh, xh, #0x00080000     @ change to quiet NAN
> +       RETc(eq)                        @ and return it.
> +
>         @ value was denormalized.  We can normalize it now.
>         do_push {r4, r5, lr}
>         mov     r4, #0x380              @ setup corresponding exponent
>
> --
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                 http://www.aurel32.net

Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[Aurelien Jarno]

On Mon, May 19, 2014 at 02:08:06PM +0800, Joey Ye wrote:
> If f2d need fix, then please fix d2f too as current implementation for
> both behave similarly.

I have done some tests with double to float conversion, and the NaN
behaviour is correct. This is due to specific code handling that in
d2f:

3:      @ chech for NAN
        mvns    r3, r2, asr #21
        bne     5f                      @ simple overflow
        orrs    r3, xl, xh, lsl #12
        do_it   ne, tt
        movne   r0, #0x7f000000
        orrne   r0, r0, #0x00c00000
        RETc(ne)                        @ return NAN

Aurelien

> On Mon, May 19, 2014 at 5:23 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On ARM soft-float, the float to double conversion doesn't convert a sNaN
> > to qNaN as the IEEE Std 754 standard mandates:
> >
> > "Under default exception handling, any operation signaling an invalid
> > operation exception and for which a floating-point result is to be
> > delivered shall deliver a quiet NaN."
> >
> > Given the soft float ARM code ignores exceptions and always provides a
> > result, a float to double conversion of a signaling NaN should return a
> > quiet NaN. Fix this in extendsfdf2.
> >
> >
> > 2014-05-18  Aurelien Jarno  <aurelien@aurel32.net>
> >
> >         PR target/61219
> >         * config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN.
> >
> >
> > Index: libgcc/config/arm/ieee754-df.S
> > ===================================================================
> > --- libgcc/config/arm/ieee754-df.S      (revision 210588)
> > +++ libgcc/config/arm/ieee754-df.S      (working copy)
> > @@ -473,11 +473,15 @@
> >         eorne   xh, xh, #0x38000000     @ fixup exponent otherwise.
> >         RETc(ne)                        @ and return it.
> >
> > -       teq     r2, #0                  @ if actually 0
> > -       do_it   ne, e
> > -       teqne   r3, #0xff000000         @ or INF or NAN
> > +       bics    r2, r2, #0xff000000     @ isolate mantissa
> > +       do_it   eq                      @ if 0, that is ZERO or INF,
> >         RETc(eq)                        @ we are done already.
> >
> > +       teq     r3, #0xff000000         @ check for NAN
> > +       do_it   eq, t
> > +       orreq   xh, xh, #0x00080000     @ change to quiet NAN
> > +       RETc(eq)                        @ and return it.
> > +
> >         @ value was denormalized.  We can normalize it now.
> >         do_push {r4, r5, lr}
> >         mov     r4, #0x380              @ setup corresponding exponent
> >
> > --
> > Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> > aurelien@aurel32.net                 http://www.aurel32.net
> 

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[Aurelien Jarno]

Ping. Note that PR61219 is a duplicate of PR59833, so this patch
actually fix PR59833.

On Sun, May 18, 2014 at 11:23:38PM +0200, Aurelien Jarno wrote:
> On ARM soft-float, the float to double conversion doesn't convert a sNaN
> to qNaN as the IEEE Std 754 standard mandates:
> 
> "Under default exception handling, any operation signaling an invalid
> operation exception and for which a floating-point result is to be
> delivered shall deliver a quiet NaN."
> 
> Given the soft float ARM code ignores exceptions and always provides a
> result, a float to double conversion of a signaling NaN should return a
> quiet NaN. Fix this in extendsfdf2.
> 
> 
> 2014-05-18  Aurelien Jarno  <aurelien@aurel32.net>
>        
> 	PR target/61219
> 	* config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN.
> 
> 
> Index: libgcc/config/arm/ieee754-df.S
> ===================================================================
> --- libgcc/config/arm/ieee754-df.S	(revision 210588)
> +++ libgcc/config/arm/ieee754-df.S	(working copy)
> @@ -473,11 +473,15 @@
>  	eorne	xh, xh, #0x38000000	@ fixup exponent otherwise.
>  	RETc(ne)			@ and return it.
>  
> -	teq	r2, #0			@ if actually 0
> -	do_it	ne, e
> -	teqne	r3, #0xff000000		@ or INF or NAN
> +	bics	r2, r2, #0xff000000	@ isolate mantissa
> +	do_it	eq			@ if 0, that is ZERO or INF,
>  	RETc(eq)			@ we are done already.
>  
> +	teq	r3, #0xff000000		@ check for NAN
> +	do_it	eq, t
> +	orreq	xh, xh, #0x00080000	@ change to quiet NAN
> +	RETc(eq)			@ and return it.
> +
>  	@ value was denormalized.  We can normalize it now.
>  	do_push	{r4, r5, lr}
>  	mov	r4, #0x380		@ setup corresponding exponent
> 
> -- 
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                 http://www.aurel32.net

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[Ramana Radhakrishnan]

On Sun, May 18, 2014 at 10:23 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On ARM soft-float, the float to double conversion doesn't convert a sNaN
> to qNaN as the IEEE Std 754 standard mandates:
>
> "Under default exception handling, any operation signaling an invalid
> operation exception and for which a floating-point result is to be
> delivered shall deliver a quiet NaN."
>
> Given the soft float ARM code ignores exceptions and always provides a
> result, a float to double conversion of a signaling NaN should return a
> quiet NaN. Fix this in extendsfdf2.
>
>
> 2014-05-18  Aurelien Jarno  <aurelien@aurel32.net>
>
>         PR target/61219
>         * config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN.

Ok if no regressions along with a testcase to catch this case please
and fixing the PR number

Sorry about the slow review.

Ramana

>
>
> Index: libgcc/config/arm/ieee754-df.S
> ===================================================================
> --- libgcc/config/arm/ieee754-df.S      (revision 210588)
> +++ libgcc/config/arm/ieee754-df.S      (working copy)
> @@ -473,11 +473,15 @@
>         eorne   xh, xh, #0x38000000     @ fixup exponent otherwise.
>         RETc(ne)                        @ and return it.
>
> -       teq     r2, #0                  @ if actually 0
> -       do_it   ne, e
> -       teqne   r3, #0xff000000         @ or INF or NAN
> +       bics    r2, r2, #0xff000000     @ isolate mantissa
> +       do_it   eq                      @ if 0, that is ZERO or INF,
>         RETc(eq)                        @ we are done already.
>
> +       teq     r3, #0xff000000         @ check for NAN
> +       do_it   eq, t
> +       orreq   xh, xh, #0x00080000     @ change to quiet NAN
> +       RETc(eq)                        @ and return it.
> +
>         @ value was denormalized.  We can normalize it now.
>         do_push {r4, r5, lr}
>         mov     r4, #0x380              @ setup corresponding exponent
>
> --
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                 http://www.aurel32.net

Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[Aurelien Jarno]

On Tue, Jun 17, 2014 at 11:29:01PM +0100, Ramana Radhakrishnan wrote:
> On Sun, May 18, 2014 at 10:23 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On ARM soft-float, the float to double conversion doesn't convert a sNaN
> > to qNaN as the IEEE Std 754 standard mandates:
> >
> > "Under default exception handling, any operation signaling an invalid
> > operation exception and for which a floating-point result is to be
> > delivered shall deliver a quiet NaN."
> >
> > Given the soft float ARM code ignores exceptions and always provides a
> > result, a float to double conversion of a signaling NaN should return a
> > quiet NaN. Fix this in extendsfdf2.
> >
> >
> > 2014-05-18  Aurelien Jarno  <aurelien@aurel32.net>
> >
> >         PR target/61219
> >         * config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN.
> 
> Ok if no regressions along with a testcase to catch this case please
> and fixing the PR number

I have added such a testcase in the new version below. I didn't add it
in the target specific subdirectory, as other architectures might be
affected. Actually aarch64 is also affected, though for different
reasons. I have tested that the testcase correctly catch the issue, and
that the whole patch doesn't cause any regression.

Please find the new patch below. If it is fine, I would appreciate if
someone can commit the patch, as I don't have SVN write access (though I
have done the copyright assignment stuff).

> Sorry about the slow review.

No problem, I am also very often short on time.

Aurelien



gcc/testsuite/ChangeLog

2014-06-18  Aurelien Jarno  <aurelien@aurel32.net>

	PR target/59833
	* gcc.dg/pr59833.c: New testcase.

Index: gcc/testsuite/gcc.dg/pr59833.c
===================================================================
--- gcc/testsuite/gcc.dg/pr59833.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr59833.c	(working copy)
@@ -0,0 +1,15 @@
+/* PR target/59833 */
+/* { dg-do run } */
+/* { dg-options "-lm" } */
+
+extern int __issignaling (double);
+
+int
+main ()
+{
+  float sNaN = __builtin_nansf ("");
+  double x = (double) sNaN;
+  if (__issignaling (x))
+      __builtin_abort ();
+  return 0;
+}


libgcc/ChangeLog
2014-06-18  Aurelien Jarno  <aurelien@aurel32.net>

	PR target/59833
	* config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN.

Index: libgcc/config/arm/ieee754-df.S
===================================================================
--- libgcc/config/arm/ieee754-df.S	(revision 211428)
+++ libgcc/config/arm/ieee754-df.S	(working copy)
@@ -473,11 +473,15 @@
 	eorne	xh, xh, #0x38000000	@ fixup exponent otherwise.
 	RETc(ne)			@ and return it.
 
-	teq	r2, #0			@ if actually 0
-	do_it	ne, e
-	teqne	r3, #0xff000000		@ or INF or NAN
+	bics	r2, r2, #0xff000000	@ isolate mantissa
+	do_it	eq			@ if 0, that is ZERO or INF
 	RETc(eq)			@ we are done already.
 
+	teq	r3, #0xff000000		@ check for NAN
+	do_it	eq, t
+	orreq	xh, xh, #0x00080000	@ change to quiet NAN
+	RETc(eq)			@ and return it.
+
 	@ value was denormalized.  We can normalize it now.
 	do_push	{r4, r5, lr}
 	mov	r4, #0x380		@ setup corresponding exponent


-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[Joseph S. Myers]

On Sat, 21 Jun 2014, Aurelien Jarno wrote:

> Index: gcc/testsuite/gcc.dg/pr59833.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr59833.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/pr59833.c	(working copy)
> @@ -0,0 +1,15 @@
> +/* PR target/59833 */
> +/* { dg-do run } */
> +/* { dg-options "-lm" } */
> +
> +extern int __issignaling (double);

__issignaling is a recent glibc addition, not a standard C library 
function; you can't assume it's available in a test without using a 
suitable effective-target that tests for whether it's available.

I believe linking with -lm is the default in the testsuite so shouldn't 
need specifying explicitly.

-- 
Joseph S. Myers
joseph@codesourcery.com