diff mbox series

[1/2] Makefile: Remove '--gcc-toolchain' flag

Message ID 20210302210646.3044738-1-nathan@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/2] Makefile: Remove '--gcc-toolchain' flag | expand

Commit Message

Nathan Chancellor March 2, 2021, 9:06 p.m. UTC 
This is not necessary anymore now that we specify '--prefix=', which
tells clang exactly where to find the GNU cross tools. This has been
verified with self compiled LLVM 10.0.1 and LLVM 13.0.0 as well as a
distribution version of LLVM 11.1.0 without binutils in the LLVM
toolchain locations.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 Makefile | 4 ----
 1 file changed, 4 deletions(-)


base-commit: 7a7fd0de4a9804299793e564a555a49c1fc924cb

Comments

Fangrui Song March 2, 2021, 9:43 p.m. UTC | #1 
Reviewed-by: Fangrui Song <maskray@google.com>

Thanks for the clean-up!
--gcc-toolchain= is an obsscure way searching for GCC installation prefixes (--prefix).
The logic is complex and different for different distributions/architectures.

If we specify --prefix= (-B) explicitly, --gcc-toolchain is not needed.

On 2021-03-02, Nathan Chancellor wrote:
>This is not necessary anymore now that we specify '--prefix=', which
>tells clang exactly where to find the GNU cross tools. This has been
>verified with self compiled LLVM 10.0.1 and LLVM 13.0.0 as well as a
>distribution version of LLVM 11.1.0 without binutils in the LLVM
>toolchain locations.
>
>Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>---
> Makefile | 4 ----
> 1 file changed, 4 deletions(-)
>
>diff --git a/Makefile b/Makefile
>index f9b54da2fca0..c20f0ad8be73 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -568,10 +568,6 @@ ifneq ($(CROSS_COMPILE),)
> CLANG_FLAGS	+= --target=$(notdir $(CROSS_COMPILE:%-=%))
> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
>-GCC_TOOLCHAIN	:= $(realpath $(GCC_TOOLCHAIN_DIR)/..)
>-endif
>-ifneq ($(GCC_TOOLCHAIN),)
>-CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
> endif
> ifneq ($(LLVM_IAS),1)
> CLANG_FLAGS	+= -no-integrated-as
>
>base-commit: 7a7fd0de4a9804299793e564a555a49c1fc924cb
>-- 
>2.31.0.rc0.75.gec125d1bc1
>
>-- 
>You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
>To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210302210646.3044738-1-nathan%40kernel.org.
Nick Desaulniers March 2, 2021, 10:33 p.m. UTC | #2 
On Tue, Mar 2, 2021 at 1:07 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> This is not necessary anymore now that we specify '--prefix=', which
> tells clang exactly where to find the GNU cross tools. This has been
> verified with self compiled LLVM 10.0.1 and LLVM 13.0.0 as well as a
> distribution version of LLVM 11.1.0 without binutils in the LLVM
> toolchain locations.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

I see this pattern still being used in
arch/arm64/kernel/vdso32/Makefile, but that can be separate cleanups.

> ---
>  Makefile | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f9b54da2fca0..c20f0ad8be73 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -568,10 +568,6 @@ ifneq ($(CROSS_COMPILE),)
>  CLANG_FLAGS    += --target=$(notdir $(CROSS_COMPILE:%-=%))
>  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS    += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> -GCC_TOOLCHAIN  := $(realpath $(GCC_TOOLCHAIN_DIR)/..)
> -endif
> -ifneq ($(GCC_TOOLCHAIN),)
> -CLANG_FLAGS    += --gcc-toolchain=$(GCC_TOOLCHAIN)
>  endif
>  ifneq ($(LLVM_IAS),1)
>  CLANG_FLAGS    += -no-integrated-as
>
> base-commit: 7a7fd0de4a9804299793e564a555a49c1fc924cb
> --
> 2.31.0.rc0.75.gec125d1bc1
>
Sedat Dilek March 3, 2021, 3:25 a.m. UTC | #3 
On Tue, Mar 2, 2021 at 10:07 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> This is not necessary anymore now that we specify '--prefix=', which
> tells clang exactly where to find the GNU cross tools. This has been
> verified with self compiled LLVM 10.0.1 and LLVM 13.0.0 as well as a
> distribution version of LLVM 11.1.0 without binutils in the LLVM
> toolchain locations.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

[ CC Behan ]

Hahaha, that is a change of a very early commit in times of the
LLVM/Clang Linux-kernel development.
So-to-say a historical change :-).

I will try this patchset later with latest Linux -v5.12-rc1+ and my
custom patchset.

Thanks.

- Sedat -


> ---
>  Makefile | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f9b54da2fca0..c20f0ad8be73 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -568,10 +568,6 @@ ifneq ($(CROSS_COMPILE),)
>  CLANG_FLAGS    += --target=$(notdir $(CROSS_COMPILE:%-=%))
>  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS    += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> -GCC_TOOLCHAIN  := $(realpath $(GCC_TOOLCHAIN_DIR)/..)
> -endif
> -ifneq ($(GCC_TOOLCHAIN),)
> -CLANG_FLAGS    += --gcc-toolchain=$(GCC_TOOLCHAIN)
>  endif
>  ifneq ($(LLVM_IAS),1)
>  CLANG_FLAGS    += -no-integrated-as
>
> base-commit: 7a7fd0de4a9804299793e564a555a49c1fc924cb
> --
> 2.31.0.rc0.75.gec125d1bc1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210302210646.3044738-1-nathan%40kernel.org.
Masahiro Yamada March 3, 2021, 8:33 a.m. UTC | #4 
Hi.

On Wed, Mar 3, 2021 at 6:44 AM Fangrui Song <maskray@google.com> wrote:
>
> Reviewed-by: Fangrui Song <maskray@google.com>
>
> Thanks for the clean-up!
> --gcc-toolchain= is an obsscure way searching for GCC installation prefixes (--prefix).
> The logic is complex and different for different distributions/architectures.
>
> If we specify --prefix= (-B) explicitly, --gcc-toolchain is not needed.


I tested this, and worked for me too.

Before applying this patch, could you please
help me understand the logic?




I checked the manual
(https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-b-dir)



-B<dir>, --prefix <arg>, --prefix=<arg>
    Add <dir> to search path for binaries and object files used implicitly

--gcc-toolchain=<arg>, -gcc-toolchain <arg>
    Use the gcc toolchain at the given directory


Hmm, this description is too concise
to understand how it works...



I use Ubuntu 20.10.

I use distro's default clang
located in /usr/bin/clang.

I place my aarch64 linaro toolchain in
/home/masahiro/tools/aarch64-linaro-7.5/bin/aarch64-linux-gnu-gcc,
which is not in my PATH environment.




From my some experiments,

clang --target=aarch64-linux-gnu -no-integrated-as \
--prefix=/home/masahiro/tools/aarch64-linaro-7.5/bin/aarch64-linux-gnu-  ...

works almost equivalent to

PATH=/home/masahiro/tools/aarch64-linaro-7.5/bin:$PATH \
clang --target=aarch64-linux-gnu -no-integrated-as ...


Then, clang will pick up aarch64-linux-gnu-as
found in the search path.

Is this correct?


On the other hand, I could not understand
what the purpose of --gcc-toolchain= is.


Even if I add --gcc-toolchain=/home/masahiro/tools/aarch64-linaro-7.5,
it does not make any difference, and it is completely useless.


I read the comment from stephenhines:
https://github.com/ClangBuiltLinux/linux/issues/78

How could --gcc-toolchain be used
in a useful way?









> On 2021-03-02, Nathan Chancellor wrote:
> >This is not necessary anymore now that we specify '--prefix=', which
> >tells clang exactly where to find the GNU cross tools. This has been
> >verified with self compiled LLVM 10.0.1 and LLVM 13.0.0 as well as a
> >distribution version of LLVM 11.1.0 without binutils in the LLVM
> >toolchain locations.
> >
> >Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> >---
> > Makefile | 4 ----
> > 1 file changed, 4 deletions(-)
> >
> >diff --git a/Makefile b/Makefile
> >index f9b54da2fca0..c20f0ad8be73 100644
> >--- a/Makefile
> >+++ b/Makefile
> >@@ -568,10 +568,6 @@ ifneq ($(CROSS_COMPILE),)
> > CLANG_FLAGS   += --target=$(notdir $(CROSS_COMPILE:%-=%))
> > GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> > CLANG_FLAGS   += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> >-GCC_TOOLCHAIN := $(realpath $(GCC_TOOLCHAIN_DIR)/..)
> >-endif
> >-ifneq ($(GCC_TOOLCHAIN),)
> >-CLANG_FLAGS   += --gcc-toolchain=$(GCC_TOOLCHAIN)
> > endif
> > ifneq ($(LLVM_IAS),1)
> > CLANG_FLAGS   += -no-integrated-as
> >
> >base-commit: 7a7fd0de4a9804299793e564a555a49c1fc924cb
> >--
> >2.31.0.rc0.75.gec125d1bc1
> >
> >--
> >You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> >To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> >To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210302210646.3044738-1-nathan%40kernel.org.



--
Best Regards

Masahiro Yamada
Sedat Dilek March 3, 2021, 2:22 p.m. UTC | #5 
On Wed, Mar 3, 2021 at 4:25 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Tue, Mar 2, 2021 at 10:07 PM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > This is not necessary anymore now that we specify '--prefix=', which
> > tells clang exactly where to find the GNU cross tools. This has been
> > verified with self compiled LLVM 10.0.1 and LLVM 13.0.0 as well as a
> > distribution version of LLVM 11.1.0 without binutils in the LLVM
> > toolchain locations.
> >
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>
> [ CC Behan ]
>
> Hahaha, that is a change of a very early commit in times of the
> LLVM/Clang Linux-kernel development.
> So-to-say a historical change :-).
>
> I will try this patchset later with latest Linux -v5.12-rc1+ and my
> custom patchset.
>

I tested these two patches in my build environment.
So far no issues.
NOTE: I have not tested the combo: Clang and GNU AS.

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v13-git

- Sedat -

> > ---
> >  Makefile | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index f9b54da2fca0..c20f0ad8be73 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -568,10 +568,6 @@ ifneq ($(CROSS_COMPILE),)
> >  CLANG_FLAGS    += --target=$(notdir $(CROSS_COMPILE:%-=%))
> >  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> >  CLANG_FLAGS    += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> > -GCC_TOOLCHAIN  := $(realpath $(GCC_TOOLCHAIN_DIR)/..)
> > -endif
> > -ifneq ($(GCC_TOOLCHAIN),)
> > -CLANG_FLAGS    += --gcc-toolchain=$(GCC_TOOLCHAIN)
> >  endif
> >  ifneq ($(LLVM_IAS),1)
> >  CLANG_FLAGS    += -no-integrated-as
> >
> > base-commit: 7a7fd0de4a9804299793e564a555a49c1fc924cb
> > --
> > 2.31.0.rc0.75.gec125d1bc1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210302210646.3044738-1-nathan%40kernel.org.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index f9b54da2fca0..c20f0ad8be73 100644
--- a/Makefile
+++ b/Makefile
@@ -568,10 +568,6 @@  ifneq ($(CROSS_COMPILE),)
 CLANG_FLAGS	+= --target=$(notdir $(CROSS_COMPILE:%-=%))
 GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
 CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
-GCC_TOOLCHAIN	:= $(realpath $(GCC_TOOLCHAIN_DIR)/..)
-endif
-ifneq ($(GCC_TOOLCHAIN),)
-CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
 endif
 ifneq ($(LLVM_IAS),1)
 CLANG_FLAGS	+= -no-integrated-as