Re: [PATCH] compiler-gcc: get back Clang build
From: Nick Desaulniers
Date: Tue Aug 21 2018 - 04:09:33 EST
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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> ---
>
> 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