2018-10-23 00:48:11

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in
inline assembly code to work around asm() related GCC inlining bugs")
added this flag to KBUILD_CFLAGS, where it works perfectly fine with
GCC. However, when building with Clang, all of the object files compile
fine but the build hangs indefinitely at init/main.o, right before the
linking stage. Don't include this flag when building with Clang.

The kernel builds and boots to a shell in QEMU with both GCC and Clang
with this patch applied.

Link: https://github.com/ClangBuiltLinux/linux/issues/213
Signed-off-by: Nathan Chancellor <[email protected]>
---

The reason this patch is labeled RFC is while I can verify that this
fixes the issue, I'm not entirely sure why the '-Wa,-' works for GCC
and not Clang. I looked into what the flag means and I couldn't really
find anything so I just assume it's taking input from stdin? The issue
could stem from how GCC forks gas versus how Clang does it. If this
isn't of concern and the maintainers are happy with this patch as is,
feel free to take it.

arch/x86/Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 5b562e464009..4736dcc1caec 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -239,7 +239,10 @@ archheaders:
archmacros:
$(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s

-ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,-
+ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
+ifneq ($(cc-name),clang)
+ASM_MACRO_FLAGS += -Wa,-
+endif
export ASM_MACRO_FLAGS
KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)

--
2.19.1



2018-10-23 18:41:56

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <[email protected]> wrote:
>
> at 5:37 PM, Nathan Chancellor <[email protected]> wrote:
>
> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in
> inline assembly code to work around asm() related GCC inlining bugs")
> added this flag to KBUILD_CFLAGS, where it works perfectly fine with
> GCC. However, when building with Clang, all of the object files compile
> fine but the build hangs indefinitely at init/main.o, right before the
> linking stage. Don't include this flag when building with Clang.
>
> The kernel builds and boots to a shell in QEMU with both GCC and Clang
> with this patch applied.
>
> Link: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&amp;data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&amp;sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&amp;reserved=0
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
>
> The reason this patch is labeled RFC is while I can verify that this
> fixes the issue, I'm not entirely sure why the '-Wa,-' works for GCC
> and not Clang. I looked into what the flag means and I couldn't really
> find anything so I just assume it's taking input from stdin? The issue
> could stem from how GCC forks gas versus how Clang does it. If this
> isn't of concern and the maintainers are happy with this patch as is,
> feel free to take it.
>
> arch/x86/Makefile | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 5b562e464009..4736dcc1caec 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -239,7 +239,10 @@ archheaders:
> archmacros:
> $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
>
> -ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,-
> +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
> +ifneq ($(cc-name),clang)
> +ASM_MACRO_FLAGS += -Wa,-
> +endif
> export ASM_MACRO_FLAGS
> KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
>
>
> The '-Wa,-‘ was indeed used to take the input from stdin when the ‘-pipe’
> option is used in GCC. Perhaps clang ignores the ‘-pipe’ flag.
>

Clang documents support for `-pipe`.
https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-pipe

$ clang hello_world.c -no-integrated-as -Wa,-

is simple enough to reproduce the hang (notice no `-pipe`).
`strace`'ing the above with and without `-Wa,-` shows clang hung in a
`wait4` call after spawning as, while without `-Wa,-` shows it
receiving a SIGCHLD.

I've filed a bug against Clang to investigate:
https://bugs.llvm.org/show_bug.cgi?id=39410

In the meantime, I think Nathan's patch is the correct way to work
around this, if it is indeed a bug in Clang.

Reviewed-and-Tested-by: Nick Desaulniers <[email protected]>

https://stackoverflow.com/questions/1512933/when-should-i-use-gccs-pipe-option
has some interesting perspectives on the use of `-pipe`.
Thanks for the patch Nathan, and more info Nadav.
--
Thanks,
~Nick Desaulniers

2018-10-23 20:03:01

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

On 10/23/18 11:40, Nick Desaulniers wrote:
> On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <[email protected]> wrote:
>>
>> at 5:37 PM, Nathan Chancellor <[email protected]> wrote:
>>
>> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in
>> inline assembly code to work around asm() related GCC inlining bugs")
>> added this flag to KBUILD_CFLAGS, where it works perfectly fine with
>> GCC. However, when building with Clang, all of the object files compile
>> fine but the build hangs indefinitely at init/main.o, right before the
>> linking stage. Don't include this flag when building with Clang.
>>
>> The kernel builds and boots to a shell in QEMU with both GCC and Clang
>> with this patch applied.
>>
>> Link: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&amp;data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&amp;sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&amp;reserved=0
>> Signed-off-by: Nathan Chancellor <[email protected]>
>> ---
>>
>> The reason this patch is labeled RFC is while I can verify that this
>> fixes the issue, I'm not entirely sure why the '-Wa,-' works for GCC
>> and not Clang. I looked into what the flag means and I couldn't really
>> find anything so I just assume it's taking input from stdin? The issue
>> could stem from how GCC forks gas versus how Clang does it. If this
>> isn't of concern and the maintainers are happy with this patch as is,
>> feel free to take it.
>>

Perhaps someone could actually, you know, time the build and see how
much -pipe actually matters, if at all?

-hpa


2018-10-23 21:59:41

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

On Tue, Oct 23, 2018 at 01:01:22PM -0700, H. Peter Anvin wrote:
> On 10/23/18 11:40, Nick Desaulniers wrote:
> > On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <[email protected]> wrote:
> >>
> >> at 5:37 PM, Nathan Chancellor <[email protected]> wrote:
> >>
> >> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in
> >> inline assembly code to work around asm() related GCC inlining bugs")
> >> added this flag to KBUILD_CFLAGS, where it works perfectly fine with
> >> GCC. However, when building with Clang, all of the object files compile
> >> fine but the build hangs indefinitely at init/main.o, right before the
> >> linking stage. Don't include this flag when building with Clang.
> >>
> >> The kernel builds and boots to a shell in QEMU with both GCC and Clang
> >> with this patch applied.
> >>
> >> Link: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&amp;data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&amp;sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&amp;reserved=0
> >> Signed-off-by: Nathan Chancellor <[email protected]>
> >> ---
> >>
> >> The reason this patch is labeled RFC is while I can verify that this
> >> fixes the issue, I'm not entirely sure why the '-Wa,-' works for GCC
> >> and not Clang. I looked into what the flag means and I couldn't really
> >> find anything so I just assume it's taking input from stdin? The issue
> >> could stem from how GCC forks gas versus how Clang does it. If this
> >> isn't of concern and the maintainers are happy with this patch as is,
> >> feel free to take it.
> >>
>
> Perhaps someone could actually, you know, time the build and see how
> much -pipe actually matters, if at all?
>
> -hpa
>

Thank you for the suggestion! With the attached diff for removing
'-pipe' and 'make -j1' with defconfig (just to make sure any variance
would stand out), here are my results:

-pipe (GCC):

real 15m55.202s
user 14m17.748s
sys 1m47.496s

No -pipe (GCC):

real 16m4.430s
user 14m16.277s
sys 1m46.604s

-pipe (Clang):

real 21m26.016s
user 19m21.722s
sys 2m2.606s

No -pipe (Clang):

real 21m27.822s
user 19m22.092s
sys 2m4.151s

Certainly seems like -pipe doesn't make a ton of difference. If this is
a better fix, I am happy to draft up a proper commit message and send
it out for review.

==================================================

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 73f4831283ac..672c689c1faa 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -213,8 +213,6 @@ ifdef CONFIG_X86_64
KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
endif

-# Speed up the build
-KBUILD_CFLAGS += -pipe
# Workaround for a gcc prelease that unfortunately was shipped in a suse release
KBUILD_CFLAGS += -Wno-sign-compare
#
@@ -239,7 +237,7 @@ archheaders:
archmacros:
$(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s

-ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,-
+ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
export ASM_MACRO_FLAGS
KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)


2018-10-23 22:05:38

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

On October 23, 2018 2:58:11 PM PDT, Nathan Chancellor <[email protected]> wrote:
>On Tue, Oct 23, 2018 at 01:01:22PM -0700, H. Peter Anvin wrote:
>> On 10/23/18 11:40, Nick Desaulniers wrote:
>> > On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <[email protected]>
>wrote:
>> >>
>> >> at 5:37 PM, Nathan Chancellor <[email protected]> wrote:
>> >>
>> >> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in
>> >> inline assembly code to work around asm() related GCC inlining
>bugs")
>> >> added this flag to KBUILD_CFLAGS, where it works perfectly fine
>with
>> >> GCC. However, when building with Clang, all of the object files
>compile
>> >> fine but the build hangs indefinitely at init/main.o, right before
>the
>> >> linking stage. Don't include this flag when building with Clang.
>> >>
>> >> The kernel builds and boots to a shell in QEMU with both GCC and
>Clang
>> >> with this patch applied.
>> >>
>> >> Link:
>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&amp;data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&amp;sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&amp;reserved=0
>> >> Signed-off-by: Nathan Chancellor <[email protected]>
>> >> ---
>> >>
>> >> The reason this patch is labeled RFC is while I can verify that
>this
>> >> fixes the issue, I'm not entirely sure why the '-Wa,-' works for
>GCC
>> >> and not Clang. I looked into what the flag means and I couldn't
>really
>> >> find anything so I just assume it's taking input from stdin? The
>issue
>> >> could stem from how GCC forks gas versus how Clang does it. If
>this
>> >> isn't of concern and the maintainers are happy with this patch as
>is,
>> >> feel free to take it.
>> >>
>>
>> Perhaps someone could actually, you know, time the build and see how
>> much -pipe actually matters, if at all?
>>
>> -hpa
>>
>
>Thank you for the suggestion! With the attached diff for removing
>'-pipe' and 'make -j1' with defconfig (just to make sure any variance
>would stand out), here are my results:
>
>-pipe (GCC):
>
>real 15m55.202s
>user 14m17.748s
>sys 1m47.496s
>
>No -pipe (GCC):
>
>real 16m4.430s
>user 14m16.277s
>sys 1m46.604s
>
>-pipe (Clang):
>
>real 21m26.016s
>user 19m21.722s
>sys 2m2.606s
>
>No -pipe (Clang):
>
>real 21m27.822s
>user 19m22.092s
>sys 2m4.151s
>
>Certainly seems like -pipe doesn't make a ton of difference. If this is
>a better fix, I am happy to draft up a proper commit message and send
>it out for review.
>
>==================================================
>
>diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>index 73f4831283ac..672c689c1faa 100644
>--- a/arch/x86/Makefile
>+++ b/arch/x86/Makefile
>@@ -213,8 +213,6 @@ ifdef CONFIG_X86_64
> KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
> endif
>
>-# Speed up the build
>-KBUILD_CFLAGS += -pipe
># Workaround for a gcc prelease that unfortunately was shipped in a
>suse release
> KBUILD_CFLAGS += -Wno-sign-compare
> #
>@@ -239,7 +237,7 @@ archheaders:
> archmacros:
> $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
>
>-ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,-
>+ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
> export ASM_MACRO_FLAGS
> KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)

make -j1 makes the test useless. You should aim for optimal performance for your machine.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2018-10-23 22:09:52

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

On Tue, Oct 23, 2018 at 2:58 PM Nathan Chancellor
<[email protected]> wrote:
>
> On Tue, Oct 23, 2018 at 01:01:22PM -0700, H. Peter Anvin wrote:
> > On 10/23/18 11:40, Nick Desaulniers wrote:
> > > On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <[email protected]> wrote:
> > >>
> > >> at 5:37 PM, Nathan Chancellor <[email protected]> wrote:
> > >>
> > >> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in
> > >> inline assembly code to work around asm() related GCC inlining bugs")
> > >> added this flag to KBUILD_CFLAGS, where it works perfectly fine with
> > >> GCC. However, when building with Clang, all of the object files compile
> > >> fine but the build hangs indefinitely at init/main.o, right before the
> > >> linking stage. Don't include this flag when building with Clang.
> > >>
> > >> The kernel builds and boots to a shell in QEMU with both GCC and Clang
> > >> with this patch applied.
> > >>
> > >> Link: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&amp;data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&amp;sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&amp;reserved=0
> > >> Signed-off-by: Nathan Chancellor <[email protected]>
> > >> ---
> > >>
> > >> The reason this patch is labeled RFC is while I can verify that this
> > >> fixes the issue, I'm not entirely sure why the '-Wa,-' works for GCC
> > >> and not Clang. I looked into what the flag means and I couldn't really
> > >> find anything so I just assume it's taking input from stdin? The issue
> > >> could stem from how GCC forks gas versus how Clang does it. If this
> > >> isn't of concern and the maintainers are happy with this patch as is,
> > >> feel free to take it.
> > >>
> >
> > Perhaps someone could actually, you know, time the build and see how
> > much -pipe actually matters, if at all?
> >
> > -hpa
> >
>
> Thank you for the suggestion! With the attached diff for removing
> '-pipe' and 'make -j1' with defconfig (just to make sure any variance
> would stand out), here are my results:
>
> -pipe (GCC):
>
> real 15m55.202s
> user 14m17.748s
> sys 1m47.496s
>
> No -pipe (GCC):
>
> real 16m4.430s
> user 14m16.277s
> sys 1m46.604s
>
> -pipe (Clang):
>
> real 21m26.016s
> user 19m21.722s
> sys 2m2.606s
>
> No -pipe (Clang):
>
> real 21m27.822s
> user 19m22.092s
> sys 2m4.151s

Looks like Clang eats `-pipe`:
https://github.com/llvm-mirror/clang/blob/391667a023f79287f9c40868f34f08c161555556/lib/Driver/Driver.cpp#L962
commit r110007 has the log:
Driver: Start ripping out support for -pipe, which is worthless
and complicates
too many other things.

>
> Certainly seems like -pipe doesn't make a ton of difference. If this is
> a better fix, I am happy to draft up a proper commit message and send
> it out for review.
>
> ==================================================
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 73f4831283ac..672c689c1faa 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -213,8 +213,6 @@ ifdef CONFIG_X86_64
> KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
> endif
>
> -# Speed up the build
> -KBUILD_CFLAGS += -pipe
> # Workaround for a gcc prelease that unfortunately was shipped in a suse release
> KBUILD_CFLAGS += -Wno-sign-compare
> #
> @@ -239,7 +237,7 @@ archheaders:
> archmacros:
> $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
>
> -ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,-
> +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
> export ASM_MACRO_FLAGS
> KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
>


--
Thanks,
~Nick Desaulniers

2018-10-23 22:45:25

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

On Tue, Oct 23, 2018 at 03:08:53PM -0700, Nick Desaulniers wrote:
> On Tue, Oct 23, 2018 at 2:58 PM Nathan Chancellor
> <[email protected]> wrote:
> >
> > On Tue, Oct 23, 2018 at 01:01:22PM -0700, H. Peter Anvin wrote:
> > > On 10/23/18 11:40, Nick Desaulniers wrote:
> > > > On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <[email protected]> wrote:
> > > >>
> > > >> at 5:37 PM, Nathan Chancellor <[email protected]> wrote:
> > > >>
> > > >> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in
> > > >> inline assembly code to work around asm() related GCC inlining bugs")
> > > >> added this flag to KBUILD_CFLAGS, where it works perfectly fine with
> > > >> GCC. However, when building with Clang, all of the object files compile
> > > >> fine but the build hangs indefinitely at init/main.o, right before the
> > > >> linking stage. Don't include this flag when building with Clang.
> > > >>
> > > >> The kernel builds and boots to a shell in QEMU with both GCC and Clang
> > > >> with this patch applied.
> > > >>
> > > >> Link: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&amp;data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&amp;sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&amp;reserved=0
> > > >> Signed-off-by: Nathan Chancellor <[email protected]>
> > > >> ---
> > > >>
> > > >> The reason this patch is labeled RFC is while I can verify that this
> > > >> fixes the issue, I'm not entirely sure why the '-Wa,-' works for GCC
> > > >> and not Clang. I looked into what the flag means and I couldn't really
> > > >> find anything so I just assume it's taking input from stdin? The issue
> > > >> could stem from how GCC forks gas versus how Clang does it. If this
> > > >> isn't of concern and the maintainers are happy with this patch as is,
> > > >> feel free to take it.
> > > >>
> > >
> > > Perhaps someone could actually, you know, time the build and see how
> > > much -pipe actually matters, if at all?
> > >
> > > -hpa
> > >
> >
> > Thank you for the suggestion! With the attached diff for removing
> > '-pipe' and 'make -j1' with defconfig (just to make sure any variance
> > would stand out), here are my results:
> >
> > -pipe (GCC):
> >
> > real 15m55.202s
> > user 14m17.748s
> > sys 1m47.496s
> >
> > No -pipe (GCC):
> >
> > real 16m4.430s
> > user 14m16.277s
> > sys 1m46.604s
> >
> > -pipe (Clang):
> >
> > real 21m26.016s
> > user 19m21.722s
> > sys 2m2.606s
> >
> > No -pipe (Clang):
> >
> > real 21m27.822s
> > user 19m22.092s
> > sys 2m4.151s
>
> Looks like Clang eats `-pipe`:
> https://github.com/llvm-mirror/clang/blob/391667a023f79287f9c40868f34f08c161555556/lib/Driver/Driver.cpp#L962
> commit r110007 has the log:
> Driver: Start ripping out support for -pipe, which is worthless
> and complicates
> too many other things.
>

In that case, we can either keep this change (I'll resend with the
explanation that Clang doesn't respect -pipe) or we can just rip out
-pipe for GCC too. Here are three separate results for GCC with my
normal jobs flag:

-pipe (GCC):

real 3m40.813s
real 3m44.449s
real 3m39.648s

No -pipe (GCC):

real 3m38.492s
real 3m38.335s
real 3m38.975s

Practically no variance.

Thanks,
Nathan

> >
> > Certainly seems like -pipe doesn't make a ton of difference. If this is
> > a better fix, I am happy to draft up a proper commit message and send
> > it out for review.
> >
> > ==================================================
> >
> > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > index 73f4831283ac..672c689c1faa 100644
> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -213,8 +213,6 @@ ifdef CONFIG_X86_64
> > KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
> > endif
> >
> > -# Speed up the build
> > -KBUILD_CFLAGS += -pipe
> > # Workaround for a gcc prelease that unfortunately was shipped in a suse release
> > KBUILD_CFLAGS += -Wno-sign-compare
> > #
> > @@ -239,7 +237,7 @@ archheaders:
> > archmacros:
> > $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
> >
> > -ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,-
> > +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
> > export ASM_MACRO_FLAGS
> > KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers

2018-10-23 22:55:33

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

On Tue, Oct 23, 2018 at 3:44 PM Nathan Chancellor
<[email protected]> wrote:
>
> On Tue, Oct 23, 2018 at 03:08:53PM -0700, Nick Desaulniers wrote:
> > On Tue, Oct 23, 2018 at 2:58 PM Nathan Chancellor
> > <[email protected]> wrote:
> > >
> > > On Tue, Oct 23, 2018 at 01:01:22PM -0700, H. Peter Anvin wrote:
> > > > On 10/23/18 11:40, Nick Desaulniers wrote:
> > > > > On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <[email protected]> wrote:
> > > > >>
> > > > >> at 5:37 PM, Nathan Chancellor <[email protected]> wrote:
> > > > >>
> > > > >> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in
> > > > >> inline assembly code to work around asm() related GCC inlining bugs")
> > > > >> added this flag to KBUILD_CFLAGS, where it works perfectly fine with
> > > > >> GCC. However, when building with Clang, all of the object files compile
> > > > >> fine but the build hangs indefinitely at init/main.o, right before the
> > > > >> linking stage. Don't include this flag when building with Clang.
> > > > >>
> > > > >> The kernel builds and boots to a shell in QEMU with both GCC and Clang
> > > > >> with this patch applied.
> > > > >>
> > > > >> Link: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&amp;data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&amp;sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&amp;reserved=0
> > > > >> Signed-off-by: Nathan Chancellor <[email protected]>
> > > > >> ---
> > > > >>
> > > > >> The reason this patch is labeled RFC is while I can verify that this
> > > > >> fixes the issue, I'm not entirely sure why the '-Wa,-' works for GCC
> > > > >> and not Clang. I looked into what the flag means and I couldn't really
> > > > >> find anything so I just assume it's taking input from stdin? The issue
> > > > >> could stem from how GCC forks gas versus how Clang does it. If this
> > > > >> isn't of concern and the maintainers are happy with this patch as is,
> > > > >> feel free to take it.
> > > > >>
> > > >
> > > > Perhaps someone could actually, you know, time the build and see how
> > > > much -pipe actually matters, if at all?
> > > >
> > > > -hpa
> > > >
> > >
> > > Thank you for the suggestion! With the attached diff for removing
> > > '-pipe' and 'make -j1' with defconfig (just to make sure any variance
> > > would stand out), here are my results:
> > >
> > > -pipe (GCC):
> > >
> > > real 15m55.202s
> > > user 14m17.748s
> > > sys 1m47.496s
> > >
> > > No -pipe (GCC):
> > >
> > > real 16m4.430s
> > > user 14m16.277s
> > > sys 1m46.604s
> > >
> > > -pipe (Clang):
> > >
> > > real 21m26.016s
> > > user 19m21.722s
> > > sys 2m2.606s
> > >
> > > No -pipe (Clang):
> > >
> > > real 21m27.822s
> > > user 19m22.092s
> > > sys 2m4.151s
> >
> > Looks like Clang eats `-pipe`:
> > https://github.com/llvm-mirror/clang/blob/391667a023f79287f9c40868f34f08c161555556/lib/Driver/Driver.cpp#L962
> > commit r110007 has the log:
> > Driver: Start ripping out support for -pipe, which is worthless
> > and complicates
> > too many other things.
> >
>
> In that case, we can either keep this change (I'll resend with the
> explanation that Clang doesn't respect -pipe) or we can just rip out
> -pipe for GCC too. Here are three separate results for GCC with my
> normal jobs flag:
>
> -pipe (GCC):
>
> real 3m40.813s
> real 3m44.449s
> real 3m39.648s
>
> No -pipe (GCC):
>
> real 3m38.492s
> real 3m38.335s
> real 3m38.975s
>
> Practically no variance.

Thanks for these measurements. With these in mind I agree with HPA
that `-pipe -Wa,-` doesn't buy us anything, and would be simpler to
remove it for compatibility with Clang.

>
> Thanks,
> Nathan
>
> > >
> > > Certainly seems like -pipe doesn't make a ton of difference. If this is
> > > a better fix, I am happy to draft up a proper commit message and send
> > > it out for review.
> > >
> > > ==================================================
> > >
> > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > > index 73f4831283ac..672c689c1faa 100644
> > > --- a/arch/x86/Makefile
> > > +++ b/arch/x86/Makefile
> > > @@ -213,8 +213,6 @@ ifdef CONFIG_X86_64
> > > KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
> > > endif
> > >
> > > -# Speed up the build
> > > -KBUILD_CFLAGS += -pipe
> > > # Workaround for a gcc prelease that unfortunately was shipped in a suse release
> > > KBUILD_CFLAGS += -Wno-sign-compare
> > > #
> > > @@ -239,7 +237,7 @@ archheaders:
> > > archmacros:
> > > $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
> > >
> > > -ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,-
> > > +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
> > > export ASM_MACRO_FLAGS
> > > KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers



--
Thanks,
~Nick Desaulniers

2018-10-23 23:03:06

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang

On October 23, 2018 3:53:10 PM PDT, Nick Desaulniers <[email protected]> wrote:
>On Tue, Oct 23, 2018 at 3:44 PM Nathan Chancellor
><[email protected]> wrote:
>>
>> On Tue, Oct 23, 2018 at 03:08:53PM -0700, Nick Desaulniers wrote:
>> > On Tue, Oct 23, 2018 at 2:58 PM Nathan Chancellor
>> > <[email protected]> wrote:
>> > >
>> > > On Tue, Oct 23, 2018 at 01:01:22PM -0700, H. Peter Anvin wrote:
>> > > > On 10/23/18 11:40, Nick Desaulniers wrote:
>> > > > > On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit
><[email protected]> wrote:
>> > > > >>
>> > > > >> at 5:37 PM, Nathan Chancellor <[email protected]>
>wrote:
>> > > > >>
>> > > > >> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using
>macros in
>> > > > >> inline assembly code to work around asm() related GCC
>inlining bugs")
>> > > > >> added this flag to KBUILD_CFLAGS, where it works perfectly
>fine with
>> > > > >> GCC. However, when building with Clang, all of the object
>files compile
>> > > > >> fine but the build hangs indefinitely at init/main.o, right
>before the
>> > > > >> linking stage. Don't include this flag when building with
>Clang.
>> > > > >>
>> > > > >> The kernel builds and boots to a shell in QEMU with both GCC
>and Clang
>> > > > >> with this patch applied.
>> > > > >>
>> > > > >> Link:
>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&amp;data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&amp;sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&amp;reserved=0
>> > > > >> Signed-off-by: Nathan Chancellor <[email protected]>
>> > > > >> ---
>> > > > >>
>> > > > >> The reason this patch is labeled RFC is while I can verify
>that this
>> > > > >> fixes the issue, I'm not entirely sure why the '-Wa,-' works
>for GCC
>> > > > >> and not Clang. I looked into what the flag means and I
>couldn't really
>> > > > >> find anything so I just assume it's taking input from stdin?
>The issue
>> > > > >> could stem from how GCC forks gas versus how Clang does it.
>If this
>> > > > >> isn't of concern and the maintainers are happy with this
>patch as is,
>> > > > >> feel free to take it.
>> > > > >>
>> > > >
>> > > > Perhaps someone could actually, you know, time the build and
>see how
>> > > > much -pipe actually matters, if at all?
>> > > >
>> > > > -hpa
>> > > >
>> > >
>> > > Thank you for the suggestion! With the attached diff for removing
>> > > '-pipe' and 'make -j1' with defconfig (just to make sure any
>variance
>> > > would stand out), here are my results:
>> > >
>> > > -pipe (GCC):
>> > >
>> > > real 15m55.202s
>> > > user 14m17.748s
>> > > sys 1m47.496s
>> > >
>> > > No -pipe (GCC):
>> > >
>> > > real 16m4.430s
>> > > user 14m16.277s
>> > > sys 1m46.604s
>> > >
>> > > -pipe (Clang):
>> > >
>> > > real 21m26.016s
>> > > user 19m21.722s
>> > > sys 2m2.606s
>> > >
>> > > No -pipe (Clang):
>> > >
>> > > real 21m27.822s
>> > > user 19m22.092s
>> > > sys 2m4.151s
>> >
>> > Looks like Clang eats `-pipe`:
>> >
>https://github.com/llvm-mirror/clang/blob/391667a023f79287f9c40868f34f08c161555556/lib/Driver/Driver.cpp#L962
>> > commit r110007 has the log:
>> > Driver: Start ripping out support for -pipe, which is worthless
>> > and complicates
>> > too many other things.
>> >
>>
>> In that case, we can either keep this change (I'll resend with the
>> explanation that Clang doesn't respect -pipe) or we can just rip out
>> -pipe for GCC too. Here are three separate results for GCC with my
>> normal jobs flag:
>>
>> -pipe (GCC):
>>
>> real 3m40.813s
>> real 3m44.449s
>> real 3m39.648s
>>
>> No -pipe (GCC):
>>
>> real 3m38.492s
>> real 3m38.335s
>> real 3m38.975s
>>
>> Practically no variance.
>
>Thanks for these measurements. With these in mind I agree with HPA
>that `-pipe -Wa,-` doesn't buy us anything, and would be simpler to
>remove it for compatibility with Clang.
>
>>
>> Thanks,
>> Nathan
>>
>> > >
>> > > Certainly seems like -pipe doesn't make a ton of difference. If
>this is
>> > > a better fix, I am happy to draft up a proper commit message and
>send
>> > > it out for review.
>> > >
>> > > ==================================================
>> > >
>> > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>> > > index 73f4831283ac..672c689c1faa 100644
>> > > --- a/arch/x86/Makefile
>> > > +++ b/arch/x86/Makefile
>> > > @@ -213,8 +213,6 @@ ifdef CONFIG_X86_64
>> > > KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
>> > > endif
>> > >
>> > > -# Speed up the build
>> > > -KBUILD_CFLAGS += -pipe
>> > > # Workaround for a gcc prelease that unfortunately was shipped
>in a suse release
>> > > KBUILD_CFLAGS += -Wno-sign-compare
>> > > #
>> > > @@ -239,7 +237,7 @@ archheaders:
>> > > archmacros:
>> > > $(Q)$(MAKE) $(build)=arch/x86/kernel
>arch/x86/kernel/macros.s
>> > >
>> > > -ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,-
>> > > +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
>> > > export ASM_MACRO_FLAGS
>> > > KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
>> > >
>> >
>> >
>> > --
>> > Thanks,
>> > ~Nick Desaulniers

So -pipe actually hurts sightly. Let's kill it.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.