* Re: Usage of __attribute__used__ in (system) headers? [not found] <87r4khnbqm.fsf@kepler.schwinge.homeip.net> @ 2013-02-18 19:00 ` Carlos O'Donell 2013-02-18 19:21 ` Richard Henderson 0 siblings, 1 reply; 7+ messages in thread From: Carlos O'Donell @ 2013-02-18 19:00 UTC (permalink / raw) To: Thomas Schwinge; +Cc: libc-alpha, libc-ports 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Usage of __attribute__used__ in (system) headers? 2013-02-18 19:00 ` Usage of __attribute__used__ in (system) headers? Carlos O'Donell @ 2013-02-18 19:21 ` Richard Henderson 2013-02-18 19:41 ` Carlos O'Donell 0 siblings, 1 reply; 7+ messages in thread From: Richard Henderson @ 2013-02-18 19:21 UTC (permalink / raw) To: Carlos O'Donell; +Cc: Thomas Schwinge, libc-alpha, libc-ports On 02/18/2013 11:00 AM, Carlos O'Donell wrote: >> > 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. The annotation should not be attribute used at all, but rather unused. Used says that the object is referenced in some non-visible way by assembly, and thus cannot be elided. Unused says that we understand the variable may not be referenced, and don't warn; but do, in particular, remove it if it is unused. As for whether or not to annotate for non-GCC... I'm ok with either. r~ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Usage of __attribute__used__ in (system) headers? 2013-02-18 19:21 ` Richard Henderson @ 2013-02-18 19:41 ` Carlos O'Donell 2013-02-20 16:06 ` Thomas Schwinge 0 siblings, 1 reply; 7+ messages in thread From: Carlos O'Donell @ 2013-02-18 19:41 UTC (permalink / raw) To: Richard Henderson; +Cc: Thomas Schwinge, libc-alpha, libc-ports On Mon, Feb 18, 2013 at 2:21 PM, Richard Henderson <rth@twiddle.net> wrote: > On 02/18/2013 11:00 AM, Carlos O'Donell wrote: >>> > 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. > > The annotation should not be attribute used at all, but rather unused. > > Used says that the object is referenced in some non-visible way by > assembly, and thus cannot be elided. Unused says that we understand the > variable may not be referenced, and don't warn; but do, in particular, > remove it if it is unused. Yes, thanks for catching that, honestly I forget that it's a "possibly unused" in that case. glibc doesn't have an __attribute_unused__. I'd be happy to review a patch that adds it and fixes up the 8 references to the bare attribute. Cheers, Carlos. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Usage of __attribute__used__ in (system) headers? 2013-02-18 19:41 ` Carlos O'Donell @ 2013-02-20 16:06 ` Thomas Schwinge 2013-02-20 16:25 ` Carlos O'Donell 0 siblings, 1 reply; 7+ messages in thread From: Thomas Schwinge @ 2013-02-20 16:06 UTC (permalink / raw) To: Carlos O'Donell, Richard Henderson; +Cc: libc-alpha, libc-ports, joseph [-- Attachment #1: Type: text/plain, Size: 5230 bytes --] Hi! On Mon, 18 Feb 2013 14:00:03 -0500, Carlos O'Donell <carlos@systemhalted.org> wrote: > I think you should update the MIPS nan.h version to > match the generic version [...] Yes, I already had prepared such a patch and planned to commit it "as obvious", without discussion. Thusly pushed in commit 2636ffe65438af689e12b7977fe8609a6ca07c90: ports/ * sysdeps/mips/bits/nan.h: Align to generic IEEE 754 file. diff --git ports/sysdeps/mips/bits/nan.h ports/sysdeps/mips/bits/nan.h index ffbb3b5..af168ce 100644 --- ports/sysdeps/mips/bits/nan.h +++ ports/sysdeps/mips/bits/nan.h @@ -1,4 +1,4 @@ -/* `NAN' constant for IEEE 754 machines. +/* `NAN' constant for IEEE 754 machines. MIPS version. Copyright (C) 1992-2013 Free Software Foundation, Inc. This file is part of the GNU C Library. @@ -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 - 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. */ +/* IEEE Not A Number. */ +/* 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 I also added <http://sourceware.org/glibc/wiki/Development_Todo/Master?action=diff&rev2=49&rev1=48>. On Mon, 18 Feb 2013 14:41:02 -0500, Carlos O'Donell <carlos@systemhalted.org> wrote: > On Mon, Feb 18, 2013 at 2:21 PM, Richard Henderson <rth@twiddle.net> wrote: > > On 02/18/2013 11:00 AM, Carlos O'Donell wrote: > >>> > static union { unsigned char __c[4]; float __d; } __nan_union > >>> > - __attribute_used__ = { __nan_bytes }; > >>> > + = { __nan_bytes }; > >>> > # define NAN (__nan_union.__d) > >>> > > >>> > #endif /* GCC. */ In commit 72f0ffdcbeb8135d04cf2dc73f8a5f5c7783a283, I first add the missing __attribute_used__: ports/ * sysdeps/mips/bits/nan.h [!__GNUC__] (__nan_union): Add __attribute_used__. diff --git ports/sysdeps/mips/bits/nan.h ports/sysdeps/mips/bits/nan.h index af168ce..71f372d 100644 --- ports/sysdeps/mips/bits/nan.h +++ ports/sysdeps/mips/bits/nan.h @@ -49,7 +49,8 @@ # define __nan_bytes { 0xff, 0xff, 0xbf, 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. */ > >> 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. > > > > The annotation should not be attribute used at all, but rather unused. Good catch, thanks! > glibc doesn't have an __attribute_unused__. It doesn't need one; per <sys/cdefs.h>'s __attribute_used__ definition, we assume that any compiler that supports __attribute__ at all also supports __attribute__ ((unused)). So, I see no reason to introduce __attribute_unused__ bit instead directly use __attribute__ ((unused)); commit c7b275d6b3bceb6b400fa3044d13d1001bc605ca: * sysdeps/ieee754/bits/nan.h [!__GNUC__] (__nan_union): Change __attribute_used__ to __attribute__ ((unused)). ports/ * sysdeps/mips/bits/nan.h [!__GNUC__] (__nan_union): Change __attribute_used__ to __attribute__ ((unused)). diff --git ports/sysdeps/mips/bits/nan.h ports/sysdeps/mips/bits/nan.h index 71f372d..8f4666d 100644 --- ports/sysdeps/mips/bits/nan.h +++ ports/sysdeps/mips/bits/nan.h @@ -50,7 +50,7 @@ # endif static union { unsigned char __c[4]; float __d; } __nan_union - __attribute_used__ = { __nan_bytes }; + __attribute__ ((unused)) = { __nan_bytes }; # define NAN (__nan_union.__d) #endif /* GCC. */ diff --git sysdeps/ieee754/bits/nan.h sysdeps/ieee754/bits/nan.h index d3ab38b..a1e6a51 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 }; + __attribute__ ((unused)) = { __nan_bytes }; # define NAN (__nan_union.__d) #endif /* GCC. */ Grüße, Thomas [-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Usage of __attribute__used__ in (system) headers? 2013-02-20 16:06 ` Thomas Schwinge @ 2013-02-20 16:25 ` Carlos O'Donell 2013-02-20 17:56 ` Thomas Schwinge 0 siblings, 1 reply; 7+ messages in thread From: Carlos O'Donell @ 2013-02-20 16:25 UTC (permalink / raw) To: Thomas Schwinge Cc: Carlos O'Donell, Richard Henderson, libc-alpha, libc-ports, joseph On 02/20/2013 11:06 AM, Thomas Schwinge wrote: > ports/ > * sysdeps/mips/bits/nan.h [!__GNUC__] (__nan_union): Change > __attribute_used__ to __attribute__ ((unused)). > > diff --git ports/sysdeps/mips/bits/nan.h ports/sysdeps/mips/bits/nan.h > index 71f372d..8f4666d 100644 > --- ports/sysdeps/mips/bits/nan.h > +++ ports/sysdeps/mips/bits/nan.h > @@ -50,7 +50,7 @@ > # endif > > static union { unsigned char __c[4]; float __d; } __nan_union > - __attribute_used__ = { __nan_bytes }; > + __attribute__ ((unused)) = { __nan_bytes }; > # define NAN (__nan_union.__d) > > #endif /* GCC. */ > diff --git sysdeps/ieee754/bits/nan.h sysdeps/ieee754/bits/nan.h > index d3ab38b..a1e6a51 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 }; > + __attribute__ ((unused)) = { __nan_bytes }; > # define NAN (__nan_union.__d) > > #endif /* GCC. */ Please fix this to use __unused__. We should not use attributes that are in the user's namespace. Cheers, Carlos. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Usage of __attribute__used__ in (system) headers? 2013-02-20 16:25 ` Carlos O'Donell @ 2013-02-20 17:56 ` Thomas Schwinge 2013-02-20 18:02 ` Carlos O'Donell 0 siblings, 1 reply; 7+ messages in thread From: Thomas Schwinge @ 2013-02-20 17:56 UTC (permalink / raw) To: Carlos O'Donell Cc: Carlos O'Donell, Richard Henderson, libc-alpha, libc-ports, joseph [-- Attachment #1: Type: text/plain, Size: 1551 bytes --] Hi! On Wed, 20 Feb 2013 11:25:22 -0500, Carlos O'Donell <carlos@redhat.com> wrote: > > static union { unsigned char __c[4]; float __d; } __nan_union > > - __attribute_used__ = { __nan_bytes }; > > + __attribute__ ((unused)) = { __nan_bytes }; > Please fix this to use __unused__. > > We should not use attributes that are in the user's namespace. Pushed as commit 50022a93fcc36a0130b9f882709ff69f4b91c0cc: * sysdeps/ieee754/bits/nan.h [!__GNUC__] (__nan_union): Change __attribute__ ((unused)) to __attribute__ ((__unused__)). ports/ * sysdeps/mips/bits/nan.h [!__GNUC__] (__nan_union): Change __attribute__ ((unused)) to __attribute__ ((__unused__)). diff --git ports/sysdeps/mips/bits/nan.h ports/sysdeps/mips/bits/nan.h index 8f4666d..7aa157b 100644 --- ports/sysdeps/mips/bits/nan.h +++ ports/sysdeps/mips/bits/nan.h @@ -50,7 +50,7 @@ # endif static union { unsigned char __c[4]; float __d; } __nan_union - __attribute__ ((unused)) = { __nan_bytes }; + __attribute__ ((__unused__)) = { __nan_bytes }; # define NAN (__nan_union.__d) #endif /* GCC. */ diff --git sysdeps/ieee754/bits/nan.h sysdeps/ieee754/bits/nan.h index a1e6a51..935271a 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__ ((unused)) = { __nan_bytes }; + __attribute__ ((__unused__)) = { __nan_bytes }; # define NAN (__nan_union.__d) #endif /* GCC. */ Grüße, Thomas [-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Usage of __attribute__used__ in (system) headers? 2013-02-20 17:56 ` Thomas Schwinge @ 2013-02-20 18:02 ` Carlos O'Donell 0 siblings, 0 replies; 7+ messages in thread From: Carlos O'Donell @ 2013-02-20 18:02 UTC (permalink / raw) To: Thomas Schwinge Cc: Carlos O'Donell, Richard Henderson, libc-alpha, libc-ports, joseph On 02/20/2013 12:56 PM, Thomas Schwinge wrote: > Hi! > > On Wed, 20 Feb 2013 11:25:22 -0500, Carlos O'Donell <carlos@redhat.com> wrote: >>> static union { unsigned char __c[4]; float __d; } __nan_union >>> - __attribute_used__ = { __nan_bytes }; >>> + __attribute__ ((unused)) = { __nan_bytes }; > >> Please fix this to use __unused__. >> >> We should not use attributes that are in the user's namespace. > > Pushed as commit 50022a93fcc36a0130b9f882709ff69f4b91c0cc: > > * sysdeps/ieee754/bits/nan.h [!__GNUC__] (__nan_union): Change > __attribute__ ((unused)) to __attribute__ ((__unused__)). > > ports/ > * sysdeps/mips/bits/nan.h [!__GNUC__] (__nan_union): Change > __attribute__ ((unused)) to __attribute__ ((__unused__)). Thanks! Cheers, Carlos. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-02-20 18:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <87r4khnbqm.fsf@kepler.schwinge.homeip.net>
2013-02-18 19:00 ` Usage of __attribute__used__ in (system) headers? Carlos O'Donell
2013-02-18 19:21 ` Richard Henderson
2013-02-18 19:41 ` Carlos O'Donell
2013-02-20 16:06 ` Thomas Schwinge
2013-02-20 16:25 ` Carlos O'Donell
2013-02-20 17:56 ` Thomas Schwinge
2013-02-20 18:02 ` Carlos O'Donell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).