linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
@ 2018-10-23  0:37 Nathan Chancellor
       [not found] ` <F3BBC06A-A9CB-4ABF-ADF4-6B0971FAE364@vmware.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Nathan Chancellor @ 2018-10-23  0:37 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov
  Cc: H. Peter Anvin, x86, linux-kernel, Nadav Amit, Kees Cook,
	Masahiro Yamada, Nick Desaulniers, Nathan Chancellor

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 <natechancellor@gmail.com>
---

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
       [not found] ` <F3BBC06A-A9CB-4ABF-ADF4-6B0971FAE364@vmware.com>
@ 2018-10-23 18:40   ` Nick Desaulniers
  2018-10-23 20:01     ` H. Peter Anvin
  0 siblings, 1 reply; 9+ messages in thread
From: Nick Desaulniers @ 2018-10-23 18:40 UTC (permalink / raw)
  To: namit
  Cc: Nathan Chancellor, Thomas Gleixner, mingo, bp, hpa, x86, LKML,
	Kees Cook, Masahiro Yamada

On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <namit@vmware.com> wrote:
>
> at 5:37 PM, Nathan Chancellor <natechancellor@gmail.com> 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 <natechancellor@gmail.com>
> ---
>
> 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 <ndesaulniers@google.com>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
  2018-10-23 18:40   ` Nick Desaulniers
@ 2018-10-23 20:01     ` H. Peter Anvin
  2018-10-23 21:58       ` Nathan Chancellor
  0 siblings, 1 reply; 9+ messages in thread
From: H. Peter Anvin @ 2018-10-23 20:01 UTC (permalink / raw)
  To: Nick Desaulniers, namit
  Cc: Nathan Chancellor, Thomas Gleixner, mingo, bp, x86, LKML,
	Kees Cook, Masahiro Yamada

On 10/23/18 11:40, Nick Desaulniers wrote:
> On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <namit@vmware.com> wrote:
>>
>> at 5:37 PM, Nathan Chancellor <natechancellor@gmail.com> 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 <natechancellor@gmail.com>
>> ---
>>
>> 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
  2018-10-23 20:01     ` H. Peter Anvin
@ 2018-10-23 21:58       ` Nathan Chancellor
  2018-10-23 22:04         ` hpa
  2018-10-23 22:08         ` Nick Desaulniers
  0 siblings, 2 replies; 9+ messages in thread
From: Nathan Chancellor @ 2018-10-23 21:58 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Nick Desaulniers, namit, Thomas Gleixner, mingo, bp, x86, LKML,
	Kees Cook, Masahiro Yamada

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 <namit@vmware.com> wrote:
> >>
> >> at 5:37 PM, Nathan Chancellor <natechancellor@gmail.com> 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 <natechancellor@gmail.com>
> >> ---
> >>
> >> 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)


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
  2018-10-23 21:58       ` Nathan Chancellor
@ 2018-10-23 22:04         ` hpa
  2018-10-23 22:08         ` Nick Desaulniers
  1 sibling, 0 replies; 9+ messages in thread
From: hpa @ 2018-10-23 22:04 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Nick Desaulniers, namit, Thomas Gleixner, mingo, bp, x86, LKML,
	Kees Cook, Masahiro Yamada

On October 23, 2018 2:58:11 PM PDT, Nathan Chancellor <natechancellor@gmail.com> 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 <namit@vmware.com>
>wrote:
>> >>
>> >> at 5:37 PM, Nathan Chancellor <natechancellor@gmail.com> 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 <natechancellor@gmail.com>
>> >> ---
>> >>
>> >> 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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
  2018-10-23 21:58       ` Nathan Chancellor
  2018-10-23 22:04         ` hpa
@ 2018-10-23 22:08         ` Nick Desaulniers
  2018-10-23 22:44           ` Nathan Chancellor
  1 sibling, 1 reply; 9+ messages in thread
From: Nick Desaulniers @ 2018-10-23 22:08 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: hpa, namit, Thomas Gleixner, mingo, bp, x86, LKML, Kees Cook,
	Masahiro Yamada

On Tue, Oct 23, 2018 at 2:58 PM Nathan Chancellor
<natechancellor@gmail.com> 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 <namit@vmware.com> wrote:
> > >>
> > >> at 5:37 PM, Nathan Chancellor <natechancellor@gmail.com> 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 <natechancellor@gmail.com>
> > >> ---
> > >>
> > >> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
  2018-10-23 22:08         ` Nick Desaulniers
@ 2018-10-23 22:44           ` Nathan Chancellor
  2018-10-23 22:53             ` Nick Desaulniers
  0 siblings, 1 reply; 9+ messages in thread
From: Nathan Chancellor @ 2018-10-23 22:44 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: hpa, namit, Thomas Gleixner, mingo, bp, x86, LKML, Kees Cook,
	Masahiro Yamada

On Tue, Oct 23, 2018 at 03:08:53PM -0700, Nick Desaulniers wrote:
> On Tue, Oct 23, 2018 at 2:58 PM Nathan Chancellor
> <natechancellor@gmail.com> 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 <namit@vmware.com> wrote:
> > > >>
> > > >> at 5:37 PM, Nathan Chancellor <natechancellor@gmail.com> 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 <natechancellor@gmail.com>
> > > >> ---
> > > >>
> > > >> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
  2018-10-23 22:44           ` Nathan Chancellor
@ 2018-10-23 22:53             ` Nick Desaulniers
  2018-10-23 23:01               ` hpa
  0 siblings, 1 reply; 9+ messages in thread
From: Nick Desaulniers @ 2018-10-23 22:53 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: hpa, namit, Thomas Gleixner, mingo, bp, x86, LKML, Kees Cook,
	Masahiro Yamada

On Tue, Oct 23, 2018 at 3:44 PM Nathan Chancellor
<natechancellor@gmail.com> 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
> > <natechancellor@gmail.com> 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 <namit@vmware.com> wrote:
> > > > >>
> > > > >> at 5:37 PM, Nathan Chancellor <natechancellor@gmail.com> 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 <natechancellor@gmail.com>
> > > > >> ---
> > > > >>
> > > > >> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang
  2018-10-23 22:53             ` Nick Desaulniers
@ 2018-10-23 23:01               ` hpa
  0 siblings, 0 replies; 9+ messages in thread
From: hpa @ 2018-10-23 23:01 UTC (permalink / raw)
  To: Nick Desaulniers, Nathan Chancellor
  Cc: namit, Thomas Gleixner, mingo, bp, x86, LKML, Kees Cook, Masahiro Yamada

On October 23, 2018 3:53:10 PM PDT, Nick Desaulniers <ndesaulniers@google.com> wrote:
>On Tue, Oct 23, 2018 at 3:44 PM Nathan Chancellor
><natechancellor@gmail.com> 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
>> > <natechancellor@gmail.com> 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
><namit@vmware.com> wrote:
>> > > > >>
>> > > > >> at 5:37 PM, Nathan Chancellor <natechancellor@gmail.com>
>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 <natechancellor@gmail.com>
>> > > > >> ---
>> > > > >>
>> > > > >> 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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-10-23 23:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-23  0:37 [PATCH RFC] x86: Don't include '-Wa,-' when building with Clang Nathan Chancellor
     [not found] ` <F3BBC06A-A9CB-4ABF-ADF4-6B0971FAE364@vmware.com>
2018-10-23 18:40   ` Nick Desaulniers
2018-10-23 20:01     ` H. Peter Anvin
2018-10-23 21:58       ` Nathan Chancellor
2018-10-23 22:04         ` hpa
2018-10-23 22:08         ` Nick Desaulniers
2018-10-23 22:44           ` Nathan Chancellor
2018-10-23 22:53             ` Nick Desaulniers
2018-10-23 23:01               ` hpa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).