This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Usage of __attribute__used__ in (system) headers?


On Fri, Feb 15, 2013 at 4:54 AM, Thomas Schwinge
<thomas@codesourcery.com> wrote:
> Hi!
>
> I found that sysdeps/ieee754/bits/nan.h is different from
> ports/sysdeps/mips/bits/nan.h in that the former has seen the following
> change, whereas the latter has not been updated accordingly:

Good catch!

Adding libc-ports since my suggestion is the opposite of yours.

> | commit b575c52b86fe0c00adec925e356eb72cf95b23a7
> | Author: Ulrich Drepper <drepper@redhat.com>
> | Date:   Fri Apr 16 22:04:55 2004 +0000
> |
> |     Update.
> |
> |     2004-04-16  Ulrich Drepper  <drepper@redhat.com>
> |
> |       * sysdeps/ieee754/bits/nan.h (__nan_union): Add __attribute_used__
> |       attribute to keep gcc quiet.
> |
> | [...]
> | --- sysdeps/ieee754/bits/nan.h
> | +++ sysdeps/ieee754/bits/nan.h
> | @@ -46,7 +46,8 @@
> |  #  define __nan_bytes                { 0, 0, 0xc0, 0x7f }
> |  # endif
> |
> | -static union { unsigned char __c[4]; float __d; } __nan_union = { __nan_bytes };
> | +static union { unsigned char __c[4]; float __d; } __nan_union
> | +    __attribute_used__ = { __nan_bytes };
> |  # define NAN (__nan_union.__d)
> |
> |  #endif       /* GCC.  */
>
> This code in question (cited/changed above) is only ever used for
> non-__GNUC__-defining compilers.  If my reading of misc/sys/cdefs.h is
> correct, for these compilers, the attribute declaration is not applied
> anyway.  So, instead of adding it to the MIPS copy of that file, I
> suggest we remove it from the generic one.  OK?
>
> diff --git sysdeps/ieee754/bits/nan.h sysdeps/ieee754/bits/nan.h
> index d3ab38b..00ad0b5 100644
> --- sysdeps/ieee754/bits/nan.h
> +++ sysdeps/ieee754/bits/nan.h
> @@ -46,7 +46,7 @@
>  # endif
>
>  static union { unsigned char __c[4]; float __d; } __nan_union
> -    __attribute_used__ = { __nan_bytes };
> +  = { __nan_bytes };
>  # define NAN   (__nan_union.__d)
>
>  #endif /* GCC.  */

I disagree, it's useful to mark the non-GCC version with
an __attribute_used__ such that in the future we might
use another compiler without problems.

I think you should update the MIPS nan.h version to
match the generic version, and on top of that you should
also apply the following untested patch to minimize the
diff between the versions.

diff --git a/ports/sysdeps/mips/bits/nan.h b/ports/sysdeps/mips/bits/nan.h
index ffbb3b5..a768dd7 100644
--- a/ports/sysdeps/mips/bits/nan.h
+++ b/ports/sysdeps/mips/bits/nan.h
@@ -13,7 +13,7 @@
    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
+   License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */

 #ifndef _MATH_H
@@ -21,20 +21,21 @@
 #endif


-/* IEEE Not A Number (QNaN). Note that MIPS has the QNaN and SNaN patterns
+/* IEEE Not A Number.  */
+/* By default a NaN is a QNaN. Note that MIPS has the QNaN and SNaN patterns
    reversed compared to most other architectures. The IEEE spec left
    the definition of this open to implementations, and for MIPS the top
    bit of the mantissa must be SET to indicate a SNaN.  */

 #if __GNUC_PREREQ(3,3)

-# define NAN	(__builtin_nanf(""))
+# define NAN	(__builtin_nanf (""))

 #elif defined __GNUC__

 # define NAN \
-  (__extension__                                                            \
-   ((union { unsigned __l __attribute__((__mode__(__SI__))); float __d; })  \
+  (__extension__							      \
+   ((union { unsigned __l __attribute__ ((__mode__ (__SI__))); float __d; })  \
     { __l: 0x7fbfffffUL }).__d)

 #else
---

Which then reduces the diff to this:
--- sysdeps/ieee754/bits/nan.h	2013-02-18 13:51:00.618157335 -0500
+++ ports/sysdeps/mips/bits/nan.h	2013-02-18 13:51:49.836908372 -0500
@@ -22,6 +22,10 @@


 /* IEEE Not A Number.  */
+/* A normal NaN is a QNaN. Note that MIPS has the QNaN and SNaN patterns
+   reversed compared to most other architectures. The IEEE spec left
+   the definition of this open to implementations, and for MIPS the top
+   bit of the mantissa must be SET to indicate a SNaN.  */

 #if __GNUC_PREREQ(3,3)

@@ -32,21 +36,20 @@
 # define NAN \
   (__extension__							      \
    ((union { unsigned __l __attribute__ ((__mode__ (__SI__))); float __d; })  \
-    { __l: 0x7fc00000UL }).__d)
+    { __l: 0x7fbfffffUL }).__d)

 #else

 # include <endian.h>

 # if __BYTE_ORDER == __BIG_ENDIAN
-#  define __nan_bytes		{ 0x7f, 0xc0, 0, 0 }
+#  define __nan_bytes		{ 0x7f, 0xbf, 0xff, 0xff }
 # endif
 # if __BYTE_ORDER == __LITTLE_ENDIAN
-#  define __nan_bytes		{ 0, 0, 0xc0, 0x7f }
+#  define __nan_bytes		{ 0xff, 0xff, 0xbf, 0x7f }
 # endif

-static union { unsigned char __c[4]; float __d; } __nan_union
-    __attribute_used__ = { __nan_bytes };
+static union { unsigned char __c[4]; float __d; } __nan_union = {
__nan_bytes };
 # define NAN	(__nan_union.__d)

 #endif	/* GCC.  */
---

Which shows clearly where the difference lies, and with your change
the diff would be even smaller.

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]