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

[Nathan Chancellor]

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 <nat...@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

[Nathan Chancellor]

When building with LLVM_IAS=1, there is no point to specifying
'--prefix=' because that flag is only used to find the cross assembler,
which is clang itself when building with LLVM_IAS=1. All of the other
tools are invoked directly from PATH or a full path specified via the
command line, which does not depend on the value of '--prefix='.

Sharing commands to reproduce issues becomes a little bit easier without
a '--prefix=' value because that '--prefix=' value is specific to a
user's machine due to it being an absolute path.

Signed-off-by: Nathan Chancellor <nat...@kernel.org>
---

Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index c20f0ad8be73..0413b8c594cd 100644
--- a/Makefile
+++ b/Makefile
@@ -566,12 +566,12 @@ CC_VERSION_TEXT = $(shell $(CC) --version 2>/dev/null | head -n 1 | sed 's/\#//g
ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)

ifneq ($(CROSS_COMPILE),)
CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))

+ifneq ($(LLVM_IAS),1)

GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))

-endif
-ifneq ($(LLVM_IAS),1)
CLANG_FLAGS += -no-integrated-as
endif
+endif
CLANG_FLAGS += -Werror=unknown-warning-option
KBUILD_CFLAGS += $(CLANG_FLAGS)
KBUILD_AFLAGS += $(CLANG_FLAGS)
--
2.31.0.rc0.75.gec125d1bc1

[Fangrui Song]

Reviewed-by: Fangrui Song <mas...@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.

>--
>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-li...@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.

[Fangrui Song]

On 2021-03-02, Nathan Chancellor wrote:

>When building with LLVM_IAS=1, there is no point to specifying
>'--prefix=' because that flag is only used to find the cross assembler,
>which is clang itself when building with LLVM_IAS=1. All of the other
>tools are invoked directly from PATH or a full path specified via the
>command line, which does not depend on the value of '--prefix='.
>
>Sharing commands to reproduce issues becomes a little bit easier without
>a '--prefix=' value because that '--prefix=' value is specific to a
>user's machine due to it being an absolute path.
>
>Signed-off-by: Nathan Chancellor <nat...@kernel.org>

Reviewed-by: Fangrui Song <mas...@google.com>

clang can spawn GNU as (if -f?no-integrated-as is specified) and GNU
objcopy (-f?no-integrated-as and -gsplit-dwarf and -g[123]).

With LLVM_IAS=1, these cases are ruled out.

[Nick Desaulniers]

But -g get's set via CONFIG_DEBUG_INFO and -gsplit-dwarf by
DEBUG_INFO_SPLIT. So if we say:
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang LLVM_IAS=1

So cross compile, use clang, use the integrated assembler (ie. with
this change, don't set --prefix), with either of the two above
configs, which objcopy get's exec'd?

>
> With LLVM_IAS=1, these cases are ruled out.

--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

On Tue, Mar 2, 2021 at 1:07 PM Nathan Chancellor <nat...@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 <nat...@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesau...@google.com>
Tested-by: Nick Desaulniers <ndesau...@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
>

--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

Ok, I spoke to Fangrui more offline, and probably misread his
response. From our chat:
```
Fangrui:
objcopy is only used for GNU as assembled object files
With integrated assembler, the object file streamer creates .o and
.dwo simultaneously
With GNU as, two objcopy commands are needed to extract .debug*.dwo to
.dwo files &&& another command to remove .debug*.dwo sections
```

Reviewed-by: Nick Desaulniers <ndesau...@google.com>
Tested-by: Nick Desaulniers <ndesau...@google.com>

I ran this series through a mix of LLVM=1 vs CC=clang, LLVM_IAS=1 vs
unset, CROSS_COMPILE vs not, without issue.

>
> >
> > With LLVM_IAS=1, these cases are ruled out.
>
>
>
> --
> Thanks,
> ~Nick Desaulniers

--
Thanks,
~Nick Desaulniers

[Sedat Dilek]

On Tue, Mar 2, 2021 at 10:07 PM Nathan Chancellor <nat...@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 <nat...@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
>

[Masahiro Yamada]

Hi.

On Wed, Mar 3, 2021 at 6:44 AM Fangrui Song <mas...@google.com> wrote:
>
> Reviewed-by: Fangrui Song <mas...@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?

--
Best Regards

Masahiro Yamada

[Sedat Dilek]

On Wed, Mar 3, 2021 at 4:25 AM Sedat Dilek <sedat...@gmail.com> wrote:
>
> On Tue, Mar 2, 2021 at 10:07 PM Nathan Chancellor <nat...@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 <nat...@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...@gmail.com> # LLVM/Clang v13-git

[Fangrui Song]

--gcc-toolchain was introduced in
https://reviews.llvm.org/rG1af7c219c7113a35415651127f05cdf056b63110
to provide a flexible alternative to autoconf configure-time --with-gcc-toolchain (now cmake variable GCC_INSTALL_PREFIX).

I agree the option is confusing, the documentation is poor, and probably very few people understand what it does.
I apologize that my previous reply is not particular correct.
So the more correct answer is below:


A --prefix option can specify either of

1) A directory (for something like /a/b/lib/gcc/arm-linux-androideabi, this should be /a/b, the parent directory of 'lib')
2) A path fragment like /usr/bin/aarch64-linux-gnu-

The directory values of the --prefix list and --gcc-toolchain are used to detect GCC installation directories. The directory is used to fetch include directories, system library directories and binutils directories (as, objcopy).
(See below, Linux kernel only needs the binutils executables, so the include/library logic is really useless to us)

The logic is around https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Gnu.cpp#L1910

Prefixes = --prefix/-B list (only the directory subset is effective)
StringRef GCCToolchainDir = --gcc-toolchain= or CMake variable GCC_INSTALL_PREFIX
if (GCCToolchainDir != "") {
Prefixes.push_back(std::string(GCCToolchainDir));
} else {
if (!D.SysRoot.empty()) {
Prefixes.push_back(D.SysRoot);
// Add D.SysRoot+"/usr" to Prefixes. Some distributions add more directories.
AddDefaultGCCPrefixes(TargetTriple, Prefixes, D.SysRoot);
}

// D.InstalledDir is the directory of the clang executable, e.g. /usr/bin
Prefixes.push_back(D.InstalledDir + "/..");

if (D.SysRoot.empty())
AddDefaultGCCPrefixes(TargetTriple, Prefixes, D.SysRoot);
}

// Gentoo / ChromeOS specific logic.
// I think this block is misplaced.
if (GCCToolchainDir == "" || GCCToolchainDir == D.SysRoot + "/usr") {
...
}

// Loop over the various components which exist and select the best GCC
// installation available. GCC installs are ranked by version number.
Version = GCCVersion::Parse("0.0.0");
for (const std::string &Prefix : Prefixes) {
auto &VFS = D.getVFS();
if (!VFS.exists(Prefix))
continue;

// CandidateLibDirs is a subset of {/lib64, /lib32, /lib}.
for (StringRef Suffix : CandidateLibDirs) {
const std::string LibDir = Prefix + Suffix.str();
if (!VFS.exists(LibDir))
continue;
bool GCCDirExists = VFS.exists(LibDir + "/gcc");
bool GCCCrossDirExists = VFS.exists(LibDir + "/gcc-cross");

// Precise match. Detect $Prefix/lib/$--target
ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, TargetTriple.str(),
false, GCCDirExists, GCCCrossDirExists);
// Usually empty.
for (StringRef Candidate : ExtraTripleAliases) // Try these first.
ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, Candidate, false,
GCCDirExists, GCCCrossDirExists);
// CandidateTripleAliases is a set with "x86_64-linux-gnu", "x86_64-unknown-linux-gnu", ...
// This loop detects directories like $Prefix/lib/x86_64-linux-gnu.
for (StringRef Candidate : CandidateTripleAliases)
ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, Candidate, false,
GCCDirExists, GCCCrossDirExists);
}
for (StringRef Suffix : CandidateBiarchLibDirs) {
const std::string LibDir = Prefix + Suffix.str();
if (!VFS.exists(LibDir))
continue;
bool GCCDirExists = VFS.exists(LibDir + "/gcc");
bool GCCCrossDirExists = VFS.exists(LibDir + "/gcc-cross");
for (StringRef Candidate : CandidateBiarchTripleAliases)
ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, Candidate, true,
GCCDirExists, GCCCrossDirExists);
}
}


The comment
// Loop over the various components which exist and select the best GCC
// installation available. GCC installs are ranked by version number.

is important. If you specify --prefix=$dir but not --gcc-toolchain,
the system cross toolchains (/usr/lib/gcc-cross) are also candidates and they may win.

Specifying just --gcc-toolchain (due to if (GCCToolchainDir != "")) can effectively ignore system cross toolchains.



In the Linux kernel use case, We specify -nostdinc and -nostdlib so GCC include/library directories are not used.
We seem to prefer the non-directory use of --prefix: CROSS_COMPILE=arm-linux-gnueabi-
So all the directory detection logic can be dropped.


A better commit message is along the lines of:
--gcc-toolchain specified directory is used to detect GCC installations
for include/library directories and binutils executables.

We already specify something like --prefix=aarch64-linux-gnu- to inform
Clang of the binutils executables, and we do not need include/library
directories, so we can drop --gcc-toolchain.

>
>
>
>
>
>
>
>> 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 <nat...@kernel.org>

>--
>Best Regards
>
>Masahiro Yamada
>

>--
>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-li...@googlegroups.com.

>To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAK7LNAS4Ri%3DK6M39hYU%2B17JVf0Z%3DhbRgSxuTdX5ZaVYLpmJRtA%40mail.gmail.com.

[Fāng-ruì Sòng]

I've sent https://reviews.llvm.org/D97902 to improve the -B and
--gcc-toolchain documentation, and posted
https://lists.llvm.org/pipermail/cfe-dev/2021-March/067820.html
(Message-Id: CAFP8O3JZBWx6OYm14KhP0f+R...@mail.gmail.com)
to discuss Clang's current weird behaviors.

* --gcc-toolchain suppresses GCC detection under sysroot, -B doesn't.
I'd like that -B suppresses GCC detection under sysroot as well but
unsure whether some folks are dependent on this behavior.
* With -B -B -B --gcc-toolchain, the largest version instead of the
first wins. I think it makes sense to select the first option where a
GCC installation is found.

Hey, this list has a lot developers using cross compilers. Do you have
thoughts to share? :)

[Masahiro Yamada]

Thanks for all the detailed info.
I cannot say I understood them all,
but this information is worth knowing.
(I didn't even know --prefix can take a directory).

I applied these patches.
Thanks.

[Masahiro Yamada]

On Wed, Mar 3, 2021 at 6:07 AM Nathan Chancellor <nat...@kernel.org> wrote:
>
> When building with LLVM_IAS=1, there is no point to specifying
> '--prefix=' because that flag is only used to find the cross assembler,
> which is clang itself when building with LLVM_IAS=1. All of the other
> tools are invoked directly from PATH or a full path specified via the
> command line, which does not depend on the value of '--prefix='.
>
> Sharing commands to reproduce issues becomes a little bit easier without
> a '--prefix=' value because that '--prefix=' value is specific to a
> user's machine due to it being an absolute path.
>
> Signed-off-by: Nathan Chancellor <nat...@kernel.org>

I was tricked by a couple of Reviewed-by/Tested-by tags.

With this patch applied, the code looks as follows:

ifneq ($(CROSS_COMPILE),)
CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))

ifneq ($(LLVM_IAS),1)
GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))

CLANG_FLAGS += -no-integrated-as
endif
endif


For the native build (empty CROSS_COMPILE),
you cannot add -no-integrated-as.


I dropped this from my tree.



Is the correct code as follows?

ifneq ($(LLVM_IAS),1)
CLANG_FLAGS += -no-integrated-as

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

endif
endif

> ---
> Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c20f0ad8be73..0413b8c594cd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -566,12 +566,12 @@ CC_VERSION_TEXT = $(shell $(CC) --version 2>/dev/null | head -n 1 | sed 's/\#//g
> ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
> ifneq ($(CROSS_COMPILE),)
> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> +ifneq ($(LLVM_IAS),1)
> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> -endif
> -ifneq ($(LLVM_IAS),1)
> CLANG_FLAGS += -no-integrated-as
> endif
> +endif
> CLANG_FLAGS += -Werror=unknown-warning-option
> KBUILD_CFLAGS += $(CLANG_FLAGS)
> KBUILD_AFLAGS += $(CLANG_FLAGS)
> --
> 2.31.0.rc0.75.gec125d1bc1
>

[Masahiro Yamada]

If you send v2, can you include comments from Fangrui Song?



Fangrui Song:

clang can spawn GNU as (if -f?no-integrated-as is specified) and GNU
objcopy (-f?no-integrated-as and -gsplit-dwarf and -g[123]).

objcopy is only used for GNU as assembled object files.

With integrated assembler, the object file streamer creates .o and

.dwo simultaneously.

With GNU as, two objcopy commands are needed to extract .debug*.dwo to

.dwo files && another command to remove .debug*.dwo sections.



I did not know the objtool part, and I think it is worth recording.

[Nathan Chancellor]

This flag was originally added to allow clang to find the GNU cross
tools in commit 785f11aa595b ("kbuild: Add better clang cross build
support"). This flag was not enough to find the tools at times so
'--prefix' was added to the list in commit ef8c4ed9db80 ("kbuild: allow
to use GCC toolchain not in Clang search path") and improved upon in
commit ca9b31f6bb9c ("Makefile: Fix GCC_TOOLCHAIN_DIR prefix for Clang
cross compilation"). Now that '--prefix' specifies a full path and
prefix, '--gcc-toolchain' serves no purpose because the kernel builds
with '-nostdinc' and '-nostdlib'.

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.

Link: https://reviews.llvm.org/D97902

Signed-off-by: Nathan Chancellor <nat...@kernel.org>
---

v1 -> v2:

* Improve commit message (add history behind flag and link to Fangrui's
documentation improvement).

I did not carry tags forward so that people could re-review and test.

Makefile | 4 ----
1 file changed, 4 deletions(-)

diff --git a/Makefile b/Makefile

index 31dcdb3d61fa..182e93d91198 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: a38fd8748464831584a19438cbb3082b5a2dab15
--
2.31.0.rc1

[Nathan Chancellor]

When building with LLVM_IAS=1, there is no point to specifying

'--prefix=' because that flag is only used to find GNU cross tools,
which will not be used indirectly when using the integrated assembler.
All of the tools are invoked directly from PATH or a full path specified

via the command line, which does not depend on the value of '--prefix='.

Sharing commands to reproduce issues becomes a little bit easier without
a '--prefix=' value because that '--prefix=' value is specific to a
user's machine due to it being an absolute path.

Some further notes from Fangrui Song:

clang can spawn GNU as (if -f?no-integrated-as is specified) and GNU
objcopy (-f?no-integrated-as and -gsplit-dwarf and -g[123]).
objcopy is only used for GNU as assembled object files.
With integrated assembler, the object file streamer creates .o and
.dwo simultaneously.
With GNU as, two objcopy commands are needed to extract .debug*.dwo to
.dwo files && another command to remove .debug*.dwo sections.

A small consequence of this change (to keep things simple) is that
'--prefix=' will always be specified now, even with a native build, when
it was not before. This should not be an issue due to the way that the
Makefile searches for the prefix (based on elfedit's location). This
ends up improving the experience for host builds because PATH is better
respected and matches GCC's behavior more closely. See the below thread
for more details:

https://lore.kernel.org/r/20210205213...@Ryzen-5-4500U.localdomain/

Signed-off-by: Nathan Chancellor <nat...@kernel.org>
---

v1 -> v2:

* Fix position of -no-integrated-as flag to fix native build (thanks to
Masahiro for catching it and sorry for the breakage).

* Add Fangrui's comments about what GNU binaries clang can spawn at
Masahiro's request.

* Reword commit message.

I did not carry tags forward so that people could re-review and test.

Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 182e93d91198..15292a6d63f5 100644
--- a/Makefile
+++ b/Makefile
@@ -566,11 +566,11 @@ CC_VERSION_TEXT = $(shell $(CC) --version 2>/dev/null | head -n 1 | sed 's/\#//g
ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)

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

endif
ifneq ($(LLVM_IAS),1)
CLANG_FLAGS += -no-integrated-as

+GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
+CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))

endif
CLANG_FLAGS += -Werror=unknown-warning-option
KBUILD_CFLAGS += $(CLANG_FLAGS)

--
2.31.0.rc1

[Fangrui Song]

On 2021-03-09, Nathan Chancellor wrote:
>This flag was originally added to allow clang to find the GNU cross
>tools in commit 785f11aa595b ("kbuild: Add better clang cross build
>support"). This flag was not enough to find the tools at times so
>'--prefix' was added to the list in commit ef8c4ed9db80 ("kbuild: allow
>to use GCC toolchain not in Clang search path") and improved upon in
>commit ca9b31f6bb9c ("Makefile: Fix GCC_TOOLCHAIN_DIR prefix for Clang
>cross compilation"). Now that '--prefix' specifies a full path and
>prefix, '--gcc-toolchain' serves no purpose because the kernel builds
>with '-nostdinc' and '-nostdlib'.
>
>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.
>
>Link: https://reviews.llvm.org/D97902
>Signed-off-by: Nathan Chancellor <nat...@kernel.org>

The wording looks good.

Reviewed-by: Fangrui Song <mas...@google.com>

[Masahiro Yamada]

On Wed, Mar 10, 2021 at 5:59 AM Nathan Chancellor <nat...@kernel.org> wrote:
>
> This flag was originally added to allow clang to find the GNU cross
> tools in commit 785f11aa595b ("kbuild: Add better clang cross build
> support"). This flag was not enough to find the tools at times so
> '--prefix' was added to the list in commit ef8c4ed9db80 ("kbuild: allow
> to use GCC toolchain not in Clang search path") and improved upon in
> commit ca9b31f6bb9c ("Makefile: Fix GCC_TOOLCHAIN_DIR prefix for Clang
> cross compilation"). Now that '--prefix' specifies a full path and
> prefix, '--gcc-toolchain' serves no purpose because the kernel builds
> with '-nostdinc' and '-nostdlib'.
>
> 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.
>
> Link: https://reviews.llvm.org/D97902
> Signed-off-by: Nathan Chancellor <nat...@kernel.org>
> ---
>
> v1 -> v2:
>
> * Improve commit message (add history behind flag and link to Fangrui's
> documentation improvement).

Both applied to linux-kbuild. Thanks.

> I did not carry tags forward so that people could re-review and test.
>
> Makefile | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 31dcdb3d61fa..182e93d91198 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: a38fd8748464831584a19438cbb3082b5a2dab15
> --
> 2.31.0.rc1
>

[Sedat Dilek]

On Mon, Mar 15, 2021 at 5:22 PM Masahiro Yamada <masa...@kernel.org> wrote:
>
> On Wed, Mar 10, 2021 at 5:59 AM Nathan Chancellor <nat...@kernel.org> wrote:
> >
> > This flag was originally added to allow clang to find the GNU cross
> > tools in commit 785f11aa595b ("kbuild: Add better clang cross build
> > support"). This flag was not enough to find the tools at times so
> > '--prefix' was added to the list in commit ef8c4ed9db80 ("kbuild: allow
> > to use GCC toolchain not in Clang search path") and improved upon in
> > commit ca9b31f6bb9c ("Makefile: Fix GCC_TOOLCHAIN_DIR prefix for Clang
> > cross compilation"). Now that '--prefix' specifies a full path and
> > prefix, '--gcc-toolchain' serves no purpose because the kernel builds
> > with '-nostdinc' and '-nostdlib'.
> >
> > 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.
> >
> > Link: https://reviews.llvm.org/D97902
> > Signed-off-by: Nathan Chancellor <nat...@kernel.org>
> > ---
> >
> > v1 -> v2:
> >
> > * Improve commit message (add history behind flag and link to Fangrui's
> > documentation improvement).
>
>
> Both applied to linux-kbuild. Thanks.
>

Sorry for being pedantic: This misses my Tested-by#s (see [1]).

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

AFAICS v2 changes some comments in the commit only but not code?

- Sedat -

[1] https://marc.info/?l=linux-kernel&m=161480031518629&w=2
[2] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/commit/?h=kbuild&id=d4aa405bc9cd506532f075456645716cdd1739c1

[Masahiro Yamada]

For 1/2, yes. The difference is only comments.


For 2/2, the code was changed.
Actually, v1 was wrong.
That is why the tags were dropped.


I will re-add the tags to 1/2
since there is no code diff.


If you re-test 2/2, I will add your tag again.

[kernel test robot]

Hi Nathan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on 7a7fd0de4a9804299793e564a555a49c1fc924cb]

url: https://github.com/0day-ci/linux/commits/Nathan-Chancellor/Makefile-Remove-gcc-toolchain-flag/20210929-112213
base: 7a7fd0de4a9804299793e564a555a49c1fc924cb
config: x86_64-randconfig-a014-20210929 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project dc6e8dfdfe7efecfda318d43a06fae18b40eb498)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/74a168f9e88d9edf5aaa8209d2c39e676de2857a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nathan-Chancellor/Makefile-Remove-gcc-toolchain-flag/20210929-112213
git checkout 74a168f9e88d9edf5aaa8209d2c39e676de2857a
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/entry/ arch/x86/kernel/ arch/x86/lib/ kernel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <l...@intel.com>

All warnings (new ones prefixed by >>):

>> arch/x86/entry/entry_64.S:47:1: warning: DWARF2 only supports one section per compilation unit
.section .entry.text, "ax"
^
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
.pushsection .altinstr_replacement,"ax"
^
<instantiation>:1:1: note: while in macro instantiation
ALTERNATIVE "jmp .Lend_2", "", ( 7*32+11)
^
arch/x86/entry/entry_64.S:93:2: note: while in macro instantiation
SWITCH_TO_KERNEL_CR3 scratch_reg=%rsp
^
arch/x86/entry/entry_64.S:741:2: warning: DWARF2 only supports one section per compilation unit
.section .fixup, "ax"
^
--
>> arch/x86/entry/entry_64_compat.S:20:2: warning: DWARF2 only supports one section per compilation unit
.section .entry.text, "ax"
^
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
.pushsection .altinstr_replacement,"ax"
^
<instantiation>:1:1: note: while in macro instantiation
ALTERNATIVE "jmp .Lend_2", "", ( 7*32+11)
^
arch/x86/entry/entry_64_compat.S:55:2: note: while in macro instantiation
SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
^
--
>> arch/x86/kernel/head_64.S:42:2: warning: DWARF2 only supports one section per compilation unit
.section ".head.text","ax"
^
arch/x86/kernel/head_64.S:352:2: warning: DWARF2 only supports one section per compilation unit
.section ".init.text","ax"
^
--
>> arch/x86/lib/copy_mc_64.S:138:2: warning: DWARF2 only supports one section per compilation unit
.section .fixup, "ax"
^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
.pushsection .altinstr_replacement,"ax"
^
arch/x86/lib/copy_page_64.S:17:2: note: while in macro instantiation
ALTERNATIVE "jmp copy_page_regs", "", ( 3*32+16)
^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
.pushsection .altinstr_replacement,"ax"
^
arch/x86/lib/copy_user_64.S:58:2: note: while in macro instantiation
ALTERNATIVE "", ".byte 0x0f,0x01,0xcb", ( 9*32+20)
^
<instantiation>:15:2: warning: DWARF2 only supports one section per compilation unit
.section .fixup,"ax"
^
arch/x86/lib/copy_user_64.S:61:2: note: while in macro instantiation
ALIGN_DESTINATION
^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
.pushsection .altinstr_replacement,"ax"
^
arch/x86/lib/getuser.S:56:2: note: while in macro instantiation
ALTERNATIVE "", ".byte 0x0f,0x01,0xcb", ( 9*32+20)
^
--
>> arch/x86/lib/memcpy_64.S:10:1: warning: DWARF2 only supports one section per compilation unit
.pushsection .noinstr.text, "ax"
^
<instantiation>:13:2: warning: DWARF2 only supports one section per compilation unit
.pushsection .altinstr_replacement,"ax"
^
arch/x86/lib/memcpy_64.S:32:2: note: while in macro instantiation
ALTERNATIVE_2 "jmp memcpy_orig", "", ( 3*32+16), "jmp memcpy_erms", ( 9*32+ 9)
^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
.pushsection .altinstr_replacement,"ax"
^
arch/x86/lib/memmove_64.S:42:2: note: while in macro instantiation
ALTERNATIVE "cmp $0x20, %rdx; jb 1f", "", (18*32+ 4)
^
--
>> <instantiation>:11:2: warning: DWARF2 only supports one section per compilation unit
.pushsection .altinstr_replacement,"ax"
^
arch/x86/lib/putuser.S:51:2: note: while in macro instantiation
ALTERNATIVE "", ".byte 0x0f,0x01,0xcb", ( 9*32+20)
^


vim +47 arch/x86/entry/entry_64.S

6fd166aae78c0a arch/x86/entry/entry_64.S Peter Zijlstra 2017-12-04 45
^1da177e4c3f41 arch/x86_64/kernel/entry.S Linus Torvalds 2005-04-16 46 .code64
ea7145477a461e arch/x86/kernel/entry_64.S Jiri Olsa 2011-03-07 @47 .section .entry.text, "ax"
ea7145477a461e arch/x86/kernel/entry_64.S Jiri Olsa 2011-03-07 48

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuil...@lists.01.org

[kernel test robot]

Hi Nathan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on 7a7fd0de4a9804299793e564a555a49c1fc924cb]

url: https://github.com/0day-ci/linux/commits/Nathan-Chancellor/Makefile-Remove-gcc-toolchain-flag/20210929-112213
base: 7a7fd0de4a9804299793e564a555a49c1fc924cb

config: x86_64-randconfig-a011-20210929 (attached as .config)

compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project dc6e8dfdfe7efecfda318d43a06fae18b40eb498)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/74a168f9e88d9edf5aaa8209d2c39e676de2857a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nathan-Chancellor/Makefile-Remove-gcc-toolchain-flag/20210929-112213
git checkout 74a168f9e88d9edf5aaa8209d2c39e676de2857a
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <l...@intel.com>

All warnings (new ones prefixed by >>):

>> arch/x86/platform/pvh/head.S:22:2: warning: DWARF2 only supports one section per compilation unit

.section ".head.text","ax"
^

vim +22 arch/x86/platform/pvh/head.S

7243b93345f7f8d arch/x86/xen/xen-pvh.S Boris Ostrovsky 2017-02-05 21
7243b93345f7f8d arch/x86/xen/xen-pvh.S Boris Ostrovsky 2017-02-05 @22 __HEAD
7243b93345f7f8d arch/x86/xen/xen-pvh.S Boris Ostrovsky 2017-02-05 23