Re: [attention machine maintainers] [PATCH] <math.h> issignaling

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

I plan to shortly "move" this patch (as present in the
tschwinge/issignaling branch) into master with the following changes
merged in, unless there are any further comments at this point.


On Tue, 26 Mar 2013 17:33:46 +0000, "Joseph S. Myers" <joseph@codesourcery.com> wrote:
> On Fri, 22 Mar 2013, Thomas Schwinge wrote:
> 
> > PowerPC -m64:
> > 
> > math/basic-test.out:
> >     Failure:  double x = (double) (long double) sNaN, !issignaling
> > 
> > That is, this type cast -- which is a IEEE 754-2008 general-computational
> > convertFormat operation (IEEE 754-2008, 5.4.2) -- does not turn the sNaN
> > into a qNaN (whilst raising an INVALID exception; not checked here),
> > which is contrary to IEEE 754-2008 5.1 and 7.2.  This I consider a
> > compiler issue (powerpc-linux-gnu-gcc (Sourcery CodeBench 2012.09-92)
> > 4.7.2).
> 
> Such a bug (assuming present in GCC trunk for 4.9) should be filed in GCC 
> Bugzilla (and I suppose a new math-tests.h macro used to disable the test 
> in question for older compilers).  Though I guess it might only be desired 
> to change this for -fsignaling-nans.

I will add the following, file in GCC Bugzilla, add its PR number to the
math_tests.h comment (also for the 32-bit x86 issue discussed before):

--- math/basic-test.c
+++ math/basic-test.c
@@ -175,10 +175,13 @@ NAME (void)								      \
     {									      \
       x1 = (FLOAT) sNaN_var;						      \
       check (" "#FLOAT" x = ("#FLOAT") ("#DOUBLE") sNaN, isnan", isnan (x1)); \
-      /* Upon type conversion, a sNaN is converted into a qNaN plus an */     \
-      /* INVALID exception (not checked here).  */			      \
-      check (" "#FLOAT" x = ("#FLOAT") ("#DOUBLE") sNaN, !issignaling",	      \
-	     !issignaling (x1));					      \
+      if (SNAN_TESTS_TYPE_CAST)						      \
+	{								      \
+	  /* Upon type conversion, a sNaN is converted into a qNaN plus an */ \
+	  /* INVALID exception (not checked here).  */			      \
+	  check (" "#FLOAT" x = ("#FLOAT") ("#DOUBLE") sNaN, !issignaling",   \
+		 !issignaling (x1));					      \
+	}								      \
       }									      \
   x2 = (FLOAT) Inf_var;							      \
   check (" "#FLOAT" x = ("#FLOAT") ("#DOUBLE") Inf", isinf (x2) != 0);	      \
--- sysdeps/generic/math-tests.h
+++ sysdeps/generic/math-tests.h
@@ -34,3 +34,9 @@
   (sizeof (x) == sizeof (float) ? SNAN_TESTS_float			\
    : sizeof (x) == sizeof (double) ? SNAN_TESTS_double			\
    : SNAN_TESTS_long_double)
+
+/* Indicate whether to run tests involving type casts of sNaN values.  These
+   are run unless overridden.  */
+#ifndef SNAN_TESTS_TYPE_CAST
+# define SNAN_TESTS_TYPE_CAST	1
+#endif
--- /dev/null
+++ sysdeps/powerpc/math-tests.h
@@ -0,0 +1,26 @@
+/* Configuration for math tests.  PowerPC version.
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* On PowerPC, in versions of GCC up to at least 4.7.2, a type cast -- which is
+   a IEEE 754-2008 general-computational convertFormat operation (IEEE
+   754-2008, 5.4.2) -- does not turn a sNaN into a qNaN (whilst raising an
+   INVALID exception), which is contrary to IEEE 754-2008 5.1 and 7.2.  This
+   renders certain tests infeasible in this scenario.  */
+#define SNAN_TESTS_TYPE_CAST	0
+
+#include_next <math-tests.h>


> > --- NEWS
> > +++ NEWS
> > @@ -26,6 +26,8 @@ Version 2.18
> >  
> >  * Added a benchmark framework to track performance of functions in glibc.
> >  
> > +* New <math.h> macro named issignaling to check for a signaling NaN (sNaN).
> > +  This is currently GNU-specific.
> 
> "GNU-specific" is a bit misleading there.  I think you mean "only defined 
> if _GNU_SOURCE is defined" (but you should note it comes from draft TS 
> 18661).
> 
> > +@comment math.h
> > +@comment GNU
> > +@deftypefn {Macro} int issignaling (@emph{float-type} @var{x})
> > +This macro returns a nonzero value if @var{x} is a signaling NaN
> > +(sNaN).  It is a GNU extension.
> 
> A GNU extension based on draft TS 18661.

I plan to change the wording as follows in both cases:

--- NEWS
+++ NEWS
@@ -27,7 +27,7 @@ Version 2.18
 * Added a benchmark framework to track performance of functions in glibc.
 
 * New <math.h> macro named issignaling to check for a signaling NaN (sNaN).
-  This is currently GNU-specific.
+  It is based on draft TS 18661 and currently enabled as a GNU extension.
 
 Version 2.17
 
--- manual/arith.texi
+++ manual/arith.texi
@@ -391,7 +391,8 @@ to
 @comment GNU
 @deftypefn {Macro} int issignaling (@emph{float-type} @var{x})
 This macro returns a nonzero value if @var{x} is a signaling NaN
-(sNaN).  It is a GNU extension.
+(sNaN).  It is based on draft TS 18661 and currently enabled as a GNU
+extension.
 @end deftypefn
 
 Another set of floating-point classification functions was provided by


> > +#undef __issignaling
> 
> Why the #undef?

In case a machine-specific header file is defining a macro of the same
name; though you're correct that is not needed at this point.  Removed.

> > +int __issignaling (double x)
> 
> Return type on a separate line.

Changed.

--- sysdeps/ieee754/dbl-64/s_issignaling.c
+++ sysdeps/ieee754/dbl-64/s_issignaling.c
@@ -19,8 +19,8 @@
 #include <math.h>
 #include <math_private.h>
 
-#undef __issignaling
-int __issignaling (double x)
+int
+__issignaling (double x)
 {
 #ifdef HIGH_ORDER_BIT_IS_SET_FOR_SNAN
   u_int32_t hxi;
--- sysdeps/ieee754/dbl-64/wordsize-64/s_issignaling.c
+++ sysdeps/ieee754/dbl-64/wordsize-64/s_issignaling.c
@@ -19,8 +19,8 @@
 #include <math.h>
 #include <math_private.h>
 
-#undef __issignaling
-int __issignaling (double x)
+int
+__issignaling (double x)
 {
   u_int64_t xi;
   EXTRACT_WORDS64 (xi, x);
--- sysdeps/ieee754/flt-32/s_issignalingf.c
+++ sysdeps/ieee754/flt-32/s_issignalingf.c
@@ -19,8 +19,8 @@
 #include <math.h>
 #include <math_private.h>
 
-#undef __issignalingf
-int __issignalingf (float x)
+int
+__issignalingf (float x)
 {
   u_int32_t xi;
   GET_FLOAT_WORD (xi, x);
--- sysdeps/ieee754/ldbl-128/s_issignalingl.c
+++ sysdeps/ieee754/ldbl-128/s_issignalingl.c
@@ -19,8 +19,8 @@
 #include <math.h>
 #include <math_private.h>
 
-#undef __issignalingl
-int __issignalingl (long double x)
+int
+__issignalingl (long double x)
 {
   u_int64_t hxi, lxi __attribute__ ((unused));
   GET_LDOUBLE_WORDS64 (hxi, lxi, x);
--- sysdeps/ieee754/ldbl-128ibm/s_issignalingl.c
+++ sysdeps/ieee754/ldbl-128ibm/s_issignalingl.c
@@ -19,8 +19,8 @@
 #include <math.h>
 #include <math_private.h>
 
-#undef __issignalingl
-int __issignalingl (long double x)
+int
+__issignalingl (long double x)
 {
   u_int64_t xi;
   /* For inspecting NaN status, we only have to look at the first of the pair
--- sysdeps/ieee754/ldbl-96/s_issignalingl.c
+++ sysdeps/ieee754/ldbl-96/s_issignalingl.c
@@ -19,8 +19,8 @@
 #include <math.h>
 #include <math_private.h>
 
-#undef __issignalingl
-int __issignalingl (long double x)
+int
+__issignalingl (long double x)
 {
   u_int32_t exi, hxi, lxi;
   GET_LDOUBLE_WORDS (exi, hxi, lxi, x);


I also removed one Â#error untested after Dave's testing:

--- sysdeps/ieee754/ldbl-128/s_issignalingl.c
+++ sysdeps/ieee754/ldbl-128/s_issignalingl.c
@@ -31,7 +31,6 @@ int __issignalingl (long double x)
   return ((hxi & UINT64_C (0x7fff800000000000))
           == UINT64_C (0x7fff800000000000));
 #else
-# error untested
   /* To keep the following comparison simple, toggle the quiet/signaling bit,
      so that it is set for sNaNs.  This is inverse to IEEE 754-2008 (as well as
      common practice for IEEE 754-1985).  */


> > +{
> > +#ifdef HIGH_ORDER_BIT_IS_SET_FOR_SNAN
> > +  u_int32_t hxi;
> 
> (I tend to think that the C99 names such as uint32_t should be preferred, 
> but since the libm code seems to be using the legacy names universally, 
> any such change should probably first be made globally to existing code if 
> there's consensus, rather than making this code in particular different.)

Exactly my thoughts.


GrÃÃe,
 Thomas

Attachment: pgp8AOLibg2Dx.pgp
Description: PGP signature