LinuxLists.cc - [PATCH] compiler-gcc: get back Clang build

[permalink] [raw]

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
<[email protected]> 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 <[email protected]>
> ---
>
> 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

2018-08-21 10:47:19

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, 2018-08-21 at 15:48 +0900, Masahiro Yamada 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

Perhaps this would work, but I can't test it as
my clang version doesn't otherwise build a defconfig
and errors out with

$ make CC=clang
arch/x86/Makefile:179: *** Compiler lacks asm-goto support.. Stop.

---
include/linux/compiler-gcc.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 3e70b7d4e9ed..3a06ad823fa4 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -3,6 +3,23 @@
#error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
#endif

+/*
+ * Override clang compiler version #defines
+ *
+ * compiler_types.h always #includes compiler-gcc.h before compiler-clang,h
+ * but clang sets these __GNUC version #defines to 4.2.1.
+ * This breaks the gcc minimum version of 4.6.0, so override the clang
+ * definitions to 4.6.0
+ */
+#ifdef __clang__
+ #undef __GNUC__
+ #undef __GNUC_MINOR__
+ #undef __GNUC_PATCHLEVEL__
+ #define __GNUC__ 4
+ #define __GNUC_MINOR__ 6
+ #define __GNUC_PATCHLEVEL__ 0
+#endif
+

2018-08-21 12:41:12

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

Nick Desaulniers Aug. 21, 2018, 8:09 a.m. UTC:
> 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.

FWIW libbcc so many BPF users also use clang, so this has more impact
than just testing to build linux with clang (not that this would be any
reason to delay fixing either way)

I would tend to agree havin a compiler-common + make clang/intel not
include compiler-gcc would probably be best in the long run but we might
want a quick fix for 4.19 meanwhile..

--
Dominique Martinet

2018-08-21 16:46:05

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, Aug 21, 2018 at 5:38 AM Dominique Martinet
<[email protected]> wrote:
>
> Nick Desaulniers Aug. 21, 2018, 8:09 a.m. UTC:
> > 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.
>
> FWIW libbcc so many BPF users also use clang, so this has more impact
> than just testing to build linux with clang (not that this would be any
> reason to delay fixing either way)
>
> I would tend to agree havin a compiler-common + make clang/intel not
> include compiler-gcc would probably be best in the long run but we might
> want a quick fix for 4.19 meanwhile..

That's fair. SOP here is quick (full) revert, then come up with a
better fix. And I do prefer Masahiro's partial revert to a full
revert of Joe's patch. That will give us more time to develop the
proper fix rather than rush. I'll try to see how we can more properly
split the compiler specific headers.

Tested with gcc-7 and clang-8.

Tested-by: Nick Desaulniers <[email protected]>
--
Thanks,
~Nick Desaulniers

2018-08-21 16:46:08

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, 2018-08-21 at 15:48 +0900, Masahiro Yamada 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 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.
>
> Also, revive the version test blocks for versions >= 4.2.1
> in order to disable features not supported by Clang.

What is the minimum clang version required to compile the kernel?
What features are not supported by the minimum clang version?

On my system, using clang

$ clang -v
clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final)

and

$ git checkout v4.16 ; make clean ; make CC=clang defconfig ; make CC=clang
HEAD is now at 0adb32858b0b... Linux 4.16

is successful

but

$ git checkout v4.17 ; make clean ; make CC=clang defconfig ; make CC=clang
HEAD is now at 29dcea88779c... Linux 4.17
arch/x86/Makefile:184: *** Compiler lacks asm-goto support.. Stop.
arch/x86/Makefile:184: *** Compiler lacks asm-goto support.. Stop.

2018-08-21 16:46:12

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, Aug 21, 2018 at 3:39 AM Joe Perches <[email protected]> wrote:
>
> On Tue, 2018-08-21 at 15:48 +0900, Masahiro Yamada 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
>
> Perhaps this would work, but I can't test it as
> my clang version doesn't otherwise build a defconfig
> and errors out with
>
> $ make CC=clang
> arch/x86/Makefile:179: *** Compiler lacks asm-goto support.. Stop.

Sorry, we're working on implementing this in clang and llvm for x86.
I recently reviewed the design doc, and am trying to see how I can
actively help push this along. It's not a small/quick change to llvm,
but we're working on it.

--
Thanks,
~Nick Desaulniers

2018-08-21 16:48:20

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, 2018-08-21 at 09:32 -0700, Nick Desaulniers wrote:
> On Tue, Aug 21, 2018 at 5:38 AM Dominique Martinet
> <[email protected]> wrote:
> >
> > Nick Desaulniers Aug. 21, 2018, 8:09 a.m. UTC:
> > > 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.
> >
> > FWIW libbcc so many BPF users also use clang, so this has more impact
> > than just testing to build linux with clang (not that this would be any
> > reason to delay fixing either way)
> >
> > I would tend to agree havin a compiler-common + make clang/intel not
> > include compiler-gcc would probably be best in the long run but we might
> > want a quick fix for 4.19 meanwhile..
>
> That's fair. SOP here is quick (full) revert, then come up with a
> better fix. And I do prefer Masahiro's partial revert to a full
> revert of Joe's patch. That will give us more time to develop the
> proper fix rather than rush. I'll try to see how we can more properly
> split the compiler specific headers.
>
> Tested with gcc-7 and clang-8.

clang-8? Isn't the latest officlal clang 6.0.1 ?

http://releases.llvm.org/

vs

https://clang.llvm.org/docs/ReleaseNotes.html

So if something other than 6.0.x is required,
then some additional check should probably be
added to compiler-clang.h as well.

2018-08-21 17:08:35

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, Aug 21, 2018 at 9:45 AM Joe Perches <[email protected]> wrote:
>
> On Tue, 2018-08-21 at 09:32 -0700, Nick Desaulniers wrote:
> > On Tue, Aug 21, 2018 at 5:38 AM Dominique Martinet
> > <[email protected]> wrote:
> > >
> > > Nick Desaulniers Aug. 21, 2018, 8:09 a.m. UTC:
> > > > 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.
> > >
> > > FWIW libbcc so many BPF users also use clang, so this has more impact
> > > than just testing to build linux with clang (not that this would be any
> > > reason to delay fixing either way)
> > >
> > > I would tend to agree havin a compiler-common + make clang/intel not
> > > include compiler-gcc would probably be best in the long run but we might
> > > want a quick fix for 4.19 meanwhile..
> >
> > That's fair. SOP here is quick (full) revert, then come up with a
> > better fix. And I do prefer Masahiro's partial revert to a full
> > revert of Joe's patch. That will give us more time to develop the
> > proper fix rather than rush. I'll try to see how we can more properly
> > split the compiler specific headers.
> >
> > Tested with gcc-7 and clang-8.
>
> clang-8? Isn't the latest officlal clang 6.0.1 ?

Yes, but I have a local llvm tree that I work out of, that's in my
$PATH, so my version of clang is never too far behind Top of Tree.
For android, we're using clang-5, but currently staging an upgrade to
clang 6.0.1.

> So if something other than 6.0.x is required,
> then some additional check should probably be
> added to compiler-clang.h as well.
>

Sure, but that doesn't need to go in Mashiro's patch today. That can
wait for a proper separation between compiler headers where we can
then implement improved version checks.

--
Thanks,
~Nick Desaulniers

2018-08-21 17:11:00

by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

2018-08-22 1:33 GMT+09:00 Joe Perches <[email protected]>:
> On Tue, 2018-08-21 at 15:48 +0900, Masahiro Yamada 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 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.
>>
>> Also, revive the version test blocks for versions >= 4.2.1
>> in order to disable features not supported by Clang.
>
> What is the minimum clang version required to compile the kernel?
> What features are not supported by the minimum clang version?
>
> On my system, using clang
>
> $ clang -v
> clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final)
>
> and
>
> $ git checkout v4.16 ; make clean ; make CC=clang defconfig ; make CC=clang
> HEAD is now at 0adb32858b0b... Linux 4.16
>
> is successful
>
> but
>
> $ git checkout v4.17 ; make clean ; make CC=clang defconfig ; make CC=clang
> HEAD is now at 29dcea88779c... Linux 4.17
> arch/x86/Makefile:184: *** Compiler lacks asm-goto support.. Stop.
> arch/x86/Makefile:184: *** Compiler lacks asm-goto support.. Stop.
>

You cannot build x86 because asm-goto support is missing in clang.

How about building arm or arm64?

$ make ARCH=arm CC=clang CROSS_COMPILE=arm-linux-gnueabihf- defconfig all

--
Best Regards
Masahiro Yamada

2018-08-21 17:17:21

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, Aug 21, 2018 at 9:33 AM Joe Perches <[email protected]> wrote:
>
> On Tue, 2018-08-21 at 15:48 +0900, Masahiro Yamada 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 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.
> >
> > Also, revive the version test blocks for versions >= 4.2.1
> > in order to disable features not supported by Clang.
>
> What is the minimum clang version required to compile the kernel?

Depends on the architecture and which kernel version/LTS branch you're
using. I'm trying to backport fixes to LTS branches, but sometimes a
compiler upgrade is required. I know that's not great, but I'm
actively trying to fix it.

> What features are not supported by the minimum clang version?
>
> On my system, using clang
>
> $ clang -v
> clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final)
>
> and
>
> $ git checkout v4.16 ; make clean ; make CC=clang defconfig ; make CC=clang
> HEAD is now at 0adb32858b0b... Linux 4.16
>
> is successful
>
> but
>
> $ git checkout v4.17 ; make clean ; make CC=clang defconfig ; make CC=clang
> HEAD is now at 29dcea88779c... Linux 4.17
> arch/x86/Makefile:184: *** Compiler lacks asm-goto support.. Stop.
> arch/x86/Makefile:184: *** Compiler lacks asm-goto support.. Stop.
>

See commit e501ce9 ("x86: Force asm-goto").

$ git describe --contains e501ce9 | sed 's/~.*//'
v4.17-rc1

--
Thanks,
~Nick Desaulniers

2018-08-21 17:17:54

by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

Hi Joe,

2018-08-21 19:39 GMT+09:00 Joe Perches <[email protected]>:
> On Tue, 2018-08-21 at 15:48 +0900, Masahiro Yamada 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
>
> Perhaps this would work, but I can't test it as
> my clang version doesn't otherwise build a defconfig
> and errors out with
>
> $ make CC=clang
> arch/x86/Makefile:179: *** Compiler lacks asm-goto support.. Stop.
>
> ---
> include/linux/compiler-gcc.h | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 3e70b7d4e9ed..3a06ad823fa4 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -3,6 +3,23 @@
> #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
> #endif
>
> +/*
> + * Override clang compiler version #defines
> + *
> + * compiler_types.h always #includes compiler-gcc.h before compiler-clang,h
> + * but clang sets these __GNUC version #defines to 4.2.1.
> + * This breaks the gcc minimum version of 4.6.0, so override the clang
> + * definitions to 4.6.0
> + */
> +#ifdef __clang__
> + #undef __GNUC__
> + #undef __GNUC_MINOR__
> + #undef __GNUC_PATCHLEVEL__
> + #define __GNUC__ 4
> + #define __GNUC_MINOR__ 6
> + #define __GNUC_PATCHLEVEL__ 0
> +#endif
> +
>

This patch does not work.

It only addresses the
"# error Sorry, your compiler is too old - please upgrade it." part.

Attributes unsupported by Clang
must be ifdef'ed out.

Otherwise, you will see lots of unknown attribute warnings.

--
Best Regards
Masahiro Yamada

2018-08-21 17:23:33

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Wed, 2018-08-22 at 02:13 +0900, Masahiro Yamada wrote:
> Hi Joe,
>
>
> 2018-08-21 19:39 GMT+09:00 Joe Perches <[email protected]>:
> > On Tue, 2018-08-21 at 15:48 +0900, Masahiro Yamada 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
> >
> > Perhaps this would work, but I can't test it as
> > my clang version doesn't otherwise build a defconfig
> > and errors out with
> >
> > $ make CC=clang
> > arch/x86/Makefile:179: *** Compiler lacks asm-goto support.. Stop.
> >
> > ---
> > include/linux/compiler-gcc.h | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> > index 3e70b7d4e9ed..3a06ad823fa4 100644
> > --- a/include/linux/compiler-gcc.h
> > +++ b/include/linux/compiler-gcc.h
> > @@ -3,6 +3,23 @@
> > #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
> > #endif
> >
> > +/*
> > + * Override clang compiler version #defines
> > + *
> > + * compiler_types.h always #includes compiler-gcc.h before compiler-clang,h
> > + * but clang sets these __GNUC version #defines to 4.2.1.
> > + * This breaks the gcc minimum version of 4.6.0, so override the clang
> > + * definitions to 4.6.0
> > + */
> > +#ifdef __clang__
> > + #undef __GNUC__
> > + #undef __GNUC_MINOR__
> > + #undef __GNUC_PATCHLEVEL__
> > + #define __GNUC__ 4
> > + #define __GNUC_MINOR__ 6
> > + #define __GNUC_PATCHLEVEL__ 0
> > +#endif
> > +
> >
>
> This patch does not work.
>
> It only addresses the
> "# error Sorry, your compiler is too old - please upgrade it." part.
>
>
> Attributes unsupported by Clang
> must be ifdef'ed out.
>
> Otherwise, you will see lots of unknown attribute warnings.

Then those attributes should be added or
overridden in compiler-clang.h.

2018-08-21 17:24:08

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, 2018-08-21 at 09:57 -0700, Nick Desaulniers wrote:
> On Tue, Aug 21, 2018 at 9:33 AM Joe Perches <[email protected]> wrote:
> >
> > On Tue, 2018-08-21 at 15:48 +0900, Masahiro Yamada 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 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.
> > >
> > > Also, revive the version test blocks for versions >= 4.2.1
> > > in order to disable features not supported by Clang.
> >
> > What is the minimum clang version required to compile the kernel?
>
> Depends on the architecture and which kernel version/LTS branch you're
> using. I'm trying to backport fixes to LTS branches, but sometimes a
> compiler upgrade is required. I know that's not great, but I'm
> actively trying to fix it.

Can you enumerate these dependencies please.

> > What features are not supported by the minimum clang version?

And enumerate this too please.

Uglifying the compiler-gcc file to support
clang features that can be overridden in
compiler-clang.h instead is not a good way
to fix compilation for clang.

But still, whatever features in the various
clang versions that need accommodation also
need listing out in compiler-clang.h, not in
compiler-gcc.h

> $ git describe --contains e501ce9 | sed 's/~.*//'
> v4.17-rc1

Thanks, but I know this.

The question remains, if clang can't compile
v4.17, why does it immediately matter for v4.19?

Why wouldn't overriding the clang __GNUC_<foo>
#defines in compiler-gcc.h work acceptably with
adding whatever is necessary to compiler-clang.h?

I'll experiment with a compiler-clang.h patch.

2018-08-22 04:33:52

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

Nick Desaulniers wrote on Tue, Aug 21, 2018:
> On Tue, Aug 21, 2018 at 9:45 AM Joe Perches <[email protected]> wrote:
> > > Tested with gcc-7 and clang-8.
> >
> > clang-8? Isn't the latest officlal clang 6.0.1 ?
> [...]
> > So if something other than 6.0.x is required,
> > then some additional check should probably be
> > added to compiler-clang.h as well.
>
> Sure, but that doesn't need to go in Mashiro's patch today. That can
> wait for a proper separation between compiler headers where we can
> then implement improved version checks.

I can confirm that the patch here works for me with clang 6.0.1 as far
as bpf programs are concerned (and gcc 8.1.1 for the kernel build itself
on x86 while I was at it); so at least there is nothing on the
compiler*.h headers that put off clang 6.0.1

(also replying to other subthread)
From Joe Perches @ 2018-08-21 17:22 UTC:
> The question remains, if clang can't compile
> v4.17, why does it immediately matter for v4.19?

I haven't had any problem with clang and 4.17 as far as building bpf
programs is concerned, because the CC_HAVE_ASM_GOTO check is done in
makefiles that aren't used in the bpf case and not in compiler-gcc.h
or another header.

So I guess the "immediately matters for v4.19" depends on how much you
would care about bcc-compiled BPF programs.

> Why wouldn't overriding the clang __GNUC_<foo>
> #defines in compiler-gcc.h work acceptably with
> adding whatever is necessary to compiler-clang.h?

I think that could work, but at the point making a separate
compiler-common.h and not including compiler-gcc.h for clang sounds
better to me... More importantly here, either solution sound complex
enough to require more than a few days and proper testing for all archs
etc when compared to the partial revert we have here.

--
Dominique Martinet

2018-08-22 04:38:20

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

Joe Perches wrote on Tue, Aug 21, 2018:
> On Wed, 2018-08-22 at 06:16 +0200, Dominique Martinet wrote:
> > I think that could work, but at the point making a separate
> > compiler-common.h and not including compiler-gcc.h for clang sounds
> > better to me... More importantly here, either solution sound complex
> > enough to require more than a few days and proper testing for all archs
> > etc when compared to the partial revert we have here.
>
> The immediate need for a partial revert seems unnecessary as
> clang hasn't really worked for a couple releases now.

Sorry for repeating myself, clang is used by bcc for compiling BPF
programs (e.g. bpf_module_create_c_from_string() or any similar function
will use the clang libs to compile the bpf program with linux headers),
and this has always been working because it's not using our makefiles.

This broke today in master and I only joined this thread after looking
at why the build started failing today and noticing this patch, it
definitely hasn't been broken for two releases, or I wouldn't be here
with this timing :)

> The separate compiler file changes are much more sensible,
> even if it takes a few days.

A few days are fine, but I think some form of fix in 4.19-rc1 would be
good.

I'll stop taking your time now, but I think you are/were underestimating
how many people use clang with the linux kernel headers indirectly.
BPF is a well-used tool :)

Thanks,
--
Dominique Martinet

2018-08-22 04:53:37

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Wed, 2018-08-22 at 06:16 +0200, Dominique Martinet wrote:
> I think that could work, but at the point making a separate
> compiler-common.h and not including compiler-gcc.h for clang sounds
> better to me... More importantly here, either solution sound complex
> enough to require more than a few days and proper testing for all archs
> etc when compared to the partial revert we have here.

The immediate need for a partial revert seems unnecessary as
clang hasn't really worked for a couple releases now.

The separate compiler file changes are much more sensible,
even if it takes a few days.

2018-08-22 18:33:35

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Tue, Aug 21, 2018 at 9:32 PM Dominique Martinet
<[email protected]> wrote:
>
> Joe Perches wrote on Tue, Aug 21, 2018:
> > On Wed, 2018-08-22 at 06:16 +0200, Dominique Martinet wrote:
> > > I think that could work, but at the point making a separate
> > > compiler-common.h and not including compiler-gcc.h for clang sounds
> > > better to me... More importantly here, either solution sound complex
> > > enough to require more than a few days and proper testing for all archs
> > > etc when compared to the partial revert we have here.
> >
> > The immediate need for a partial revert seems unnecessary as
> > clang hasn't really worked for a couple releases now.
>
> Sorry for repeating myself, clang is used by bcc for compiling BPF
> programs (e.g. bpf_module_create_c_from_string() or any similar function
> will use the clang libs to compile the bpf program with linux headers),
> and this has always been working because it's not using our makefiles.
>
> This broke today in master and I only joined this thread after looking
> at why the build started failing today and noticing this patch, it
> definitely hasn't been broken for two releases, or I wouldn't be here
> with this timing :)
>
>
> > The separate compiler file changes are much more sensible,
> > even if it takes a few days.
>
> A few days are fine, but I think some form of fix in 4.19-rc1 would be
> good.
>
> I'll stop taking your time now, but I think you are/were underestimating
> how many people use clang with the linux kernel headers indirectly.
> BPF is a well-used tool :)

Hi Dominique,
I'm currently testing a fix in
https://github.com/ClangBuiltLinux/linux/commit/1f89ae7622c26b8131f42f3a362d6ef41b88a595,
can you please share with me your steps to test/verify that the patch
fixes the issue for eBPF? I'll go talk to a co-worker who know more
about eBPF, but I've not yet done anything with it.

Also, does anyone know who I should talk to about ICC testing?
--
Thanks,
~Nick Desaulniers

2018-08-22 19:04:11

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Wed, Aug 22, 2018 at 11:31 AM Nick Desaulniers
<[email protected]> wrote:
> Hi Dominique,
> I'm currently testing a fix in
> https://github.com/ClangBuiltLinux/linux/commit/1f89ae7622c26b8131f42f3a362d6ef41b88a595,

Sorry, maybe https://github.com/ClangBuiltLinux/linux/commits/compiler_detection
is a better link, as the sha changes whenever I modify the patch and
force push.
--
Thanks,
~Nick Desaulniers

2018-08-22 20:51:59

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Wed, 2018-08-22 at 11:31 -0700, Nick Desaulniers wrote:
> On Tue, Aug 21, 2018 at 9:32 PM Dominique Martinet
> <[email protected]> wrote:
> >
> > Joe Perches wrote on Tue, Aug 21, 2018:
> > > On Wed, 2018-08-22 at 06:16 +0200, Dominique Martinet wrote:
> > > > I think that could work, but at the point making a separate
> > > > compiler-common.h and not including compiler-gcc.h for clang sounds
> > > > better to me... More importantly here, either solution sound complex
> > > > enough to require more than a few days and proper testing for all archs
> > > > etc when compared to the partial revert we have here.
> > >
> > > The immediate need for a partial revert seems unnecessary as
> > > clang hasn't really worked for a couple releases now.
> >
> > Sorry for repeating myself, clang is used by bcc for compiling BPF
> > programs (e.g. bpf_module_create_c_from_string() or any similar function
> > will use the clang libs to compile the bpf program with linux headers),
> > and this has always been working because it's not using our makefiles.
> >
> > This broke today in master and I only joined this thread after looking
> > at why the build started failing today and noticing this patch, it
> > definitely hasn't been broken for two releases, or I wouldn't be here
> > with this timing :)
> >
> >
> > > The separate compiler file changes are much more sensible,
> > > even if it takes a few days.
> >
> > A few days are fine, but I think some form of fix in 4.19-rc1 would be
> > good.
> >
> > I'll stop taking your time now, but I think you are/were underestimating
> > how many people use clang with the linux kernel headers indirectly.
> > BPF is a well-used tool :)
>
> Hi Dominique,
> I'm currently testing a fix in
> https://github.com/ClangBuiltLinux/linux/commit/1f89ae7622c26b8131f42f3a362d6ef41b88a595,
> can you please share with me your steps to test/verify that the patch
> fixes the issue for eBPF? I'll go talk to a co-worker who know more
> about eBPF, but I've not yet done anything with it.

A mild suggestion about the patch would be to break it up into
2 patches to improve how people read and review them.

1 include/linux/compiler-*
2 everything else

Yes, some kernel configs might not build properly between 1 and 2
but that likely doesn't matter as those configs probably don't
build before 1 either.

Perhaps the test in arch/arm/kernel/asm-offsets.c for incompatible
gcc compiler versions from 4.8.0 to 4.8.2 should be moved to
compiler-gcc.h.

> Also, does anyone know who I should talk to about ICC testing?

No clue

2018-08-22 23:08:06

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Wed, Aug 22, 2018 at 1:50 PM Joe Perches <[email protected]> wrote:
>
> On Wed, 2018-08-22 at 11:31 -0700, Nick Desaulniers wrote:
> > On Tue, Aug 21, 2018 at 9:32 PM Dominique Martinet
> > <[email protected]> wrote:
> > >
> > > Joe Perches wrote on Tue, Aug 21, 2018:
> > > > On Wed, 2018-08-22 at 06:16 +0200, Dominique Martinet wrote:
> > > > > I think that could work, but at the point making a separate
> > > > > compiler-common.h and not including compiler-gcc.h for clang sounds
> > > > > better to me... More importantly here, either solution sound complex
> > > > > enough to require more than a few days and proper testing for all archs
> > > > > etc when compared to the partial revert we have here.
> > > >
> > > > The immediate need for a partial revert seems unnecessary as
> > > > clang hasn't really worked for a couple releases now.
> > >
> > > Sorry for repeating myself, clang is used by bcc for compiling BPF
> > > programs (e.g. bpf_module_create_c_from_string() or any similar function
> > > will use the clang libs to compile the bpf program with linux headers),
> > > and this has always been working because it's not using our makefiles.
> > >
> > > This broke today in master and I only joined this thread after looking
> > > at why the build started failing today and noticing this patch, it
> > > definitely hasn't been broken for two releases, or I wouldn't be here
> > > with this timing :)
> > >
> > >
> > > > The separate compiler file changes are much more sensible,
> > > > even if it takes a few days.
> > >
> > > A few days are fine, but I think some form of fix in 4.19-rc1 would be
> > > good.
> > >
> > > I'll stop taking your time now, but I think you are/were underestimating
> > > how many people use clang with the linux kernel headers indirectly.
> > > BPF is a well-used tool :)
> >
> > Hi Dominique,
> > I'm currently testing a fix in
> > https://github.com/ClangBuiltLinux/linux/commit/1f89ae7622c26b8131f42f3a362d6ef41b88a595,
> > can you please share with me your steps to test/verify that the patch
> > fixes the issue for eBPF? I'll go talk to a co-worker who know more
> > about eBPF, but I've not yet done anything with it.
>
> A mild suggestion about the patch would be to break it up into
> 2 patches to improve how people read and review them.
>
> 1 include/linux/compiler-*
> 2 everything else
>
> Yes, some kernel configs might not build properly between 1 and 2
> but that likely doesn't matter as those configs probably don't
> build before 1 either.

If we ordered the patches so that the "everything else" went in first,
it would not be a problem. The first patch would just be the checks
that GCC_VERSION is defined.

In general, I'm happy to split patches, but in this suggested case, it
only shaves off 26 lines from the main body of work. I don't think 26
lines is enough to justify splitting for readability. But let me know
if you feel strongly about that point and I'll be happy to split. (or
possibly by "everything else" you meant something else?)

> Perhaps the test in arch/arm/kernel/asm-offsets.c for incompatible
> gcc compiler versions from 4.8.0 to 4.8.2 should be moved to
> compiler-gcc.h.

This a good suggestion, and one I've thought about, although in the
context of builtins. *Detour ahead*. Builtins are not portable by
default, and their use really should be feature detected (impossible
currently on gcc due to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970) or version checks
and protected by macros like the symbols in my current patch. I think
I might soon hoist them to a shared header that safely detects their
support or provides a fallback when possible. One issue is that some
builtins are arch specific, so do you want to feature detect arch
specific ones for builds of a different arch? And that's the problem
I have with hoisting arch specific compiler checks into a shared
header; builds of other archs should pay no build penalty for
unrelated archs. All that to say that I think it's best to keep arch
specific checks in arch/ specific subdirs, but I welcome more thoughts
on this.

Ok, I've boot tested this patch for x86_64 and arm64 in qemu, with
CC=gcc-7.2, CC=clang-8, and CC=clang-8 HOSTCC=clang-8 and am feeling
quite confident to send it for review.

>> Also, does anyone know who I should talk to about ICC testing?
> No clue

+ Daniel and HPA (the last commits to include/linux/compiler*
mentioning `icc` in 2015 and 2013 respectively)
--
Thanks,
~Nick Desaulniers

2018-08-22 23:34:15

[permalink] [raw]

Subject: Re: [PATCH] compiler-gcc: get back Clang build

On Wed, 2018-08-22 at 16:05 -0700, Nick Desaulniers wrote:

Hey Nick.

> On Wed, Aug 22, 2018 at 1:50 PM Joe Perches <[email protected]> wrote:
> > A mild suggestion about the patch would be to break it up into
> > 2 patches to improve how people read and review them.
> >
> > 1 include/linux/compiler-*
> > 2 everything else
> >
> > Yes, some kernel configs might not build properly between 1 and 2
> > but that likely doesn't matter as those configs probably don't
> > build before 1 either.
>
> If we ordered the patches so that the "everything else" went in first,
> it would not be a problem. The first patch would just be the checks
> that GCC_VERSION is defined.
>
> In general, I'm happy to split patches, but in this suggested case, it
> only shaves off 26 lines from the main body of work.

No worries, I rarely care _that_ much about code, but
seeing the subject with compiler-gcc and the first
part of the patch about arch/ was a bit off-putting.

You're the one doing the work here.
Do what you think best.

2018-08-22 23:58:57