Messages in this thread |  | | From | Nick Desaulniers <> | Date | Tue, 21 Aug 2018 01:09:17 -0700 | Subject | Re: [PATCH] compiler-gcc: get back Clang build |
| |
Thanks for noticing, and sending this patch. I'm happy to see others testing with Clang. I noticed this too near the end of the day https://github.com/ClangBuiltLinux/linux/issues/27.
On Mon, Aug 20, 2018 at 11:48 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > Commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6") > missed the fact that <linux/compiler-gcc.h> is included by Clang > as well as by GCC. > > Clang actually defines __GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__ > and it looks like GCC 4.2.1. > > $ scripts/gcc-version.sh -p clang > 040201
If you too are wondering why, see: https://reviews.llvm.org/D51011#1206981.
> > If you try to build the kernel with Clang, you will get the > "Sorry, your compiler is too old - please upgrade it." > followed by a bunch of "unknown attribute" warnings. > > Add !defined(__clang__) to the minimum version check.
I think this may eventually be part of a more-proper compiler check from the C preprocessor.
> > Also, revive the version test blocks for versions >= 4.2.1 > in order to disable features not supported by Clang.
Eh, this I'm not too enthused to see these being added back.
> > Fixes: cafa0010cd51 ("Raise the minimum required gcc version to 4.6") > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > include/linux/compiler-gcc.h | 34 ++++++++++++++++++++++++++++------ > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > index 250b9b7..8e41fd2 100644 > --- a/include/linux/compiler-gcc.h > +++ b/include/linux/compiler-gcc.h > @@ -10,7 +10,7 @@ > + __GNUC_MINOR__ * 100 \ > + __GNUC_PATCHLEVEL__) > > -#if GCC_VERSION < 40600 > +#if !defined(__clang__) && GCC_VERSION < 40600 > # error Sorry, your compiler is too old - please upgrade it. > #endif > > @@ -163,7 +163,16 @@ > #define __used __attribute__((__used__)) > #define __compiler_offsetof(a, b) \ > __builtin_offsetof(a, b) > +#define __compiletime_object_size(obj) __builtin_object_size(obj, 0) > > +/* > + * GCC version specific checks > + * > + * Keep the version checks for 4.2.1 or newer > + * because Clang defines GCC_VERSION as 40201 > + */ > + > +#if GCC_VERSION >= 40300 > /* Mark functions as cold. gcc will assume any path leading to a call > * to them will be unlikely. This means a lot of manual unlikely()s > * are unnecessary now for any paths leading to the usual suspects > @@ -182,15 +191,20 @@ > > #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__) > > +#ifndef __CHECKER__ > +# define __compiletime_warning(message) __attribute__((warning(message))) > +# define __compiletime_error(message) __attribute__((error(message))) > +#endif /* __CHECKER__ */ > +#endif /* GCC_VERSION >= 40300 */ > + > +#if GCC_VERSION >= 40400 > #define __optimize(level) __attribute__((__optimize__(level))) > #define __nostackprotector __optimize("no-stack-protector") > +#endif /* GCC_VERSION >= 40400 */ > > -#define __compiletime_object_size(obj) __builtin_object_size(obj, 0) > +#if GCC_VERSION >= 40500 > > #ifndef __CHECKER__ > -#define __compiletime_warning(message) __attribute__((warning(message))) > -#define __compiletime_error(message) __attribute__((error(message))) > - > #ifdef LATENT_ENTROPY_PLUGIN > #define __latent_entropy __attribute__((latent_entropy)) > #endif > @@ -232,6 +246,9 @@ > #define randomized_struct_fields_end } __randomize_layout; > #endif > > +#endif /* GCC_VERSION >= 40500 */ > + > +#if GCC_VERSION >= 40600 > /* > * When used with Link Time Optimization, gcc can optimize away C functions or > * variables which are referenced only from assembly code. __visible tells the > @@ -240,7 +257,7 @@ > */ > #define __visible __attribute__((externally_visible)) > > -/* gcc version specific checks */ > +#endif /* GCC_VERSION >= 40600 */ > > #if GCC_VERSION >= 40900 && !defined(__CHECKER__) > /* > @@ -274,8 +291,10 @@ > * folding in __builtin_bswap*() (yet), so don't set these for it. > */ > #if defined(CONFIG_ARCH_USE_BUILTIN_BSWAP) && !defined(__CHECKER__) > +#if GCC_VERSION >= 40400 > #define __HAVE_BUILTIN_BSWAP32__ > #define __HAVE_BUILTIN_BSWAP64__ > +#endif > #if GCC_VERSION >= 40800 > #define __HAVE_BUILTIN_BSWAP16__ > #endif > @@ -327,9 +346,12 @@ > #define __diag_GCC_warn warning > #define __diag_GCC_error error > > +/* Compilers before gcc-4.6 do not understand "#pragma GCC diagnostic push" */ > +#if GCC_VERSION >= 40600 > #define __diag_str1(s) #s > #define __diag_str(s) __diag_str1(s) > #define __diag(s) _Pragma(__diag_str(GCC diagnostic s)) > +#endif > > #if GCC_VERSION >= 80000 > #define __diag_GCC_8(s) __diag(s) > -- > 2.7.4 >
I think the current state of always including compiler-gcc.h, then possibly including compiler-clang.h or compiler-intel.h to overwrite some things is kind of a spaghetti mess, because now we wind up with these circular dependencies on GCC_VERSION. And that Clang just happened to declare __GNUC_MAJOR__ and friends in a way that didn't blow up until today was a bit of luck; that was a time bomb waiting to go off. I think it's time to shave this yak. Adding these GCC_VERSION checks back doesn't simplify the state of things.
I think a more proper fix might be something along the lines of (in compiler_types.h):
#include <linux/compiler-generic.h> /* potential new header for common definitions (or just put them above) */ #if /* more proper check for gcc and JUST gcc */ #include <linux/compiler-gcc.h> #elif /* compiler check for icc */ #include <linux/compiler-intel.h> #elif /* compiler check for clang */ #include <linux/compiler-clang.h> #endif
#ifndef /* something from the above that's not mission critical */ #warning "compiler missing foo" #undef foo #endif
#ifndef /* something from above that IS mission critical */ #error "compiler missing bar" #endif
I appreciate the patch, but I think there's another way we can clean this up. -- Thanks, ~Nick Desaulniers
|  |