* Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion
2014-05-18 21:23 [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion Aurelien Jarno
@ 2014-05-19 6:08 ` Joey Ye
2014-05-19 6:48 ` Aurelien Jarno
2014-06-07 10:54 ` Aurelien Jarno
2014-06-17 22:29 ` Ramana Radhakrishnan
2 siblings, 1 reply; 7+ messages in thread
From: Joey Ye @ 2014-05-19 6:08 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion
2014-05-19 6:08 ` Joey Ye
@ 2014-05-19 6:48 ` Aurelien Jarno
0 siblings, 0 replies; 7+ messages in thread
From: Aurelien Jarno @ 2014-05-19 6:48 UTC (permalink / raw)
To: Joey Ye; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion
2014-05-18 21:23 [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion Aurelien Jarno
2014-05-19 6:08 ` Joey Ye
@ 2014-06-07 10:54 ` Aurelien Jarno
2014-06-17 22:29 ` Ramana Radhakrishnan
2 siblings, 0 replies; 7+ messages in thread
From: Aurelien Jarno @ 2014-06-07 10:54 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion
2014-05-18 21:23 [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion Aurelien Jarno
2014-05-19 6:08 ` Joey Ye
2014-06-07 10:54 ` Aurelien Jarno
@ 2014-06-17 22:29 ` Ramana Radhakrishnan
2014-06-20 22:17 ` Aurelien Jarno
2 siblings, 1 reply; 7+ messages in thread
From: Ramana Radhakrishnan @ 2014-06-17 22:29 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion
2014-06-17 22:29 ` Ramana Radhakrishnan
@ 2014-06-20 22:17 ` Aurelien Jarno
2014-06-20 22:58 ` Joseph S. Myers
0 siblings, 1 reply; 7+ messages in thread
From: Aurelien Jarno @ 2014-06-20 22:17 UTC (permalink / raw)
To: ramrad01; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion
2014-06-20 22:17 ` Aurelien Jarno
@ 2014-06-20 22:58 ` Joseph S. Myers
0 siblings, 0 replies; 7+ messages in thread
From: Joseph S. Myers @ 2014-06-20 22:58 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: ramrad01, gcc-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread