diff mbox series

[v2,3/4] Makefile: lld: tell clang to use lld

Message ID 20190211193008.24101-3-ndesaulniers@google.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/4] init/Kconfig: add config support for detecting linker | expand

Commit Message

Nick Desaulniers Feb. 11, 2019, 7:30 p.m. UTC
This is needed because clang doesn't select which linker to use based on
$LD but rather -fuse-ld=lld. 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>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1->V2:
* add reviewed and tested by tags.
* move this addition up 2 statments so that it's properly added to
  KBUILD_*FLAGS as per Nathan.

 Makefile | 3 +++
 1 file changed, 3 insertions(+)

Comments

Masahiro Yamada Feb. 13, 2019, 2:58 p.m. UTC | #1
On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
>
> This is needed because clang doesn't select which linker to use based on
> $LD but rather -fuse-ld=lld. This is problematic especially for
> cc-ldoption, which checks for linker flag support via invoking the
> compiler, rather than the linker.


Sorry, please explain what kind of problem
this patch solves.



[1] $(LD) is used to link vmlinux, modules, etc.

[2] $(CC) is used to link vdso, etc.
    and -fuse-ld= selects which linker is invoked from $(CC)


Is it a problem to use a different
type of linker for [1] and [2] ?


Thanks.





> Link: https://github.com/ClangBuiltLinux/linux/issues/342
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1->V2:
> * add reviewed and tested by tags.
> * move this addition up 2 statments so that it's properly added to
>   KBUILD_*FLAGS as per Nathan.
>
>  Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index d3b65e96d183..00e8e01d23fc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -500,6 +500,9 @@ ifneq ($(GCC_TOOLCHAIN),)
>  CLANG_FLAGS    += --gcc-toolchain=$(GCC_TOOLCHAIN)
>  endif
>  CLANG_FLAGS    += -no-integrated-as
> +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> +CLANG_FLAGS    += -fuse-ld=lld
> +endif
>  KBUILD_CFLAGS  += $(CLANG_FLAGS)
>  KBUILD_AFLAGS  += $(CLANG_FLAGS)
>  export CLANG_FLAGS
> --
> 2.20.1.791.gb4d0f1c61a-goog
>


--
Best Regards
Masahiro Yamada
Nick Desaulniers Feb. 13, 2019, 5:41 p.m. UTC | #2
On Wed, Feb 13, 2019 at 6:59 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
> >
> > This is needed because clang doesn't select which linker to use based on
> > $LD but rather -fuse-ld=lld. This is problematic especially for
> > cc-ldoption, which checks for linker flag support via invoking the
> > compiler, rather than the linker.
>
>
> Sorry, please explain what kind of problem
> this patch solves.
>
>
>
> [1] $(LD) is used to link vmlinux, modules, etc.
>
> [2] $(CC) is used to link vdso, etc.
>     and -fuse-ld= selects which linker is invoked from $(CC)

It solves case 2.

>
>
> Is it a problem to use a different
> type of linker for [1] and [2] ?

Ideally, no, but I can think of at least one case where it might be
problematic to mix linkers like that:
You might be mixing linker flags added via ld-option from one linker
that aren't actually supported by the other linker.
Masahiro Yamada Feb. 16, 2019, 3:07 a.m. UTC | #3
On Thu, Feb 14, 2019 at 8:08 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Feb 13, 2019 at 6:59 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
> > >
> > > This is needed because clang doesn't select which linker to use based on
> > > $LD but rather -fuse-ld=lld. This is problematic especially for
> > > cc-ldoption, which checks for linker flag support via invoking the
> > > compiler, rather than the linker.
> >
> >
> > Sorry, please explain what kind of problem
> > this patch solves.
> >
> >
> >
> > [1] $(LD) is used to link vmlinux, modules, etc.
> >
> > [2] $(CC) is used to link vdso, etc.
> >     and -fuse-ld= selects which linker is invoked from $(CC)
>
> It solves case 2.
>
> >
> >
> > Is it a problem to use a different
> > type of linker for [1] and [2] ?
>
> Ideally, no, but I can think of at least one case where it might be
> problematic to mix linkers like that:
> You might be mixing linker flags added via ld-option from one linker
> that aren't actually supported by the other linker.

You can do this only when you are sure
that the _exactly_ same linker is used.

In my understanding, -fuse-ld=lld does not guarantee it.
Nick Desaulniers April 2, 2019, 3:54 a.m. UTC | #4
On Sat, Feb 16, 2019 at 10:09 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Thu, Feb 14, 2019 at 8:08 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Wed, Feb 13, 2019 at 6:59 AM Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> > >
> > > On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
> > > >
> > > > This is needed because clang doesn't select which linker to use based on
> > > > $LD but rather -fuse-ld=lld. This is problematic especially for
> > > > cc-ldoption, which checks for linker flag support via invoking the
> > > > compiler, rather than the linker.
> > >
> > >
> > > Sorry, please explain what kind of problem
> > > this patch solves.
> > >
> > >
> > >
> > > [1] $(LD) is used to link vmlinux, modules, etc.
> > >
> > > [2] $(CC) is used to link vdso, etc.
> > >     and -fuse-ld= selects which linker is invoked from $(CC)
> >
> > It solves case 2.
> >
> > >
> > >
> > > Is it a problem to use a different
> > > type of linker for [1] and [2] ?
> >
> > Ideally, no, but I can think of at least one case where it might be
> > problematic to mix linkers like that:
> > You might be mixing linker flags added via ld-option from one linker
> > that aren't actually supported by the other linker.
>
> You can do this only when you are sure
> that the _exactly_ same linker is used.
>
> In my understanding, -fuse-ld=lld does not guarantee it.

I really don't think we should be mixing and matching linkers during a
kernel build.  When we compile with clang, we don't have escape
hatches that allow for some object files to be compiled with GCC
(mixing clang and GCC compiled object files into one build).
Following the same logic, I think mixing linkers during kernel build
should similarly be dissuaded.  This patch AVOIDS clang using a
different linker than what was specified via $LD, which is CRITICAL
for cc-ldoption kbuild macro.  Masahiro, I hope this patch can be
re-evaluated, or if I'm not understanding your point, that you can
provide additional clarification.
Masahiro Yamada April 2, 2019, 4:49 a.m. UTC | #5
Hi Nick,


On Tue, Apr 2, 2019 at 12:54 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Sat, Feb 16, 2019 at 10:09 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > On Thu, Feb 14, 2019 at 8:08 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > On Wed, Feb 13, 2019 at 6:59 AM Masahiro Yamada
> > > <yamada.masahiro@socionext.com> wrote:
> > > >
> > > > On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
> > > > >
> > > > > This is needed because clang doesn't select which linker to use based on
> > > > > $LD but rather -fuse-ld=lld. This is problematic especially for
> > > > > cc-ldoption, which checks for linker flag support via invoking the
> > > > > compiler, rather than the linker.
> > > >
> > > >
> > > > Sorry, please explain what kind of problem
> > > > this patch solves.
> > > >
> > > >
> > > >
> > > > [1] $(LD) is used to link vmlinux, modules, etc.
> > > >
> > > > [2] $(CC) is used to link vdso, etc.
> > > >     and -fuse-ld= selects which linker is invoked from $(CC)
> > >
> > > It solves case 2.
> > >
> > > >
> > > >
> > > > Is it a problem to use a different
> > > > type of linker for [1] and [2] ?
> > >
> > > Ideally, no, but I can think of at least one case where it might be
> > > problematic to mix linkers like that:
> > > You might be mixing linker flags added via ld-option from one linker
> > > that aren't actually supported by the other linker.
> >
> > You can do this only when you are sure
> > that the _exactly_ same linker is used.
> >
> > In my understanding, -fuse-ld=lld does not guarantee it.
>
> I really don't think we should be mixing and matching linkers during a
> kernel build.  When we compile with clang, we don't have escape
> hatches that allow for some object files to be compiled with GCC
> (mixing clang and GCC compiled object files into one build).
> Following the same logic, I think mixing linkers during kernel build
> should similarly be dissuaded.  This patch AVOIDS clang using a
> different linker than what was specified via $LD, which is CRITICAL
> for cc-ldoption kbuild macro.  Masahiro, I hope this patch can be
> re-evaluated, or if I'm not understanding your point, that you can
> provide additional clarification.
>



You can pass an absolute pass to LD, like

make LD=/path/to/my/llvm/install/dir/bin/ld.lld

This clarifies which linker is being used
even when multiple versions of llvm are installed
on the machine.


However, -fuse-ld=lld is ambiguous.
Will it use the first ld.lld found in PATH?

So, you cannot avoid mixing linkers by this means.


If we could do -fuse=$(LD), I would agree with it.
Clang accepts -fuse=<absolute-path>, GCC does not.

Is there a way to control the linker search path?
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index d3b65e96d183..00e8e01d23fc 100644
--- a/Makefile
+++ b/Makefile
@@ -500,6 +500,9 @@  ifneq ($(GCC_TOOLCHAIN),)
 CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
 endif
 CLANG_FLAGS	+= -no-integrated-as
+ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
+CLANG_FLAGS	+= -fuse-ld=lld
+endif
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)
 KBUILD_AFLAGS	+= $(CLANG_FLAGS)
 export CLANG_FLAGS