linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/4] Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD
       [not found] <20190207220155.81351-1-ndesaulniers@google.com>
@ 2019-02-07 22:01 ` ndesaulniers
  2019-02-08  5:45   ` Nathan Chancellor
  2019-02-07 22:01 ` [PATCH 3/4] Makefile: lld: tell clang to use lld ndesaulniers
  2019-02-07 22:01 ` [PATCH 4/4] Makefile: lld: set -O2 linker flag when linking with LLD ndesaulniers
  2 siblings, 1 reply; 13+ messages in thread
From: ndesaulniers @ 2019-02-07 22:01 UTC (permalink / raw)
  To: yamada.masahiro
  Cc: Nick Desaulniers, Nathan Chancellor, Michal Marek, linux-kbuild,
	linux-kernel

This causes an issue when trying to build with `make LD=ld.lld` if
ld.lld and the rest of your cross tools aren't in the same directory
(ex. /usr/local/bin) (as is the case for Android's build system), as the
GCC_TOOLCHAIN_DIR then gets set based on `which $(LD)` which will point
where LLVM tools are, not GCC/binutils tools are located.

Instead, select the GCC_TOOLCHAIN_DIR based on another tool provided by
binutils for which LLVM does not provide a substitute for, such as
elfedit.

Fixes commit 785f11aa595b ("kbuild: Add better clang cross build support")

Link: https://github.com/ClangBuiltLinux/linux/issues/341
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 3142e67d03f1..0eae4277206e 100644
--- a/Makefile
+++ b/Makefile
@@ -492,7 +492,7 @@ endif
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
 ifneq ($(CROSS_COMPILE),)
 CLANG_FLAGS	:= --target=$(notdir $(CROSS_COMPILE:%-=%))
-GCC_TOOLCHAIN_DIR := $(dir $(shell which $(LD)))
+GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
 CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)
 GCC_TOOLCHAIN	:= $(realpath $(GCC_TOOLCHAIN_DIR)/..)
 endif
-- 
2.20.1.791.gb4d0f1c61a-goog

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

* [PATCH 3/4] Makefile: lld: tell clang to use lld
       [not found] <20190207220155.81351-1-ndesaulniers@google.com>
  2019-02-07 22:01 ` [PATCH 2/4] Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD ndesaulniers
@ 2019-02-07 22:01 ` ndesaulniers
  2019-02-08  5:50   ` Nathan Chancellor
  2019-02-07 22:01 ` [PATCH 4/4] Makefile: lld: set -O2 linker flag when linking with LLD ndesaulniers
  2 siblings, 1 reply; 13+ messages in thread
From: ndesaulniers @ 2019-02-07 22:01 UTC (permalink / raw)
  To: yamada.masahiro
  Cc: Nick Desaulniers, Nathan Chancellor, Michal Marek, linux-kbuild,
	linux-kernel

This is needed because clang doesn't select which linker to use based on
$LD but rather -fuse-ld=$(LD). This is problematic especially for
cc-ldoption, which checks for linker flag support via invoking the
compiler, rather than the linker.

Link: https://github.com/ClangBuiltLinux/linux/issues/342
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile b/Makefile
index 0eae4277206e..6307c17259ea 100644
--- a/Makefile
+++ b/Makefile
@@ -502,6 +502,9 @@ endif
 CLANG_FLAGS	+= -no-integrated-as
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)
 KBUILD_AFLAGS	+= $(CLANG_FLAGS)
+ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
+CLANG_FLAGS	+= -fuse-ld=lld
+endif
 export CLANG_FLAGS
 endif
 
-- 
2.20.1.791.gb4d0f1c61a-goog

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

* [PATCH 4/4] Makefile: lld: set -O2 linker flag when linking with LLD
       [not found] <20190207220155.81351-1-ndesaulniers@google.com>
  2019-02-07 22:01 ` [PATCH 2/4] Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD ndesaulniers
  2019-02-07 22:01 ` [PATCH 3/4] Makefile: lld: tell clang to use lld ndesaulniers
@ 2019-02-07 22:01 ` ndesaulniers
  2019-02-08  6:07   ` Nathan Chancellor
  2 siblings, 1 reply; 13+ messages in thread
From: ndesaulniers @ 2019-02-07 22:01 UTC (permalink / raw)
  To: yamada.masahiro
  Cc: Nick Desaulniers, Rui Ueyama, Nathan Chancellor, Michal Marek,
	linux-kbuild, linux-kernel

-O2 enables tail merging of string table strings.

For arm64:
0.34% size improvement with lld -O2 over lld for vmlinux.
3.30% size improvement with lld -O2 over lld for Image.lz4-dtb.

Link: https://github.com/ClangBuiltLinux/linux/issues/343
Suggested-by: Rui Ueyama <ruiu@google.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 6307c17259ea..c07208ec49d4 100644
--- a/Makefile
+++ b/Makefile
@@ -718,6 +718,10 @@ else
 KBUILD_CFLAGS += -Wno-unused-but-set-variable
 endif
 
+ifdef CONFIG_LD_IS_LLD
+KBUILD_LDFLAGS += -O2
+endif
+
 KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
 ifdef CONFIG_FRAME_POINTER
 KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
-- 
2.20.1.791.gb4d0f1c61a-goog

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

* Re: [PATCH 2/4] Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD
  2019-02-07 22:01 ` [PATCH 2/4] Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD ndesaulniers
@ 2019-02-08  5:45   ` Nathan Chancellor
  0 siblings, 0 replies; 13+ messages in thread
From: Nathan Chancellor @ 2019-02-08  5:45 UTC (permalink / raw)
  To: ndesaulniers; +Cc: yamada.masahiro, Michal Marek, linux-kbuild, linux-kernel

On Thu, Feb 07, 2019 at 02:01:50PM -0800, ndesaulniers@google.com wrote:
> This causes an issue when trying to build with `make LD=ld.lld` if
> ld.lld and the rest of your cross tools aren't in the same directory
> (ex. /usr/local/bin) (as is the case for Android's build system), as the
> GCC_TOOLCHAIN_DIR then gets set based on `which $(LD)` which will point
> where LLVM tools are, not GCC/binutils tools are located.
> 
> Instead, select the GCC_TOOLCHAIN_DIR based on another tool provided by
> binutils for which LLVM does not provide a substitute for, such as
> elfedit.
> 
> Fixes commit 785f11aa595b ("kbuild: Add better clang cross build support")
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/341
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 3142e67d03f1..0eae4277206e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -492,7 +492,7 @@ endif
>  ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
>  ifneq ($(CROSS_COMPILE),)
>  CLANG_FLAGS	:= --target=$(notdir $(CROSS_COMPILE:%-=%))
> -GCC_TOOLCHAIN_DIR := $(dir $(shell which $(LD)))
> +GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)
>  GCC_TOOLCHAIN	:= $(realpath $(GCC_TOOLCHAIN_DIR)/..)
>  endif
> -- 
> 2.20.1.791.gb4d0f1c61a-goog
> 

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

* Re: [PATCH 3/4] Makefile: lld: tell clang to use lld
  2019-02-07 22:01 ` [PATCH 3/4] Makefile: lld: tell clang to use lld ndesaulniers
@ 2019-02-08  5:50   ` Nathan Chancellor
  2019-02-08 17:27     ` Nick Desaulniers
  2019-02-11 19:26     ` Nick Desaulniers
  0 siblings, 2 replies; 13+ messages in thread
From: Nathan Chancellor @ 2019-02-08  5:50 UTC (permalink / raw)
  To: ndesaulniers; +Cc: yamada.masahiro, Michal Marek, linux-kbuild, linux-kernel

On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> This is needed because clang doesn't select which linker to use based on
> $LD but rather -fuse-ld=$(LD). This is problematic especially for
> cc-ldoption, which checks for linker flag support via invoking the
> compiler, rather than the linker.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/342
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 0eae4277206e..6307c17259ea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -502,6 +502,9 @@ endif
>  CLANG_FLAGS	+= -no-integrated-as
>  KBUILD_CFLAGS	+= $(CLANG_FLAGS)
>  KBUILD_AFLAGS	+= $(CLANG_FLAGS)
> +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> +CLANG_FLAGS	+= -fuse-ld=lld
> +endif

This section needs to be moved up above KBUILD_CFLAGS otherwise it never
actually gets passed along.

With that change, please add:

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

>  export CLANG_FLAGS
>  endif
>  
> -- 
> 2.20.1.791.gb4d0f1c61a-goog
> 

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

* Re: [PATCH 4/4] Makefile: lld: set -O2 linker flag when linking with LLD
  2019-02-07 22:01 ` [PATCH 4/4] Makefile: lld: set -O2 linker flag when linking with LLD ndesaulniers
@ 2019-02-08  6:07   ` Nathan Chancellor
  0 siblings, 0 replies; 13+ messages in thread
From: Nathan Chancellor @ 2019-02-08  6:07 UTC (permalink / raw)
  To: ndesaulniers
  Cc: yamada.masahiro, Rui Ueyama, Michal Marek, linux-kbuild,
	linux-kernel

On Thu, Feb 07, 2019 at 02:01:52PM -0800, ndesaulniers@google.com wrote:
> -O2 enables tail merging of string table strings.
> 
> For arm64:
> 0.34% size improvement with lld -O2 over lld for vmlinux.
> 3.30% size improvement with lld -O2 over lld for Image.lz4-dtb.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/343
> Suggested-by: Rui Ueyama <ruiu@google.com>
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 6307c17259ea..c07208ec49d4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -718,6 +718,10 @@ else
>  KBUILD_CFLAGS += -Wno-unused-but-set-variable
>  endif
>  
> +ifdef CONFIG_LD_IS_LLD
> +KBUILD_LDFLAGS += -O2
> +endif
> +
>  KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
>  ifdef CONFIG_FRAME_POINTER
>  KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
> -- 
> 2.20.1.791.gb4d0f1c61a-goog
> 

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

* Re: [PATCH 3/4] Makefile: lld: tell clang to use lld
  2019-02-08  5:50   ` Nathan Chancellor
@ 2019-02-08 17:27     ` Nick Desaulniers
  2019-02-08 17:45       ` Nathan Chancellor
  2019-02-11 15:32       ` Masahiro Yamada
  2019-02-11 19:26     ` Nick Desaulniers
  1 sibling, 2 replies; 13+ messages in thread
From: Nick Desaulniers @ 2019-02-08 17:27 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, LKML,
	sedat.dilek

On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > This is needed because clang doesn't select which linker to use based on
> > $LD but rather -fuse-ld=$(LD). This is problematic especially for
> > cc-ldoption, which checks for linker flag support via invoking the
> > compiler, rather than the linker.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 0eae4277206e..6307c17259ea 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -502,6 +502,9 @@ endif
> >  CLANG_FLAGS  += -no-integrated-as
> >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > +CLANG_FLAGS  += -fuse-ld=lld

Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
since you may have lld installed as lld-7 or lld-8, for example:
https://github.com/ClangBuiltLinux/linux/issues/358

> > +endif
>
> This section needs to be moved up above KBUILD_CFLAGS otherwise it never
> actually gets passed along.
>
> With that change, please add:
>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>
> >  export CLANG_FLAGS
> >  endif
> >
> > --
> > 2.20.1.791.gb4d0f1c61a-goog
> >



-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 3/4] Makefile: lld: tell clang to use lld
  2019-02-08 17:27     ` Nick Desaulniers
@ 2019-02-08 17:45       ` Nathan Chancellor
  2019-02-11 15:32       ` Masahiro Yamada
  1 sibling, 0 replies; 13+ messages in thread
From: Nathan Chancellor @ 2019-02-08 17:45 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, LKML,
	sedat.dilek

On Fri, Feb 08, 2019 at 09:27:19AM -0800, Nick Desaulniers wrote:
> On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > > This is needed because clang doesn't select which linker to use based on
> > > $LD but rather -fuse-ld=$(LD). This is problematic especially for
> > > cc-ldoption, which checks for linker flag support via invoking the
> > > compiler, rather than the linker.
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  Makefile | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 0eae4277206e..6307c17259ea 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -502,6 +502,9 @@ endif
> > >  CLANG_FLAGS  += -no-integrated-as
> > >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> > >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > > +CLANG_FLAGS  += -fuse-ld=lld
> 
> Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
> matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
> since you may have lld installed as lld-7 or lld-8, for example:
> https://github.com/ClangBuiltLinux/linux/issues/358
> 

We'll need to make sure that it is compatible with the way that Clang
expects arguments to '-fuse-ld=' because it looks like 'ld.' is
automatically appended.

https://github.com/llvm/llvm-project/blob/5be3dbdb082ffba4f846e3572ea802d156dd8d34/clang/lib/Driver/ToolChain.cpp#L438-L468

> > > +endif
> >
> > This section needs to be moved up above KBUILD_CFLAGS otherwise it never
> > actually gets passed along.
> >
> > With that change, please add:
> >
> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> > Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> >
> > >  export CLANG_FLAGS
> > >  endif
> > >
> > > --
> > > 2.20.1.791.gb4d0f1c61a-goog
> > >
> 
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers

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

* Re: [PATCH 3/4] Makefile: lld: tell clang to use lld
  2019-02-08 17:27     ` Nick Desaulniers
  2019-02-08 17:45       ` Nathan Chancellor
@ 2019-02-11 15:32       ` Masahiro Yamada
  2019-02-11 16:05         ` Sedat Dilek
  2019-02-11 19:27         ` Nick Desaulniers
  1 sibling, 2 replies; 13+ messages in thread
From: Masahiro Yamada @ 2019-02-11 15:32 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Michal Marek, Linux Kbuild mailing list, LKML,
	Sedat Dilek

On Sat, Feb 9, 2019 at 2:27 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > > This is needed because clang doesn't select which linker to use based on
> > > $LD but rather -fuse-ld=$(LD). This is problematic especially for
> > > cc-ldoption, which checks for linker flag support via invoking the
> > > compiler, rather than the linker.

What do you mean by 'problematic' ?

I see no build error in
arch/arm64/kernel/vdso/Makefile, at least.

Clang uses BFD linker by default,
which may not match to the $(LD) passed in.

Is this what you mean?





> > > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  Makefile | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 0eae4277206e..6307c17259ea 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -502,6 +502,9 @@ endif
> > >  CLANG_FLAGS  += -no-integrated-as
> > >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> > >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > > +CLANG_FLAGS  += -fuse-ld=lld
>
> Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
> matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
> since you may have lld installed as lld-7 or lld-8, for example:
> https://github.com/ClangBuiltLinux/linux/issues/358


Hmm, $(LD) might be a full path.

Looks like Clang can take a full-path for -fuse-ld=,
but does it work for GCC?






> > > +endif
> >
> > This section needs to be moved up above KBUILD_CFLAGS otherwise it never
> > actually gets passed along.
> >
> > With that change, please add:
> >
> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> > Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> >
> > >  export CLANG_FLAGS
> > >  endif
> > >
> > > --
> > > 2.20.1.791.gb4d0f1c61a-goog
> > >
>
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 3/4] Makefile: lld: tell clang to use lld
  2019-02-11 15:32       ` Masahiro Yamada
@ 2019-02-11 16:05         ` Sedat Dilek
  2019-02-11 19:30           ` Nick Desaulniers
  2019-02-11 19:27         ` Nick Desaulniers
  1 sibling, 1 reply; 13+ messages in thread
From: Sedat Dilek @ 2019-02-11 16:05 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nick Desaulniers, Nathan Chancellor, Michal Marek,
	Linux Kbuild mailing list, LKML

On Mon, Feb 11, 2019 at 4:33 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
[ ... ]
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > ---
> > > >  Makefile | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 0eae4277206e..6307c17259ea 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -502,6 +502,9 @@ endif
> > > >  CLANG_FLAGS  += -no-integrated-as
> > > >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> > > >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > > > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > > > +CLANG_FLAGS  += -fuse-ld=lld
> >
> > Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
> > matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
> > since you may have lld installed as lld-7 or lld-8, for example:
> > https://github.com/ClangBuiltLinux/linux/issues/358
>
>
> Hmm, $(LD) might be a full path.
>
> Looks like Clang can take a full-path for -fuse-ld=,
> but does it work for GCC?
>

Nope. GCC expects "-fuse-ld={bfd,gold,lld} ".
Adding suffix "$-VER" was not recognized while building a
llvm-toolchain, so I guess this will be the same for the Linux kernel
side.

- Sedat -

[1] https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html
[2] https://github.com/ClangBuiltLinux/linux/issues/358

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

* Re: [PATCH 3/4] Makefile: lld: tell clang to use lld
  2019-02-08  5:50   ` Nathan Chancellor
  2019-02-08 17:27     ` Nick Desaulniers
@ 2019-02-11 19:26     ` Nick Desaulniers
  1 sibling, 0 replies; 13+ messages in thread
From: Nick Desaulniers @ 2019-02-11 19:26 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, LKML

On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > This is needed because clang doesn't select which linker to use based on
> > $LD but rather -fuse-ld=$(LD). This is problematic especially for
> > cc-ldoption, which checks for linker flag support via invoking the
> > compiler, rather than the linker.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 0eae4277206e..6307c17259ea 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -502,6 +502,9 @@ endif
> >  CLANG_FLAGS  += -no-integrated-as
> >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > +CLANG_FLAGS  += -fuse-ld=lld
> > +endif
>
> This section needs to be moved up above KBUILD_CFLAGS otherwise it never
> actually gets passed along.

Great catch! I had that correct in my Pixel kernel patches; messed
that up forward porting them.  Included in v2 (sending imminently).

-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 3/4] Makefile: lld: tell clang to use lld
  2019-02-11 15:32       ` Masahiro Yamada
  2019-02-11 16:05         ` Sedat Dilek
@ 2019-02-11 19:27         ` Nick Desaulniers
  1 sibling, 0 replies; 13+ messages in thread
From: Nick Desaulniers @ 2019-02-11 19:27 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nathan Chancellor, Michal Marek, Linux Kbuild mailing list, LKML,
	Sedat Dilek

On Mon, Feb 11, 2019 at 7:33 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Sat, Feb 9, 2019 at 2:27 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > >
> > > On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > > > This is needed because clang doesn't select which linker to use based on
> > > > $LD but rather -fuse-ld=$(LD). This is problematic especially for
> > > > cc-ldoption, which checks for linker flag support via invoking the
> > > > compiler, rather than the linker.
>
> What do you mean by 'problematic' ?
>
> I see no build error in
> arch/arm64/kernel/vdso/Makefile, at least.
>
> Clang uses BFD linker by default,
> which may not match to the $(LD) passed in.
>
> Is this what you mean?

Right, when using a linker other than BFD, we want cc-ldoption to tell
us whether that linker supports a linker flag or not.

-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH 3/4] Makefile: lld: tell clang to use lld
  2019-02-11 16:05         ` Sedat Dilek
@ 2019-02-11 19:30           ` Nick Desaulniers
  0 siblings, 0 replies; 13+ messages in thread
From: Nick Desaulniers @ 2019-02-11 19:30 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Masahiro Yamada, Nathan Chancellor, Michal Marek,
	Linux Kbuild mailing list, LKML

On Mon, Feb 11, 2019 at 8:05 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Mon, Feb 11, 2019 at 4:33 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> [ ... ]
> > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > > > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > > ---
> > > > >  Makefile | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index 0eae4277206e..6307c17259ea 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -502,6 +502,9 @@ endif
> > > > >  CLANG_FLAGS  += -no-integrated-as
> > > > >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> > > > >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > > > > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > > > > +CLANG_FLAGS  += -fuse-ld=lld
> > >
> > > Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
> > > matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
> > > since you may have lld installed as lld-7 or lld-8, for example:
> > > https://github.com/ClangBuiltLinux/linux/issues/358
> >
> >
> > Hmm, $(LD) might be a full path.
> >
> > Looks like Clang can take a full-path for -fuse-ld=,
> > but does it work for GCC?
> >
>
> Nope. GCC expects "-fuse-ld={bfd,gold,lld} ".
> Adding suffix "$-VER" was not recognized while building a
> llvm-toolchain, so I guess this will be the same for the Linux kernel
> side.

Well for now, let's add support for just `ld.lld` since a build of
llvm will create such a symlink.  If folks really need to
differentiate between different versions of lld, let's cross that
bridge when we get there.
-- 
Thanks,
~Nick Desaulniers

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

end of thread, other threads:[~2019-02-11 19:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20190207220155.81351-1-ndesaulniers@google.com>
2019-02-07 22:01 ` [PATCH 2/4] Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD ndesaulniers
2019-02-08  5:45   ` Nathan Chancellor
2019-02-07 22:01 ` [PATCH 3/4] Makefile: lld: tell clang to use lld ndesaulniers
2019-02-08  5:50   ` Nathan Chancellor
2019-02-08 17:27     ` Nick Desaulniers
2019-02-08 17:45       ` Nathan Chancellor
2019-02-11 15:32       ` Masahiro Yamada
2019-02-11 16:05         ` Sedat Dilek
2019-02-11 19:30           ` Nick Desaulniers
2019-02-11 19:27         ` Nick Desaulniers
2019-02-11 19:26     ` Nick Desaulniers
2019-02-07 22:01 ` [PATCH 4/4] Makefile: lld: set -O2 linker flag when linking with LLD ndesaulniers
2019-02-08  6:07   ` Nathan Chancellor

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).