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

From: Fangrui Song
Date: Wed Mar 03 2021 - 19:11:35 EST

Next message: kernel test robot: "s390-linux-ld: timer-of.c:undefined reference to `iounmap'"
Previous message: Andrew Morton: "Re: [PATCH v4 0/3] Cleanup and fixups for vmemmap handling"
In reply to: Sedat Dilek: "Re: [PATCH 1/2] Makefile: Remove '--gcc-toolchain' flag"
Next in thread: Fāng-ruì Sòng: "Re: [PATCH 1/2] Makefile: Remove '--gcc-toolchain' flag"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2021-03-03, Masahiro Yamada wrote:

Hi.

On Wed, Mar 3, 2021 at 6:44 AM Fangrui Song <maskray@xxxxxxxxxx> wrote:

Reviewed-by: Fangrui Song <maskray@xxxxxxxxxx>

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?

--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 <nathan@xxxxxxxxxx>
>---
> 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@xxxxxxxxxxxxxxxx.
>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

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

Next message: kernel test robot: "s390-linux-ld: timer-of.c:undefined reference to `iounmap'"
Previous message: Andrew Morton: "Re: [PATCH v4 0/3] Cleanup and fixups for vmemmap handling"
In reply to: Sedat Dilek: "Re: [PATCH 1/2] Makefile: Remove '--gcc-toolchain' flag"
Next in thread: Fāng-ruì Sòng: "Re: [PATCH 1/2] Makefile: Remove '--gcc-toolchain' flag"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]