From owner-svn-src-all@FreeBSD.ORG Wed Dec 14 15:19:04 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0202A1065676; Wed, 14 Dec 2011 15:19:04 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail07.syd.optusnet.com.au (mail07.syd.optusnet.com.au [211.29.132.188]) by mx1.freebsd.org (Postfix) with ESMTP id 7C6DF8FC0A; Wed, 14 Dec 2011 15:19:03 +0000 (UTC) Received: from c211-28-227-231.carlnfd1.nsw.optusnet.com.au (c211-28-227-231.carlnfd1.nsw.optusnet.com.au [211.28.227.231]) by mail07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id pBEFIxBe012953 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 15 Dec 2011 02:19:01 +1100 Date: Thu, 15 Dec 2011 02:18:59 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ed Schouten In-Reply-To: <201112140909.pBE99bS3090646@svn.freebsd.org> Message-ID: <20111214234615.B3839@besplex.bde.org> References: <201112140909.pBE99bS3090646@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r228495 - head/sys/sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Dec 2011 15:19:04 -0000 On Wed, 14 Dec 2011, Ed Schouten wrote: > Log: > Slightly alter the C1X definitions in in cdefs.h: > > - Add _Alignas(). Unfortunately this macro is only partially functional. > The C1X standard will allow both an integer and a type name to be > passed to this macro, while this macro only allows an integer. To be > portable, one must use _Alignas(_Alignof(double)) to use type names. This is still quite broken. > - Don't do _Static_assert() when __COUNTER__ is not supported. We'd > better keep this implementation robust and allow it to be used in > header files, without mysteriously breaking older compilers. Not sure about this. > Modified: head/sys/sys/cdefs.h > ============================================================================== > --- head/sys/sys/cdefs.h Wed Dec 14 08:52:27 2011 (r228494) > +++ head/sys/sys/cdefs.h Wed Dec 14 09:09:37 2011 (r228495) > @@ -222,6 +222,7 @@ > * Keywords added in C1X. > */ > #if defined(__cplusplus) && __cplusplus >= 201103L > +#define _Alignas(e) alignas(e) > #define _Alignof(e) alignof(e) > #define _Noreturn [[noreturn]] > #define _Static_assert(e, s) static_assert(e, s) > @@ -231,21 +232,23 @@ > #else > /* Not supported. Implement them manually. */ > #ifdef __GNUC__ > +#define _Alignas(e) __attribute__((__aligned__(e))) This breaks versions of gcc that don't have __attribute__ or __aligned__. Elsewhere in the same file, use of __attribute__ is restricted starting with the condition: #if __GNUC__ == 2 && __GNUC_MINOR__ >= 5 && __GNUC_MINOR__ < 7 && \ !defined(__INTEL_COMPILER) (this actually gives the starting point for 2 attributes, with many necessary and unnecessary convolutions -- see below). and use of __aligned__ is carefully restricted by the condition #if __GNUC_PREREQ__(2, 7) || defined(__INTEL_COMPILER) (now it starts with gcc-2.7 where the above starts with gcc-2.5, since __aligned__ apparently wasn't in the original set of attributes). The other places give a definition of __aligned() that can almost be used here. There is a bogus one for lint and a duplicate one for __INTEL_COMPILER. But there is none for !__GNUC_PREREQ__(2, 7) && !__INTEL_COMPILER. That is, all cases where the compiler is either gcc-before-2.6, or non-gcc other than lint and icc, are already broken. > #define _Alignof(e) __alignof__(e) This one mainly has a style bug. __alignof__ would be be spelled __alignof if this were FreeBSD code. gcc has accepted all of alignof(), __alignof() and __alignof__() for a long time. I checked this for some gccs back to gcc-2.95.4. It might even work for gcc-1. FreeBSD uses the extra trailing underscores for __attribute__(()) and individual attributes because unlike for __alignof() and __typeof(), gcc used to require them and changing this now would give style bugs. gcc-4.2.1 accepts the __aligned__ attribute, but gcc-3.3.3 doesn't. I don't know when __attribute(()) started working. I checked it for some gccs back to gcc-2.95.4, and all accepted it. > #define _Noreturn __attribute__((__noreturn__)) Elsewhere in this file, the __noreturn__ attribute is handled like the __aligned__ attribute, but more carefully, with many style bugs but no bugs as far as I can see: - For lint, the macro that uses it (__dead2) is bogusly defined as nothing. I thought that lint bogusly defines __GNUC__, but can't find this now. Lint should not define __GNUC__ since it only supports a tiny subsets of gcc features like __attribute__(), even with -g. If lint defined __GNUC__, then the extra ifdef for it would be be justified for some of the macros including __dead2/_Noreturn, since __dead2 has no semantic effect so it is safe to make it null for compilers that don't support it. If lint doesn't define __GNUC__ then for __dead2 and now hopefully for _Noreturn, an extra ifdef is not needed since lint is given a null definition in the section for nondescript non-gcc compilers. - For non-gcc, or gcc < 2.5, and not icc, __dead2 is defined as nothing under the condition: #if !__GNUC_PREREQ__(2, 5) && !defined(__INTEL_COMPILER) - For gcc-2.5 and gcc-2.6, and icc, __attribute__((__noreturn__)) is used to define __dead2 under the condition: #if __GNUC__ == 2 && __GNUC_MINOR__ >= 5 && __GNUC_MINOR__ < 7 && \ !defined(__INTEL_COMPILER) Note the style bug in this condition (it doesn't use GNUC_PREREQ()). - For gcc >= 2.7, the definition of __dead2 is duplicated under the condition: #if __GNUC_PREREQ(2, 7) - For icc, the definition of __dead2 is duplicated yet again, under the condition: #if defined(__INTEL_COMPILER) Note the style bug in this (defined() when ifdef would do). There are 2 other definitions under the gcc-2.5, gcc-2.6, or icc ifdef. Only the one for __unused belongs there, since its attribute is new in gcc-2.7. The others should be define without duplication for gcc >= 2.5. Under the the gcc >= 2.7 and separate icc condition, there are definitions to use 7 attributes. All of these have style bugs or worse: - __noreturn__ attribute. Shouldn't be duplicated. - __const__ attribute. Shouldn't be duplicated. - __unused__ attribute. May actually be correct. - __used__ attribute. Doesn't belong here, since it is much newer than gcc-2.7. It is in gcc-3.3.3 but not in gcc-3.2.2. - __packed__ attribute. Not defined for older gcc. - __aligned__ attribute. Not defined for older gcc. - __section attribute. Not defined for older gcc. These definitions have lots more of bugs: - random order - all duplicated verbatim for icc. > #define _Thread_local __thread This has similar problems. __thread doesn't work for at least gcc-2.95.4 and gcc-3.2.2, but works (compiles at least) for gcc-3.3.3. > #else > +#define _Alignas(e) > #define _Alignof(e) __offsetof(struct { char __a; e __b; }, __b) > #define _Noreturn > #define _Thread_local > #endif _Alignas() can be defined in terms of __align() for all cases without repeating the ifdefs, after fixing old bugs for __align(). _Noreturn can't be handled so simply. Its use has syntactic differences. I have already complained about using it in breaking portability in a critical header. All these messy ifdefs here are just obfuscations if they don't work for . After writing the above and running some more tests, the limits of the breakage are clearer: - the __noreturn__ attribute is apparently new in gcc-2.5. Before that, the old __pure macro presumably worked. __pure was implemented as __volatile, and has the same syntax as C1x _Noreturn (it must be before the function name). - the __noreturn__ attribute (and presumably other attributes) is a syntax errors if it is used before a function name (presumably the same for before a variable name) in gcc-2.95.4. It works before a function name in gcc-3.2.2. Thus to work with all compilers that it used to work with, must either use only __dead2 after function names like it used to, or if it wants to use _Noreturn then it must use both _Noreturn before function names (to support newer compilers) and __dead2 after function names (to support older compilers), and _Noreturn must be more carefully ifdefed here. There are the following compilers and ranges of compilers: - not gcc and not icc and not clang: both macros null - icc: hopefully like newer gcc - clang: hopefully like newer gcc - gcc-1: like gcc-2.early - gcc-2.early: define __dead2 as null; define _Noreturn as either __volatile (to bring back the special support for gcc-1 that I deprecated in 1996 and removed in 1998) or define _Noreturn as null (to preserve the removal, so that gcc-2.early is treated as a nondescript compiler). _Noreturn restores the syntax differences that made the gcc-2.early support touch much more than . Now we have the syntax differences in another way, so restoring the gcc-2.early support only takes 1 line. - gcc-2.5 through gcc-3.2.2 or a little earlier: must define _Noreturn as null, and to restore functionality in , restore the uses of __dead2 there. Don't forget to fix the new style bugs there (see the 4.4BSD version for normal style with directives before function names). - post gcc-3.2.2: can define _Noreturn as non-null and __dead2 as null, or vice versa, or both non-null. Bruce