[RFC 1/2] Documentation/llvm: add documentation on building w/ Clang/LLVM

[Nick Desaulniers]

Added to kbuild documentation. Provides more official info on building
kernels with Clang and LLVM than our wiki.

Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---
Documentation/kbuild/index.rst | 1 +
Documentation/kbuild/llvm.rst | 59 ++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
create mode 100644 Documentation/kbuild/llvm.rst

diff --git a/Documentation/kbuild/index.rst b/Documentation/kbuild/index.rst
index 0f144fad99a6..3882bd5f7728 100644
--- a/Documentation/kbuild/index.rst
+++ b/Documentation/kbuild/index.rst
@@ -19,6 +19,7 @@ Kernel Build System

issues
reproducible-builds
+ llvm

.. only:: subproject and html

diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
new file mode 100644
index 000000000000..05c9a28a5a6d
--- /dev/null
+++ b/Documentation/kbuild/llvm.rst
@@ -0,0 +1,59 @@
+==============================
+Building Linux with Clang/LLVM
+==============================
+
+This document covers how to build the Linux kernel with Clang and LLVM
+utilities.
+
+About
+-----
+
+The Linux kernel has always traditionally been compiled with GNU toolchains
+such as GCC and binutils. On going work has allowed for `Clang
+<https://clang.llvm.org/>`_ and `LLVM <https://llvm.org/>`_ utilities to be
+used as viable substitutes. Distributions such as Android, ChromeOS, and
+OpenMandriva use Clang built kernels. `LLVM is a collection of toolchain
+components implemented in terms of C++ objects
+<https://www.aosabook.org/en/llvm.html>`_. Clang is a front-end to LLVM that
+supports C and the GNU C extensions required by the kernel, and is pronounced
+"klang," not "see-lang."
+
+Clang
+-----
+
+The compiler used can be swapped out via `CC=` command line argument to `make`.
+`CC=` should be set when selecting a config and during a build.
+
+ make CC=clang defconfig
+
+ make CC=clang
+
+Cross Compiling
+---------------
+
+A single Clang binary will typically contain all supported backends, which can
+help simplify cross compiling.
+
+ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang
+
+LLVM Utilities
+--------------
+
+LLVM has substitutes for GNU binutils utilities. These can be invoked as
+additional parameters to `make`.
+
+ make CC=clang AS=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \\
+ OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-objsize \\
+ READELF=llvm-readelf HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar \\
+ HOSTLD=ld.lld
+
+Getting Help
+------------
+
+- `Website <https://clangbuiltlinux.github.io/>`_
+- `Mailing List <https://groups.google.com/forum/#!forum/clang-built-linux>`_: <clang-bu...@googlegroups.com>
+- `Issue Tracker <https://github.com/ClangBuiltLinux/linux/issues>`_
+- IRC: #clangbuiltlinux on chat.freenode.net
+- `Telegram <https://t.me/ClangBuiltLinux>`_: @ClangBuiltLinux
+- `Wiki <https://github.com/ClangBuiltLinux/linux/wiki>`_
+- `Beginner Bugs <https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22>`_
--
2.25.0.265.gbab2e86ba0-goog

[Nick Desaulniers]

Prior to this patch, building the Linux kernel with Clang
looked like:

$ make CC=clang

or when cross compiling:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang

which got very verbose and unwieldy when using all of LLVM's substitutes
for GNU binutils:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang AS=clang \
LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-objsize \
READELF=llvm-readelf HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar \
HOSTLD=ld.lld

This change adds a new Makefile under scripts/ which will be included in
the top level Makefile AFTER CC and friends are set, in order to make
the use of LLVM utilities when building a Linux kernel more ergonomic.

With this patch, the above now looks like:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1

Then you can "opt out" of certain LLVM utilities explicitly:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 AS=as

will instead invoke arm-linux-gnueabihf-as in place of clang for AS.

This would make it more verbose to opt into just one tool from LLVM, but
this patch doesn't actually break the old style; just leave off LLVM=1.
Also, LLVM=1 CC=clang would wind up prefixing clang with $CROSS_COMPILE.
In that case, it's recommended to just drop LLVM=1 and use the old
style. So LLVM=1 can be thought of as default to LLVM with explicit opt
ins for GNU, vs the current case of default to GNU and opt in for LLVM.

A key part of the design of this patch is to be minimally invasive to
the top level Makefile and not break existing workflows. We could get
more aggressive, but I'd prefer to save larger refactorings for another
day.

About the script:
The pattern used in the script is in the form:

ifeq "$(origin $(CC))" "file"
$(CC) := $(clang)
else
override $(CC) := $(CROSS_COMPILE)$(CC)
endif

"Metaprogramming" (eval) is used to template the above to make it more
concise for specifying all of the substitutions.

The "origin" of a variable tracks whether a variable was explicitly set
via "command line", "environment", was defined earlier via Makefile
"file", was provided by "default", or was "undefined".

Variable assignment in GNU Make has some special and complicated rules.

If the variable was set earlier explicitly in the Makefile, we can
simply reassign a new value to it. If a variable was unspecified, then
earlier assignments were executed and change the origin to file.
Otherwise, the variable was explicitly specified.

If a variable's "origin" was "command line" or "environment",
non-"override" assignments are not executed. The "override" directive
forces the assignment regardless of "origin".

Some tips I found useful for debugging for future travelers:

$(info $$origin of $$CC is $(origin CC))

at the start of the new script for all of the variables can help you
understand "default" vs "undefined" variable origins.

$(info $$CC is [${CC}])

in the top level Makefile after including the new script, for all of the
variables can help you check that they're being set as expected.

Link: https://www.gnu.org/software/make/manual/html_node/Eval-Function.html
Link: https://www.gnu.org/software/make/manual/html_node/Origin-Function.html
Link: https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
Link: https://www.gnu.org/software/make/manual/html_node/Override-Directive.html

Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---

Makefile | 4 ++++
scripts/Makefile.llvm | 24 ++++++++++++++++++++++++
2 files changed, 28 insertions(+)
create mode 100644 scripts/Makefile.llvm

diff --git a/Makefile b/Makefile
index b954c304c479..8c8888ebb822 100644
--- a/Makefile
+++ b/Makefile
@@ -472,6 +472,10 @@ KBUILD_LDFLAGS :=
GCC_PLUGINS_CFLAGS :=
CLANG_FLAGS :=

+ifeq ($(LLVM),1)
+include scripts/Makefile.llvm
+endif
+
export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE AS LD CC
export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
diff --git a/scripts/Makefile.llvm b/scripts/Makefile.llvm
new file mode 100644
index 000000000000..a4401c8677dc
--- /dev/null
+++ b/scripts/Makefile.llvm
@@ -0,0 +1,24 @@
+define META_set =
+ifeq "$(origin $(1))" "file"
+$(1) := $(2)
+else
+override $(1) := $(CROSS_COMPILE)$($(1))
+endif
+endef
+
+$(eval $(call META_set,CC,clang))
+$(eval $(call META_set,AS,clang))
+$(eval $(call META_set,LD,ld.lld))
+$(eval $(call META_set,AR,llvm-ar))
+$(eval $(call META_set,NM,llvm-nm))
+$(eval $(call META_set,STRIP,llvm-strip))
+$(eval $(call META_set,OBJCOPY,llvm-objcopy))
+$(eval $(call META_set,OBJDUMP,llvm-objdump))
+$(eval $(call META_set,OBJSIZE,llvm-objsize))
+$(eval $(call META_set,READELF,llvm-readelf))
+$(eval $(call META_set,HOSTCC,clang))
+$(eval $(call META_set,HOSTCXX,clang++))
+$(eval $(call META_set,HOSTAR,llvm-ar))
+$(eval $(call META_set,HOSTLD,ld.lld))
+
+## TODO: HOSTAR, HOSTLD in tools/objtool/Makefile
--
2.25.0.265.gbab2e86ba0-goog

[Nick Desaulniers]

+ Alistair, Sandeep

--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

+ Masahiro, Matthias

--
Thanks,
~Nick Desaulniers

[Kees Cook]

For clarity for people who have only ever seen GCC, I would add
"compiler":

"A single Clang compiler binary ..."

> +help simplify cross compiling.
> +
> + ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang

Maybe note that CROSS_COMPILE, when using Clang, gets parse for the
architecture triple, and doesn't get used (as with GCC) as an executable
prefix for locating the appropriate compiler binary...

> +
> +LLVM Utilities
> +--------------
> +
> +LLVM has substitutes for GNU binutils utilities. These can be invoked as
> +additional parameters to `make`.
> +
> + make CC=clang AS=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \\
> + OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-objsize \\
> + READELF=llvm-readelf HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar \\
> + HOSTLD=ld.lld

Do you want to add a section for "building latest Clang" here, or is
that just asking for needing constant updates to this doc?

Otherwise, yes, looks great!

-Kees

> +
> +Getting Help
> +------------
> +
> +- `Website <https://clangbuiltlinux.github.io/>`_
> +- `Mailing List <https://groups.google.com/forum/#!forum/clang-built-linux>`_: <clang-bu...@googlegroups.com>
> +- `Issue Tracker <https://github.com/ClangBuiltLinux/linux/issues>`_
> +- IRC: #clangbuiltlinux on chat.freenode.net
> +- `Telegram <https://t.me/ClangBuiltLinux>`_: @ClangBuiltLinux
> +- `Wiki <https://github.com/ClangBuiltLinux/linux/wiki>`_
> +- `Beginner Bugs <https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22>`_
> --
> 2.25.0.265.gbab2e86ba0-goog
>

> --
> 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/20200220234858.145546-1-ndesaulniers%40google.com.

--
Kees Cook

[Kees Cook]

On Thu, Feb 20, 2020 at 03:48:58PM -0800, 'Nick Desaulniers' via Clang Built Linux wrote:
> Prior to this patch, building the Linux kernel with Clang
> looked like:
>
> $ make CC=clang
>
> or when cross compiling:
>
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
>
> which got very verbose and unwieldy when using all of LLVM's substitutes
> for GNU binutils:
>
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang AS=clang \
> LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
> OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-objsize \
> READELF=llvm-readelf HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar \
> HOSTLD=ld.lld
>
> This change adds a new Makefile under scripts/ which will be included in
> the top level Makefile AFTER CC and friends are set, in order to make
> the use of LLVM utilities when building a Linux kernel more ergonomic.
>
> With this patch, the above now looks like:
>
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1

I like this idea, but I think it might not be happening in the right
place... but I'm not sure what that place is. This will collide, for
example, with the Clang LTO series from Sami, as that _requires_ the
LLVM utilities in many places. I think some coordination with Kconfig
will be needed to not make this kind of crazy.

e.g. Sami currently has to hard-code this logic:

https://github.com/samitolvanen/linux/commit/42572f50c5f951cac7ea4720658d9abd2b995529#diff-b67911656ef5d18c4ae36cb6741b7965R652

-Kees

> --
> 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/20200220234858.145546-2-ndesaulniers%40google.com.

--
Kees Cook

[Nathan Chancellor]

Mentioning tc-build might be nice since I have been active in keeping
that up to date and it tries to make building a new version of clang as
brainless as possible since most people don't want to muck around with
build options and such. However, it might be best to just link to the
LLVM building documentation that way it always stays up to date.

Or just do both?

> Otherwise, yes, looks great!
>
> -Kees
>
> > +
> > +Getting Help
> > +------------
> > +
> > +- `Website <https://clangbuiltlinux.github.io/>`_
> > +- `Mailing List <https://groups.google.com/forum/#!forum/clang-built-linux>`_: <clang-bu...@googlegroups.com>
> > +- `Issue Tracker <https://github.com/ClangBuiltLinux/linux/issues>`_
> > +- IRC: #clangbuiltlinux on chat.freenode.net
> > +- `Telegram <https://t.me/ClangBuiltLinux>`_: @ClangBuiltLinux
> > +- `Wiki <https://github.com/ClangBuiltLinux/linux/wiki>`_
> > +- `Beginner Bugs <https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22>`_
> > --
> > 2.25.0.265.gbab2e86ba0-goog

Otherwise, I think the document looks good. I should probably try to
build it at some point to make sure everything passes and looks good.

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

[Nick Desaulniers]

Added to kbuild documentation. Provides more official info on building
kernels with Clang and LLVM than our wiki.

Suggested-by: Kees Cook <kees...@chromium.org>
Reviewed-by: Nathan Chancellor <natecha...@gmail.com>

Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---
Documentation/kbuild/index.rst | 1 +

Documentation/kbuild/llvm.rst | 80 ++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+)

create mode 100644 Documentation/kbuild/llvm.rst

diff --git a/Documentation/kbuild/index.rst b/Documentation/kbuild/index.rst
index 0f144fad99a6..3882bd5f7728 100644
--- a/Documentation/kbuild/index.rst
+++ b/Documentation/kbuild/index.rst
@@ -19,6 +19,7 @@ Kernel Build System

issues
reproducible-builds
+ llvm

.. only:: subproject and html

diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
new file mode 100644

index 000000000000..68ae022aebc0
--- /dev/null
+++ b/Documentation/kbuild/llvm.rst
@@ -0,0 +1,80 @@

+==============================
+Building Linux with Clang/LLVM
+==============================
+
+This document covers how to build the Linux kernel with Clang and LLVM
+utilities.
+
+About
+-----
+
+The Linux kernel has always traditionally been compiled with GNU toolchains
+such as GCC and binutils. On going work has allowed for `Clang
+<https://clang.llvm.org/>`_ and `LLVM <https://llvm.org/>`_ utilities to be

+used as viable substitutes. Distributions such as `Android
+<https://www.android.com/>`_, `ChromeOS
+<https://www.chromium.org/chromium-os>`_, and `OpenMandriva
+<https://www.openmandriva.org/>`_ use Clang built kernels. `LLVM is a
+collection of toolchain components implemented in terms of C++ objects

+<https://www.aosabook.org/en/llvm.html>`_. Clang is a front-end to LLVM that
+supports C and the GNU C extensions required by the kernel, and is pronounced
+"klang," not "see-lang."
+
+Clang
+-----
+
+The compiler used can be swapped out via `CC=` command line argument to `make`.
+`CC=` should be set when selecting a config and during a build.
+
+ make CC=clang defconfig
+
+ make CC=clang
+
+Cross Compiling
+---------------
+

+A single Clang compiler binary will typically contain all supported backends,
+which can help simplify cross compiling.

+
+ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang

+
+`CROSS_COMPILE` is not used to suffix the Clang compiler binary, instead
+`CROSS_COMPILE` is used to set a command line flag: `--target <triple>`. For
+example:
+
+ clang --target aarch64-linux-gnu foo.c

+
+LLVM Utilities
+--------------
+
+LLVM has substitutes for GNU binutils utilities. These can be invoked as
+additional parameters to `make`.
+
+ make CC=clang AS=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \\
+ OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-objsize \\
+ READELF=llvm-readelf HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar \\
+ HOSTLD=ld.lld

+
+Getting Help
+------------
+
+- `Website <https://clangbuiltlinux.github.io/>`_
+- `Mailing List <https://groups.google.com/forum/#!forum/clang-built-linux>`_: <clang-bu...@googlegroups.com>
+- `Issue Tracker <https://github.com/ClangBuiltLinux/linux/issues>`_
+- IRC: #clangbuiltlinux on chat.freenode.net
+- `Telegram <https://t.me/ClangBuiltLinux>`_: @ClangBuiltLinux
+- `Wiki <https://github.com/ClangBuiltLinux/linux/wiki>`_
+- `Beginner Bugs <https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22>`_

+
+Getting LLVM
+-------------
+
+- http://releases.llvm.org/download.html
+- https://github.com/llvm/llvm-project
+- https://llvm.org/docs/GettingStarted.html
+- https://llvm.org/docs/CMake.html
+- https://apt.llvm.org/
+- https://www.archlinux.org/packages/extra/x86_64/llvm/
+- https://github.com/ClangBuiltLinux/tc-build
+- https://github.com/ClangBuiltLinux/linux/wiki/Building-Clang-from-source
+- https://android.googlesource.com/platform/prebuilts/clang/host/linux-x86/
--
2.25.0.265.gbab2e86ba0-goog

[Randy Dunlap]

Ongoing

> +<https://clang.llvm.org/>`_ and `LLVM <https://llvm.org/>`_ utilities to be
> +used as viable substitutes. Distributions such as `Android
> +<https://www.android.com/>`_, `ChromeOS
> +<https://www.chromium.org/chromium-os>`_, and `OpenMandriva
> +<https://www.openmandriva.org/>`_ use Clang built kernels. `LLVM is a
> +collection of toolchain components implemented in terms of C++ objects
> +<https://www.aosabook.org/en/llvm.html>`_. Clang is a front-end to LLVM that
> +supports C and the GNU C extensions required by the kernel, and is pronounced
> +"klang," not "see-lang."

Thanks for the info.

--
~Randy

[Masahiro Yamada]

On Tue, Feb 25, 2020 at 2:41 AM Nick Desaulniers
<ndesau...@google.com> wrote:
>
> Added to kbuild documentation. Provides more official info on building
> kernels with Clang and LLVM than our wiki.
>
> Suggested-by: Kees Cook <kees...@chromium.org>
> Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> ---

Perhaps, is it better to explicitly add it to MAINTAINERS?

--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4118,6 +4118,7 @@ W: https://clangbuiltlinux.github.io/
B: https://github.com/ClangBuiltLinux/linux/issues
C: irc://chat.freenode.net/clangbuiltlinux
S: Supported
+F: Documentation/kbuild/llvm.rst
K: \b(?i:clang|llvm)\b

CLEANCACHE API



Thanks.

--
Best Regards
Masahiro Yamada

[Kees Cook]

s/suffix/prefix/

Should this also include an update to Documentation/process/changes.rst
with the minimum version required? (I would expect this to be "9" for Clang,
and "11" for ld.lld.)

Otherwise, yes, with Randy and Masahiro's suggestions, please consider it:

Reviewed-by: Kees Cook <kees...@chromium.org>

--
Kees Cook

[Nathan Chancellor]

I think the clang one should be added in a separate patch that
solidifies that in include/linux/compiler-clang.h with a CLANG_VERSION
macro and version check, like in include/linux/compiler-gcc.h.

ld.lld's minimum version should also be 9, what is the blocking issue
that makes it 11?

> Otherwise, yes, with Randy and Masahiro's suggestions, please consider it:
>
> Reviewed-by: Kees Cook <kees...@chromium.org>
>
> --
> Kees Cook

Cheers,
Nathan

[Kees Cook]

On Mon, Feb 24, 2020 at 09:16:43PM -0700, Nathan Chancellor wrote:
> I think the clang one should be added in a separate patch that
> solidifies that in include/linux/compiler-clang.h with a CLANG_VERSION
> macro and version check, like in include/linux/compiler-gcc.h.

Sounds good.

> ld.lld's minimum version should also be 9, what is the blocking issue
> that makes it 11?

I think I've mostly got future patches on my mind. lld before 11 will
blow up on this:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=build/link-time
(but I haven't sent this patch yet since I didn't want to break lld
until it was fixed, which it is now, but I got distracted by other
stuff).

Additionally there will be LTO, but LTO will be version-checked. Then I
was thinking about the orphan section handling, and there is very
clearly more work to be done there too... (i.e. the synthesized sections
get reported by the orphan warning before they're actually processed by
DISCARD is some cases -- I still need to isolate the behavior and open a
bug.)

--
Kees Cook

[Sedat Dilek]

Hi,

that update for documentation purposes was overdue.

My last experiments were with Linux v5.3 and llvm-toolchain 9.0 means
Clang compiler v9.0 and LLD linker v9.0 on x86-64.
With Debian's kernel-config I was able to build OOTB (out-of-the-box)
with no extra patches.
I cannot speak for higher Linux and/or llvm-toolchain versions/combinations.

I would prefer such an information also for the *supported* Linux
versions, so people have a good orientation.

So for the above scenario, you can add:

Reviewed-by: Sedat Dilek <sedat...@gmail.com>.(Clang and LLD v9.0,
Linux v5.3, x86-64)

Regards,
- Sedat -




> --
> Kees Cook

>
> --
> 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/202002242003.870E5F80%40keescook.

[Nick Desaulniers]

I'm happy to leave it to the maintainers of Documentation/. Otherwise
we have a file for which there is no MAINTAINER, which seems
ambiguous.

--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

On Mon, Feb 24, 2020 at 8:16 PM Nathan Chancellor
<natecha...@gmail.com> wrote:
> > Should this also include an update to Documentation/process/changes.rst
> > with the minimum version required? (I would expect this to be "9" for Clang,
> > and "11" for ld.lld.)
>
> I think the clang one should be added in a separate patch that
> solidifies that in include/linux/compiler-clang.h with a CLANG_VERSION
> macro and version check, like in include/linux/compiler-gcc.h.
>
> ld.lld's minimum version should also be 9, what is the blocking issue
> that makes it 11?

I'm super hesitant to put a minimally required version of Clang, since
it really depends on the configs you're using. Sure, clang-9 will
probably work better than clang-4 for some configs, but I would say
ToT clang built from source would be even better, as unrealistic as
that is for most people. The question of "what's our support model"
hasn't realistically come up yet, so I don't really want to make a
decision on that right now and potentially pigeonhole us into some
support scheme that's theoretical or hypothetical. We need to expand
out the CI more, and get more people to even care about Clang, before
we start to concern ourselves with providing an answer to the question
"what versions of clang are supported?" But it's just a strong
opinion of mine, held loosely.

Either way, it can be done (or not) in a follow up patch. I would
like to land some Documentation/ even if it's not perfect, we can go
from there.
--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

Added to kbuild documentation. Provides more official info on building
kernels with Clang and LLVM than our wiki.

Suggested-by: Randy Dunlap <rdu...@infradead.org>
Reviewed-by: Kees Cook <kees...@chromium.org>
Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
Reviewed-by: Sedat Dilek <sedat...@gmail.com>

Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---

Changes V1 -> V2:
* s/On going/ongoing/
* add Randy's Suggested-by
* add Nathan and Sedat's Reviewed-by
* Upgrade Kees' Sugguested-by to Reviewed-by
* s/suffix/prefix/

Documentation/kbuild/index.rst | 1 +
Documentation/kbuild/llvm.rst | 80 ++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+)
create mode 100644 Documentation/kbuild/llvm.rst

diff --git a/Documentation/kbuild/index.rst b/Documentation/kbuild/index.rst
index 0f144fad99a6..3882bd5f7728 100644
--- a/Documentation/kbuild/index.rst
+++ b/Documentation/kbuild/index.rst
@@ -19,6 +19,7 @@ Kernel Build System

issues
reproducible-builds
+ llvm

.. only:: subproject and html

diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
new file mode 100644

index 000000000000..d6c79eb4e23e

--- /dev/null
+++ b/Documentation/kbuild/llvm.rst
@@ -0,0 +1,80 @@
+==============================
+Building Linux with Clang/LLVM
+==============================
+
+This document covers how to build the Linux kernel with Clang and LLVM
+utilities.
+
+About
+-----
+
+The Linux kernel has always traditionally been compiled with GNU toolchains

+such as GCC and binutils. Ongoing work has allowed for `Clang

+`CROSS_COMPILE` is not used to prefix the Clang compiler binary, instead

[Kees Cook]

On Tue, Feb 25, 2020 at 12:59:25PM -0800, Nick Desaulniers wrote:
> On Mon, Feb 24, 2020 at 8:16 PM Nathan Chancellor
> <natecha...@gmail.com> wrote:
> > > Should this also include an update to Documentation/process/changes.rst
> > > with the minimum version required? (I would expect this to be "9" for Clang,
> > > and "11" for ld.lld.)
> >
> > I think the clang one should be added in a separate patch that
> > solidifies that in include/linux/compiler-clang.h with a CLANG_VERSION
> > macro and version check, like in include/linux/compiler-gcc.h.
> >
> > ld.lld's minimum version should also be 9, what is the blocking issue
> > that makes it 11?
>
> I'm super hesitant to put a minimally required version of Clang, since
> it really depends on the configs you're using. Sure, clang-9 will
> probably work better than clang-4 for some configs, but I would say

I think it's not unreasonable to say clang-9 due to x86 not building
prior to clang-9. (Yes, other archs can build with earlier clang, but
that's true for earlier gccs too.)

> ToT clang built from source would be even better, as unrealistic as
> that is for most people. The question of "what's our support model"
> hasn't realistically come up yet, so I don't really want to make a
> decision on that right now and potentially pigeonhole us into some
> support scheme that's theoretical or hypothetical. We need to expand
> out the CI more, and get more people to even care about Clang, before
> we start to concern ourselves with providing an answer to the question
> "what versions of clang are supported?" But it's just a strong
> opinion of mine, held loosely.

"Supported" is hand-wavey anyway. I would say, "this version is
_expected_ to build the kernel", etc.

> Either way, it can be done (or not) in a follow up patch. I would
> like to land some Documentation/ even if it's not perfect, we can go
> from there.

Sounds fine, but I think we should take a specific version stand as the
"minimum" version. Being able to build x86 defconfig is a good minimum
IMO.

--
Kees Cook

[Joe Perches]

On Tue, 2020-02-25 at 13:56 -0800, Kees Cook wrote:
> I think we should take a specific version stand as the
> "minimum" version. Being able to build x86 defconfig is a good minimum
> IMO.

Agree.

It's odd to say that clang 4 is fine for arm when it's
not fine for x86. It's also reasonable to expect arm
users to upgrade their compiler to a more recent version
when the only cost is a very small bit of time.

[Masahiro Yamada]

Hi.

On Wed, Feb 26, 2020 at 6:02 AM Nick Desaulniers
<ndesau...@google.com> wrote:
>
> Added to kbuild documentation. Provides more official info on building
> kernels with Clang and LLVM than our wiki.
>
> Suggested-by: Randy Dunlap <rdu...@infradead.org>
> Reviewed-by: Kees Cook <kees...@chromium.org>
> Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
> Reviewed-by: Sedat Dilek <sedat...@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> ---
> Changes V1 -> V2:
> * s/On going/ongoing/
> * add Randy's Suggested-by

I do not understand this tag update.

As far as I saw the review process,
I do not think Randy deserves to have Suggested-by
because he just pointed out a typo (on going -> ongoing) :
https://patchwork.kernel.org/patch/11401189/#23179575

(or, was there off-line activity I had missed?)

> * add Nathan and Sedat's Reviewed-by
> * Upgrade Kees' Sugguested-by to Reviewed-by

We can add both

Suggested-by: Kees Cook <kees...@chromium.org>

and

Reviewed-by: Kees Cook <kees...@chromium.org>



I think Suggested-by and Reviewed-by should be orthogonal.


Thanks.

[Masahiro Yamada]

On Wed, Feb 26, 2020 at 5:52 AM Nick Desaulniers

<ndesau...@google.com> wrote:
>
> On Mon, Feb 24, 2020 at 4:34 PM Masahiro Yamada <masa...@kernel.org> wrote:
> >
> > On Tue, Feb 25, 2020 at 2:41 AM Nick Desaulniers
> > <ndesau...@google.com> wrote:
> > >
> > > Added to kbuild documentation. Provides more official info on building
> > > kernels with Clang and LLVM than our wiki.
> > >
> > > Suggested-by: Kees Cook <kees...@chromium.org>
> > > Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
> > > Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> > > ---
> >
> >
> > Perhaps, is it better to explicitly add it to MAINTAINERS?
> >
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4118,6 +4118,7 @@ W: https://clangbuiltlinux.github.io/
> > B: https://github.com/ClangBuiltLinux/linux/issues
> > C: irc://chat.freenode.net/clangbuiltlinux
> > S: Supported
> > +F: Documentation/kbuild/llvm.rst
> > K: \b(?i:clang|llvm)\b
>
> I'm happy to leave it to the maintainers of Documentation/. Otherwise
> we have a file for which there is no MAINTAINER, which seems
> ambiguous.

It is common that MAINTAINERS lists per-file (per-driver) maintainers.
It does not necessarily mean a person who picks up patches.

scripts/get_maintainer.pl lists maintainers that
match any F:, N:, K: patterns.
So, both Doc and Kbuild maintainers/ML will still be listed.

Having said that, it is up to you. Either is fine with me.
Another pattern 'K: \b(?i:clang|llvm)\b' covers this file anyway.


(BTW, I am also happy to see your name as the maintainer of this entry.)

[Sedat Dilek]

On Wed, Feb 26, 2020 at 12:31 PM Masahiro Yamada <masa...@kernel.org> wrote:
>
> Hi.
>
>
> On Wed, Feb 26, 2020 at 6:02 AM Nick Desaulniers
> <ndesau...@google.com> wrote:
> >
> > Added to kbuild documentation. Provides more official info on building
> > kernels with Clang and LLVM than our wiki.
> >
> > Suggested-by: Randy Dunlap <rdu...@infradead.org>
> > Reviewed-by: Kees Cook <kees...@chromium.org>
> > Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
> > Reviewed-by: Sedat Dilek <sedat...@gmail.com>
> > Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> > ---
> > Changes V1 -> V2:
> > * s/On going/ongoing/
> > * add Randy's Suggested-by
>
>
> I do not understand this tag update.
>
> As far as I saw the review process,
> I do not think Randy deserves to have Suggested-by
> because he just pointed out a typo (on going -> ongoing) :
> https://patchwork.kernel.org/patch/11401189/#23179575
>
> (or, was there off-line activity I had missed?)
>

Hi Masahiro,

I got some credits from Nick for a review by seeing a typo - not on a
review of the code - and H. Peter Anvin asked why.

I am not sure what is here the correct credit to give.
Depends a "Reviewed-by" and/or "Suggested-by" on a technical review?

My POV: When people take time to look over patches they should get
credits - sort of esteem.

Regards,
- Sedat -

P.S.: Tipp: Use codespell to find typos :-).

[1] https://git.kernel.org/linus/0e2e160033283e20f688d8bad5b89460cc5bfcc4
"x86/asm: Add _ASM_ARG* constants for argument registers to <asm/asm.h>"

[Sedat Dilek]

+1 (Please drop the BTW - This was suggested in the brainstorming
session on the ClangBuiltLinux Meetup in Zurich).

I suggest to add Nathan and Kees if they are willing to be responsible
for the maintainer job.

- Sedat -

[Masahiro Yamada]

Documentation/process/submitting-patches.rst

13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:

is a helpful guideline.

[Sedat Dilek]

Thanks for CCing Randy and the pointer to
"Documentation/process/submitting-patches.rst" file.

This document [1] is mostly focusing on technical handling.

- Sedat -

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n584

[Randy Dunlap]

On 2/26/20 3:30 AM, Masahiro Yamada wrote:
> Hi.
>
>
> On Wed, Feb 26, 2020 at 6:02 AM Nick Desaulniers
> <ndesau...@google.com> wrote:
>>
>> Added to kbuild documentation. Provides more official info on building
>> kernels with Clang and LLVM than our wiki.
>>
>> Suggested-by: Randy Dunlap <rdu...@infradead.org>
>> Reviewed-by: Kees Cook <kees...@chromium.org>
>> Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
>> Reviewed-by: Sedat Dilek <sedat...@gmail.com>
>> Signed-off-by: Nick Desaulniers <ndesau...@google.com>
>> ---
>> Changes V1 -> V2:
>> * s/On going/ongoing/
>> * add Randy's Suggested-by
>
>
> I do not understand this tag update.
>
> As far as I saw the review process,
> I do not think Randy deserves to have Suggested-by
> because he just pointed out a typo (on going -> ongoing) :

I agree.

> https://patchwork.kernel.org/patch/11401189/#23179575
>
> (or, was there off-line activity I had missed?)

--
~Randy

[Nick Desaulniers]

That's a very x86 centric point of view.
--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

Added to kbuild documentation. Provides more official info on building
kernels with Clang and LLVM than our wiki.

Suggested-by: Kees Cook <kees...@chromium.org>

Reviewed-by: Kees Cook <kees...@chromium.org>
Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
Reviewed-by: Sedat Dilek <sedat...@gmail.com>
Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---

Changes V2 -> V3:
* Drop Randy's Suggested-by tag. Sorry Randy, I do appreciate the review
though.
* Add F: line to MAINTAINERS.

Changes V1 -> V2:
* s/On going/ongoing/
* add Randy's Suggested-by
* add Nathan and Sedat's Reviewed-by
* Upgrade Kees' Sugguested-by to Reviewed-by
* s/suffix/prefix/

Documentation/kbuild/index.rst | 1 +
Documentation/kbuild/llvm.rst | 80 ++++++++++++++++++++++++++++++++++

MAINTAINERS | 1 +
3 files changed, 82 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8b85f22b9b69..79e1f9bfb2b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4124,6 +4124,7 @@ B: https://github.com/ClangBuiltLinux/linux/issues

C: irc://chat.freenode.net/clangbuiltlinux
S: Supported

K: \b(?i:clang|llvm)\b

+F: Documentation/kbuild/llvm.rst

CLEANCACHE API
M: Konrad Rzeszutek Wilk <konra...@oracle.com>
--
2.25.0.265.gbab2e86ba0-goog

[Joe Perches]

Really?

How many code generation improvements and possible defects
have been corrected between clang 4 and clang 10 for arm?

I presume more than a few.

[Masahiro Yamada]

On Thu, Feb 27, 2020 at 8:23 AM Nick Desaulniers
<ndesau...@google.com> wrote:
>
> Added to kbuild documentation. Provides more official info on building
> kernels with Clang and LLVM than our wiki.
>
> Suggested-by: Kees Cook <kees...@chromium.org>
> Reviewed-by: Kees Cook <kees...@chromium.org>
> Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
> Reviewed-by: Sedat Dilek <sedat...@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> ---
> Changes V2 -> V3:
> * Drop Randy's Suggested-by tag. Sorry Randy, I do appreciate the review
> though.
> * Add F: line to MAINTAINERS.
>
> Changes V1 -> V2:
> * s/On going/ongoing/
> * add Randy's Suggested-by
> * add Nathan and Sedat's Reviewed-by
> * Upgrade Kees' Sugguested-by to Reviewed-by
> * s/suffix/prefix/

Setting aside the minimal version discussion,
I am fine with this patch version.

I will apply it shortly.

Thanks.

[Nathan Chancellor]

I would not mind having my name added under this MAINTAINERS entry but I
definitely think that is a conversation for a different thread.

Cheers,
Nathan

[Nathan Chancellor]

On Tue, Feb 25, 2020 at 12:59:25PM -0800, Nick Desaulniers wrote:

I think the question of support model is something that we are going to
have to sit down and figure out sooner rather than later, especially if
we are adding this file to the Documentation; we are saying that this IS
supported in some fashion, we need to be able to answer how we are going
to resolve issues and what versions can be adequately expect to work
with the kernel.

I think that being able to tell people to update is not unreasonable,
given how efficient we are getting fixing into clang. However, clang's
release model is definitely different from gcc's and that can make
getting fixes into the hands of regular users harder, aside from asking
them to build their own version, which again, is not that difficult and
hard to do with something like tc-build.

I agree this should happen in a follow up patch. I think starting with
clang-9 and saying if anything older works, cool, otherwise update is
probably a decent initial model.

Cheers,
Nathan

[Masahiro Yamada]

On Thu, Feb 27, 2020 at 8:23 AM Nick Desaulniers
<ndesau...@google.com> wrote:
>
> Added to kbuild documentation. Provides more official info on building
> kernels with Clang and LLVM than our wiki.
>
> Suggested-by: Kees Cook <kees...@chromium.org>
> Reviewed-by: Kees Cook <kees...@chromium.org>
> Reviewed-by: Nathan Chancellor <natecha...@gmail.com>
> Reviewed-by: Sedat Dilek <sedat...@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> ---

Applied to linux-kbuild.

Thanks.

[Nick Desaulniers]

On Thu, Feb 20, 2020 at 4:45 PM Kees Cook <kees...@chromium.org> wrote:
>
> On Thu, Feb 20, 2020 at 03:48:58PM -0800, 'Nick Desaulniers' via Clang Built Linux wrote:
> > Prior to this patch, building the Linux kernel with Clang
> > looked like:
> >
> > $ make CC=clang
> >
> > or when cross compiling:
> >
> > $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
> >
> > which got very verbose and unwieldy when using all of LLVM's substitutes
> > for GNU binutils:
> >
> > $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang AS=clang \
> > LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
> > OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-objsize \
> > READELF=llvm-readelf HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar \
> > HOSTLD=ld.lld
> >
> > This change adds a new Makefile under scripts/ which will be included in
> > the top level Makefile AFTER CC and friends are set, in order to make
> > the use of LLVM utilities when building a Linux kernel more ergonomic.
> >
> > With this patch, the above now looks like:
> >
> > $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1
>
> I like this idea, but I think it might not be happening in the right
> place... but I'm not sure what that place is. This will collide, for
> example, with the Clang LTO series from Sami, as that _requires_ the
> LLVM utilities in many places. I think some coordination with Kconfig
> will be needed to not make this kind of crazy.
>
> e.g. Sami currently has to hard-code this logic:
>
> https://github.com/samitolvanen/linux/commit/42572f50c5f951cac7ea4720658d9abd2b995529#diff-b67911656ef5d18c4ae36cb6741b7965R652

Thanks for the link. I don't think Sami's patch should hard code
these values. My change will set these values BEFORE Sami's first
hunk (my Makefile change is L472, his is L652. I think with my patch,
Sami could:
1. drop the first hunk outright.
2. use `$(NM)` in scripts/Makefile.build. (Notice how it does this
with `$(AR)`, and does not export `LLVM_AR`).
3. require the use of LLVM=1 for CONFIG_LTO_CLANG, either via
documentation, or build time test.

I don't think we should have to suffer at the command line for tools
that need additional test coverage, for an out of tree patch (sorry
Sami, I'll still do whatever I can to help rectify that). I want
`make LLVM=1` to be the default way we build things going forward; if
that complicates some workflows somewhere, sorry, and we'll help you
fix it, but we're not going to hold up the train over it.

Nathan & Masahiro, I was really hoping to get your thoughts on this RFC.

>
> -Kees
>
> >
> > Then you can "opt out" of certain LLVM utilities explicitly:
> >
> > $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 AS=as
> >
> > will instead invoke arm-linux-gnueabihf-as in place of clang for AS.
> >
> > This would make it more verbose to opt into just one tool from LLVM, but
> > this patch doesn't actually break the old style; just leave off LLVM=1.
> > Also, LLVM=1 CC=clang would wind up prefixing clang with $CROSS_COMPILE.
> > In that case, it's recommended to just drop LLVM=1 and use the old
> > style. So LLVM=1 can be thought of as default to LLVM with explicit opt
> > ins for GNU, vs the current case of default to GNU and opt in for LLVM.
> >
> > A key part of the design of this patch is to be minimally invasive to
> > the top level Makefile and not break existing workflows. We could get
> > more aggressive, but I'd prefer to save larger refactorings for another
> > day.
> >
> > About the script:
> > The pattern used in the script is in the form:
> >
> > ifeq "$(origin $(CC))" "file"
> > $(CC) := $(clang)
> > else
> > override $(CC) := $(CROSS_COMPILE)$(CC)
> > endif
> >
> > "Metaprogramming" (eval) is used to template the above to make it more
> > concise for specifying all of the substitutions.
> >
> > The "origin" of a variable tracks whether a variable was explicitly set
> > via "command line", "environment", was defined earlier via Makefile
> > "file", was provided by "default", or was "undefined".
> >
> > Variable assignment in GNU Make has some special and complicated rules.
> >
> > If the variable was set earlier explicitly in the Makefile, we can
> > simply reassign a new value to it. If a variable was unspecified, then
> > earlier assignments were executed and change the origin to file.
> > Otherwise, the variable was explicitly specified.
> >
> > If a variable's "origin" was "command line" or "environment",
> > non-"override" assignments are not executed. The "override" directive
> > forces the assignment regardless of "origin".
> >
> > Some tips I found useful for debugging for future travelers:
> >
> > $(info $$origin of $$CC is $(origin CC))
> >
> > at the start of the new script for all of the variables can help you
> > understand "default" vs "undefined" variable origins.
> >
> > $(info $$CC is [${CC}])
> >
> > in the top level Makefile after including the new script, for all of the
> > variables can help you check that they're being set as expected.
> >
> > Link: https://www.gnu.org/software/make/manual/html_node/Eval-Function.html
> > Link: https://www.gnu.org/software/make/manual/html_node/Origin-Function.html
> > Link: https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
> > Link: https://www.gnu.org/software/make/manual/html_node/Override-Directive.html

> > Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> > ---

> > Makefile | 4 ++++
> > scripts/Makefile.llvm | 24 ++++++++++++++++++++++++
> > 2 files changed, 28 insertions(+)
> > create mode 100644 scripts/Makefile.llvm
> >
> > diff --git a/Makefile b/Makefile
> > index b954c304c479..8c8888ebb822 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -472,6 +472,10 @@ KBUILD_LDFLAGS :=
> > GCC_PLUGINS_CFLAGS :=
> > CLANG_FLAGS :=
> >
> > +ifeq ($(LLVM),1)
> > +include scripts/Makefile.llvm
> > +endif
> > +
> > export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE AS LD CC
> > export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
> > export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> > diff --git a/scripts/Makefile.llvm b/scripts/Makefile.llvm
> > new file mode 100644
> > index 000000000000..a4401c8677dc
> > --- /dev/null
> > +++ b/scripts/Makefile.llvm
> > @@ -0,0 +1,24 @@
> > +define META_set =
> > +ifeq "$(origin $(1))" "file"
> > +$(1) := $(2)
> > +else
> > +override $(1) := $(CROSS_COMPILE)$($(1))
> > +endif
> > +endef
> > +
> > +$(eval $(call META_set,CC,clang))
> > +$(eval $(call META_set,AS,clang))
> > +$(eval $(call META_set,LD,ld.lld))
> > +$(eval $(call META_set,AR,llvm-ar))
> > +$(eval $(call META_set,NM,llvm-nm))
> > +$(eval $(call META_set,STRIP,llvm-strip))
> > +$(eval $(call META_set,OBJCOPY,llvm-objcopy))
> > +$(eval $(call META_set,OBJDUMP,llvm-objdump))
> > +$(eval $(call META_set,OBJSIZE,llvm-objsize))
> > +$(eval $(call META_set,READELF,llvm-readelf))
> > +$(eval $(call META_set,HOSTCC,clang))
> > +$(eval $(call META_set,HOSTCXX,clang++))
> > +$(eval $(call META_set,HOSTAR,llvm-ar))
> > +$(eval $(call META_set,HOSTLD,ld.lld))
> > +
> > +## TODO: HOSTAR, HOSTLD in tools/objtool/Makefile
> > --
> > 2.25.0.265.gbab2e86ba0-goog
> >
> > -
--
Thanks,
~Nick Desaulniers

[Sami Tolvanen]

On Mon, Mar 16, 2020 at 11:28 AM Nick Desaulniers
<ndesau...@google.com> wrote:
> I don't think we should have to suffer at the command line for tools
> that need additional test coverage, for an out of tree patch

Fully agreed. The patch looks good to me.

Sami

[Nathan Chancellor]

On Thu, Feb 20, 2020 at 03:48:58PM -0800, 'Nick Desaulniers' via Clang Built Linux wrote:
> Prior to this patch, building the Linux kernel with Clang
> looked like:
>
> $ make CC=clang
>
> or when cross compiling:
>
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
>
> which got very verbose and unwieldy when using all of LLVM's substitutes
> for GNU binutils:
>
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang AS=clang \
> LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
> OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-objsize \
> READELF=llvm-readelf HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar \
> HOSTLD=ld.lld
>
> This change adds a new Makefile under scripts/ which will be included in
> the top level Makefile AFTER CC and friends are set, in order to make
> the use of LLVM utilities when building a Linux kernel more ergonomic.
>
> With this patch, the above now looks like:
>
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1
>

The overall approach seems relatively sane to me, as someone who is not
a GNU make expert. This could be handy for seeing how hermetic the build
system is by symlinking /bin/false to gcc, ld, as, etc and adding that
folder to PATH then passing LLVM=1.

Something that needs to be considered is how do we handle people who use
something like apt.llvm.org, where those versions are suffixed with a
number. Realistically, they could just add /usr/lib/llvm-#/bin to PATH
and then everything would work but should we add something like
LLVM_SUFFIX= so that they could say LLVM_SUFFIX=-11 to get clang-11,
ld.lld-11, etc?

Cheers,
Nathan

[Nick Desaulniers]

Prior to this patch, building the Linux kernel with Clang
looked like:

$ make CC=clang

or when cross compiling:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang

which got very verbose and unwieldy when using all of LLVM's substitutes
for GNU binutils:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang AS=clang \
LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump OBJSIZE=llvm-objsize \
READELF=llvm-readelf HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar \
HOSTLD=ld.lld

This change adds a new Makefile under scripts/ which will be included in
the top level Makefile AFTER CC and friends are set, in order to make
the use of LLVM utilities when building a Linux kernel more ergonomic.

With this patch, the above now looks like:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=y

Then you can "opt out" of certain LLVM utilities explicitly:

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=y AS=as

will instead invoke arm-linux-gnueabihf-as in place of clang for AS.

Or when not cross compiling:

$ make LLVM=y AS=as

This would make it more verbose to opt into just one tool from LLVM, but

this patch doesn't actually break the old style; just leave off LLVM=y.
Also, LLVM=y CC=clang would wind up prefixing clang with $CROSS_COMPILE.
In that case, it's recommended to just drop LLVM=y and use the old
style. So LLVM=y can be thought of as default to LLVM with explicit opt

ins for GNU, vs the current case of default to GNU and opt in for LLVM.

A key part of the design of this patch is to be minimally invasive to
the top level Makefile and not break existing workflows. We could get
more aggressive, but I'd prefer to save larger refactorings for another
day.

Finally, some linux distributions package specific versions of LLVM
utilities with naming conventions that use the version as a suffix, ie.
clang-11. In that case, you can use LLVM=<number> and that number will
be used as a suffix. Example:

$ make LLVM=11

will invoke clang-11, ld.lld-11, llvm-objcopy-11, etc.

Suggested-by: Nathan Chancellor <natecha...@gmail.com>

Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---

Changes V1 -> V2:
* Rather than LLVM=1, use LLVM=y to enable all.
* LLVM=<anything other than y> becomes a suffix, LLVM_SUFFIX.
* strip has to be used on the LLVM_SUFFIX to avoid an extra whitespace.


Makefile | 4 ++++
scripts/Makefile.llvm | 30 ++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+)

create mode 100644 scripts/Makefile.llvm

diff --git a/Makefile b/Makefile

index 402f276da062..72ec9dfea15e 100644
--- a/Makefile
+++ b/Makefile
@@ -475,6 +475,10 @@ KBUILD_LDFLAGS :=
GCC_PLUGINS_CFLAGS :=
CLANG_FLAGS :=

+ifneq ($(LLVM),)

+include scripts/Makefile.llvm
+endif
+
export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE AS LD CC
export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
diff --git a/scripts/Makefile.llvm b/scripts/Makefile.llvm
new file mode 100644

index 000000000000..0bab45a100a3
--- /dev/null
+++ b/scripts/Makefile.llvm
@@ -0,0 +1,30 @@
+LLVM_SUFFIX=
+
+ifneq ($(LLVM),y)
+LLVM_SUFFIX += -$(LLVM)
+endif
+

+define META_set =
+ifeq "$(origin $(1))" "file"

+$(1) := $(2)$(strip $(LLVM_SUFFIX))

+else
+override $(1) := $(CROSS_COMPILE)$($(1))
+endif
+endef
+
+$(eval $(call META_set,CC,clang))
+$(eval $(call META_set,AS,clang))
+$(eval $(call META_set,LD,ld.lld))
+$(eval $(call META_set,AR,llvm-ar))
+$(eval $(call META_set,NM,llvm-nm))
+$(eval $(call META_set,STRIP,llvm-strip))
+$(eval $(call META_set,OBJCOPY,llvm-objcopy))
+$(eval $(call META_set,OBJDUMP,llvm-objdump))
+$(eval $(call META_set,OBJSIZE,llvm-objsize))
+$(eval $(call META_set,READELF,llvm-readelf))
+$(eval $(call META_set,HOSTCC,clang))
+$(eval $(call META_set,HOSTCXX,clang++))
+$(eval $(call META_set,HOSTAR,llvm-ar))
+$(eval $(call META_set,HOSTLD,ld.lld))
+
+## TODO: HOSTAR, HOSTLD in tools/objtool/Makefile
--

2.25.1.481.gfbce0eb801-goog

[Nick Desaulniers]

Masahiro and Nathan,
I get daily reminders that `make CC=clang LD=ld.lld NM=llvm-nm ...`
sucks. I always prioritize code reviews for you, can you please carve
out time to review+test+provide feedback for me?

--
Thanks,
~Nick Desaulniers

[Nathan Chancellor]

Sorry for the delay in review :(

The use of override appears to break exporting the value to Kconfig,
which causes Kconfig to endlessly loop at the --syncconfig step:

$ make -j$(nproc) ARCH=riscv CROSS_COMPILE=riscv64-linux-gnu- LD=ld LLVM=y defconfig prepare
...
Makefile: LD is riscv64-linux-gnu-ld
*** Default configuration is based on 'defconfig'
Kconfig: LD is ld
#
# No change to .config
#
Makefile: LD is riscv64-linux-gnu-ld
Makefile: LD is riscv64-linux-gnu-ld
scripts/kconfig/conf --syncconfig Kconfig
Kconfig: LD is ld
Makefile: LD is riscv64-linux-gnu-ld
auto.conf.cmd: LD is riscv64-linux-gnu-ld
Makefile: LD is riscv64-linux-gnu-ld
scripts/kconfig/conf --syncconfig Kconfig
Kconfig: LD is ld
Makefile: LD is riscv64-linux-gnu-ld
auto.conf.cmd: LD is riscv64-linux-gnu-ld
Makefile: LD is riscv64-linux-gnu-ld
scripts/kconfig/conf --syncconfig Kconfig
Kconfig: LD is ld
...

This endless loops happens because auto.conf.cmd is constantly being
regenerated because Kconfig is not picking up the override value, as you
can see from the debugging output above and auto.conf.cmd below:

$ rg -A 2 "LD\)" include/config/auto.conf.cmd | cat
ifneq "$(LD)" "ld"
include/config/auto.conf: FORCE
endif

$(LD) evaluates to riscv64-linux-gnu-ld, which is not equal to ld, so
include/config/auto.conf is regenerated, but env_write_dep in
scripts/kconfig/preprocess.c just picks up the environment value, rather
than the value that was overridden. This appears to be related to
recursive make calls from what I can tell.

$ cat Makefile
override FOO := foo
export FOO

all:
$(info Makefile: $$(FOO) is $(FOO))
$(MAKE) -f Makefile.sub all

$ cat Makefile.sub
all:
$(info Makefile.sub: $$(FOO) is $(FOO))

$ make -s
Makefile: $(FOO) is foo
Makefile.sub: $(FOO) is foo

$ make -s FOO=bar
Makefile: $(FOO) is foo
Makefile.sub: $(FOO) is bar

No idea if this is a bug in make or not, this seems subtle. Not really
sure where to go from here, hopefully this is useful to you or Masahiro.

You will only notice this with variables that impact Kconfig, which
is why you never noticed with AS=as. See include/config/auto.conf.cmd
for the full list (CC, LD, NM, OBJCOPY, HOSTCC, and HOSTCXX are the ones
I see).

Debug patch if you want to play around with this.

Cheers,
Nathan

================================

diff --git a/Makefile b/Makefile
index acd8022ddb52..81cbb940e035 100644
--- a/Makefile
+++ b/Makefile
@@ -479,6 +479,8 @@ ifneq ($(LLVM),)
include scripts/Makefile.llvm
endif

+$(info Makefile: LD is $(LD))

+
export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE AS LD CC
export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX

diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 8074f14d9d0d..43a852e0ee93 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -57,3 +57,5 @@ gcc-version := $(shell,$(srctree)/scripts/gcc-version.sh $(CC))
cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$(1))
m32-flag := $(cc-option-bit,-m32)
m64-flag := $(cc-option-bit,-m64)
+
+$(info,Kconfig: LD is $(LD))
diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c
index 0243086fb168..fd828efc85fc 100644
--- a/scripts/kconfig/preprocess.c
+++ b/scripts/kconfig/preprocess.c
@@ -93,6 +93,7 @@ void env_write_dep(FILE *f, const char *autoconfig_name)

list_for_each_entry_safe(e, tmp, &env_list, node) {
fprintf(f, "ifneq \"$(%s)\" \"%s\"\n", e->name, e->value);
+ fprintf(f, "$(info auto.conf.cmd: %s is $(%s))\n", e->name, e->name);
fprintf(f, "%s: FORCE\n", autoconfig_name);
fprintf(f, "endif\n");
env_del(e);

[Masahiro Yamada]

Kconfig is invoked from scripts/kconfig/Makefile
instead of the top Makefile.


Just add one more debug code

$(info scripts/kconfig/Makefile: LD is $(LD))

into scripts/kconfig/Makefile.


It is "ld", not "riscv64-linux-gnu-ld".



The variable passed from the command line
is strong, and overrides the variable in sub-make as well.

This is not a bug.

The variable passed from the command line
(which is inherited to sub-make as well)
is stronger than the environment variable.

"make -s FOO=BAR" is not equivalent to "FOO=BAR make -s".

[Masahiro Yamada]

I also had planned to provide a single switch to change
all the tool defaults to LLVM.

So, supporting 'LLVM' is fine, but I'd rather want this
look symmetrical, and easy to understand.

CPP = $(CC) -E
ifneq ($(LLVM),)
CC = $(LLVM_DIR)clang
LD = $(LLVM_DIR)ld.lld
AR = $(LLVM_DIR)llvm-ar
NM = $(LLVM_DIR)llvm-nm
OBJCOPY = $(LLVM_DIR)llvm-objcopy
OBJDUMP = $(LLVM_DIR)llvm-objdump
READELF = $(LLVM_DIR)llvm-readelf
OBJSIZE = $(LLVM_DIR)llvm-size
STRIP = $(LLVM_DIR)llvm-strip
else
CC = $(CROSS_COMPILE)gcc
LD = $(CROSS_COMPILE)ld
AR = $(CROSS_COMPILE)ar
NM = $(CROSS_COMPILE)nm
OBJCOPY = $(CROSS_COMPILE)objcopy
OBJDUMP = $(CROSS_COMPILE)objdump
READELF = $(CROSS_COMPILE)readelf
OBJSIZE = $(CROSS_COMPILE)size
STRIP = $(CROSS_COMPILE)strip
endif



I attached two patches.
Comments appreciated.

[Nick Desaulniers]

On Sat, Mar 28, 2020 at 6:57 PM Masahiro Yamada <masa...@kernel.org> wrote:
>
> I also had planned to provide a single switch to change
> all the tool defaults to LLVM.
>
> So, supporting 'LLVM' is fine, but I'd rather want this
> look symmetrical, and easy to understand.
>
> CPP = $(CC) -E
> ifneq ($(LLVM),)

Yes, a simple if statement is much simpler than the overly complex patch I had.

> CC = $(LLVM_DIR)clang

Do we need $LLVM_DIR? Shouldn't users just have that in their $PATH?

Also, I think we need to support suffixed binaries, as debian
distributes these with version suffixes, as Nathan points out. Or do
the debian packages install suffixed binaries AND path versioned
non-suffixed binaries?

> LD = $(LLVM_DIR)ld.lld
> AR = $(LLVM_DIR)llvm-ar
> NM = $(LLVM_DIR)llvm-nm
> OBJCOPY = $(LLVM_DIR)llvm-objcopy
> OBJDUMP = $(LLVM_DIR)llvm-objdump
> READELF = $(LLVM_DIR)llvm-readelf
> OBJSIZE = $(LLVM_DIR)llvm-size
> STRIP = $(LLVM_DIR)llvm-strip
> else
> CC = $(CROSS_COMPILE)gcc
> LD = $(CROSS_COMPILE)ld
> AR = $(CROSS_COMPILE)ar
> NM = $(CROSS_COMPILE)nm
> OBJCOPY = $(CROSS_COMPILE)objcopy
> OBJDUMP = $(CROSS_COMPILE)objdump
> READELF = $(CROSS_COMPILE)readelf
> OBJSIZE = $(CROSS_COMPILE)size
> STRIP = $(CROSS_COMPILE)strip
> endif
>
>
>
> I attached two patches.
> Comments appreciated.

I'm not sure the second one that recommends changing cc/c++ is the way
to go; I think it might harm hermeticity.

> If you are so addicted to LLVM

Lol, maybe?
--
Thanks,
~Nick Desaulniers

[Nathan Chancellor]

On Mon, Mar 30, 2020 at 11:58:19AM -0700, Nick Desaulniers wrote:
> On Sat, Mar 28, 2020 at 6:57 PM Masahiro Yamada <masa...@kernel.org> wrote:
> >
> > I also had planned to provide a single switch to change
> > all the tool defaults to LLVM.
> >
> > So, supporting 'LLVM' is fine, but I'd rather want this
> > look symmetrical, and easy to understand.
> >
> > CPP = $(CC) -E
> > ifneq ($(LLVM),)
>
> Yes, a simple if statement is much simpler than the overly complex patch I had.
>
> > CC = $(LLVM_DIR)clang
>
> Do we need $LLVM_DIR? Shouldn't users just have that in their $PATH?
>
> Also, I think we need to support suffixed binaries, as debian
> distributes these with version suffixes, as Nathan points out. Or do
> the debian packages install suffixed binaries AND path versioned
> non-suffixed binaries?

I think the idea here is that ultimately, the suffixed versions of clang
that Debian has in /usr/bin are symlinks to binaries in
/usr/lib/llvm-#/bin; as a result, a user could say
LLVM_DIR=/usr/lib/llvm-#/bin/ and all of those tools would be picked up
automatically. I am not really sure what is better.

I'll try to have some other comments by later today/tonight.

> > LD = $(LLVM_DIR)ld.lld
> > AR = $(LLVM_DIR)llvm-ar
> > NM = $(LLVM_DIR)llvm-nm
> > OBJCOPY = $(LLVM_DIR)llvm-objcopy
> > OBJDUMP = $(LLVM_DIR)llvm-objdump
> > READELF = $(LLVM_DIR)llvm-readelf
> > OBJSIZE = $(LLVM_DIR)llvm-size
> > STRIP = $(LLVM_DIR)llvm-strip
> > else
> > CC = $(CROSS_COMPILE)gcc
> > LD = $(CROSS_COMPILE)ld
> > AR = $(CROSS_COMPILE)ar
> > NM = $(CROSS_COMPILE)nm
> > OBJCOPY = $(CROSS_COMPILE)objcopy
> > OBJDUMP = $(CROSS_COMPILE)objdump
> > READELF = $(CROSS_COMPILE)readelf
> > OBJSIZE = $(CROSS_COMPILE)size
> > STRIP = $(CROSS_COMPILE)strip
> > endif
> >
> >
> >
> > I attached two patches.
> > Comments appreciated.
>
> I'm not sure the second one that recommends changing cc/c++ is the way
> to go; I think it might harm hermeticity.

Agreed. I do not modify my host system at all for this project, just
relying on PATH modification. In theory, we can still override HOSTCC
and HOSTCXX but that would defeat the purpose of that patch.

Cheers,
Nathan

[Masahiro Yamada]

On Tue, Mar 31, 2020 at 4:03 AM Nathan Chancellor
<natecha...@gmail.com> wrote:
>
> On Mon, Mar 30, 2020 at 11:58:19AM -0700, Nick Desaulniers wrote:
> > On Sat, Mar 28, 2020 at 6:57 PM Masahiro Yamada <masa...@kernel.org> wrote:
> > >
> > > I also had planned to provide a single switch to change
> > > all the tool defaults to LLVM.
> > >
> > > So, supporting 'LLVM' is fine, but I'd rather want this
> > > look symmetrical, and easy to understand.
> > >
> > > CPP = $(CC) -E
> > > ifneq ($(LLVM),)
> >
> > Yes, a simple if statement is much simpler than the overly complex patch I had.
> >
> > > CC = $(LLVM_DIR)clang
> >
> > Do we need $LLVM_DIR? Shouldn't users just have that in their $PATH?
> >
> > Also, I think we need to support suffixed binaries, as debian
> > distributes these with version suffixes, as Nathan points out. Or do
> > the debian packages install suffixed binaries AND path versioned
> > non-suffixed binaries?
>
> I think the idea here is that ultimately, the suffixed versions of clang
> that Debian has in /usr/bin are symlinks to binaries in
> /usr/lib/llvm-#/bin; as a result, a user could say
> LLVM_DIR=/usr/lib/llvm-#/bin/ and all of those tools would be picked up
> automatically. I am not really sure what is better.

I periodically build the latest llvm from the trunk,
and install it under my home directory.

So, I just thought it would be useful to
allow a user to specify the llvm directory.

Of course, I can do the equivalent by tweaking PATH, but
I hesitate to make the non-released version my default.



Having both LLVM_DIR and LLVM_SUFFIX seems verbose.

In fact, the debian provides multiple versions of GCC.
For example, my machine has

masahiro@pug:~$ ls -1 /usr/bin/gcc-*
/usr/bin/gcc-4.8
/usr/bin/gcc-5
/usr/bin/gcc-7
/usr/bin/gcc-ar
/usr/bin/gcc-ar-4.8
/usr/bin/gcc-ar-5
/usr/bin/gcc-ar-7
/usr/bin/gcc-nm
/usr/bin/gcc-nm-4.8
/usr/bin/gcc-nm-5
/usr/bin/gcc-nm-7
/usr/bin/gcc-ranlib
/usr/bin/gcc-ranlib-4.8
/usr/bin/gcc-ranlib-5
/usr/bin/gcc-ranlib-7

But, nobody has suggested GCC_SUFFIX.

So, I guess CROSS_COMPILE was enough to
choose a specific tool version.

> --
> 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/20200330190312.GA32257%40ubuntu-m2-xlarge-x86.

[Nick Desaulniers]

On Mon, Mar 30, 2020 at 11:25 PM Masahiro Yamada <masa...@kernel.org> wrote:
>
> On Tue, Mar 31, 2020 at 4:03 AM Nathan Chancellor
> <natecha...@gmail.com> wrote:
> >
> > On Mon, Mar 30, 2020 at 11:58:19AM -0700, Nick Desaulniers wrote:
> > > On Sat, Mar 28, 2020 at 6:57 PM Masahiro Yamada <masa...@kernel.org> wrote:
> > > >
> > > > I also had planned to provide a single switch to change
> > > > all the tool defaults to LLVM.
> > > >
> > > > So, supporting 'LLVM' is fine, but I'd rather want this
> > > > look symmetrical, and easy to understand.
> > > >
> > > > CPP = $(CC) -E
> > > > ifneq ($(LLVM),)
> > >
> > > Yes, a simple if statement is much simpler than the overly complex patch I had.
> > >
> > > > CC = $(LLVM_DIR)clang
> > >
> > > Do we need $LLVM_DIR? Shouldn't users just have that in their $PATH?
> > >
> > > Also, I think we need to support suffixed binaries, as debian
> > > distributes these with version suffixes, as Nathan points out. Or do
> > > the debian packages install suffixed binaries AND path versioned
> > > non-suffixed binaries?
> >
> > I think the idea here is that ultimately, the suffixed versions of clang
> > that Debian has in /usr/bin are symlinks to binaries in
> > /usr/lib/llvm-#/bin; as a result, a user could say
> > LLVM_DIR=/usr/lib/llvm-#/bin/ and all of those tools would be picked up
> > automatically. I am not really sure what is better.

$ sudo apt install clang-8
$ which clang-8
/usr/bin/clang-8
$ ls -l `!!`
/usr/bin/clang-8 -> ../lib/llvm-8/bin/clang
$ ls /usr/lib/llvm-8/bin
<non suffixed versions>

Ok, so Nathan, it looks like we don't need the version suffixes.
Instead, we can be more explicit with our $PATH, and only add the
above (and bintutils). I was thinking supporting the suffix was
required for our CI, but it seems like maybe not.

> I periodically build the latest llvm from the trunk,
> and install it under my home directory.
> So, I just thought it would be useful to
> allow a user to specify the llvm directory.
> Of course, I can do the equivalent by tweaking PATH, but
> I hesitate to make the non-released version my default.

Respectfully, I strongly disagree. This should be handled by
modifications to $PATH, either by your shell's .rc file when you
always want it, or exported for a session when you want it, or
prefixed to an invocation for the duration of that command. We should
not have a new variable just for the path of a few tools.

Rather than `make LLVM_DIR=~/llvm-project LLVM=1`, you can do
`PATH=$PATH:~/llvm-project make LLVM=1`. (or export it manually or via
your shell .rc, depending on how comfortable you are with that
version).

> Having both LLVM_DIR and LLVM_SUFFIX seems verbose.

I agree, so maybe just LLVM=y, and we can support both non-standard
locations and debian suffixes via modifications to PATH.

>
> In fact, the debian provides multiple versions of GCC.
> For example, my machine has
>
> masahiro@pug:~$ ls -1 /usr/bin/gcc-*
> /usr/bin/gcc-4.8
> /usr/bin/gcc-5
> /usr/bin/gcc-7
> /usr/bin/gcc-ar
> /usr/bin/gcc-ar-4.8
> /usr/bin/gcc-ar-5
> /usr/bin/gcc-ar-7
> /usr/bin/gcc-nm
> /usr/bin/gcc-nm-4.8
> /usr/bin/gcc-nm-5
> /usr/bin/gcc-nm-7
> /usr/bin/gcc-ranlib
> /usr/bin/gcc-ranlib-4.8
> /usr/bin/gcc-ranlib-5
> /usr/bin/gcc-ranlib-7
>
> But, nobody has suggested GCC_SUFFIX.
>
> So, I guess CROSS_COMPILE was enough to
> choose a specific tool version.

Or no one was testing specific versions of gcc with more than one
installed. I can ask the KernelCI folks next week if this is an issue
they face or have faced.
--
Thanks,
~Nick Desaulniers

[Nathan Chancellor]

Correct. This should probably be documented though, otherwise people
might use LLVM=y and be surprised when the latest version of their tools
are not being picked up.

> > I periodically build the latest llvm from the trunk,
> > and install it under my home directory.
> > So, I just thought it would be useful to
> > allow a user to specify the llvm directory.
> > Of course, I can do the equivalent by tweaking PATH, but
> > I hesitate to make the non-released version my default.
>
> Respectfully, I strongly disagree. This should be handled by
> modifications to $PATH, either by your shell's .rc file when you
> always want it, or exported for a session when you want it, or
> prefixed to an invocation for the duration of that command. We should
> not have a new variable just for the path of a few tools.
>
> Rather than `make LLVM_DIR=~/llvm-project LLVM=1`, you can do
> `PATH=$PATH:~/llvm-project make LLVM=1`. (or export it manually or via
> your shell .rc, depending on how comfortable you are with that
> version).

I always do PATH=...:${PATH} make CC=clang...

Well gcc is just one tool, so specified CC=gcc-5 is not that
complicated; it would get a lot more gnarly if one had different
versions of binutils as well.

Cheers,
Nathan

[Nick Desaulniers]

Have you had to test different releases of binutils yet? I have, and
it was not fun. I don't even remember what I did but I recall it
being painful trying to get it to work. (I think I finally solved it
via temporary symlink). Speaking of, I should get back to those
dwarf-5 patches I started, now that binutils devs implemented every
missing feature I could find.
--
Thanks,
~Nick Desaulniers

[Masahiro Yamada]

OK, so we will start with the boolean switch 'LLVM'.

People can use PATH to cope with directory path and suffixes.

[Nick Desaulniers]

Sounds good, we will modify our CI to use PATH modifications rather
than suffixes. We can even do that before such a patch to Makefile
exists.
--
Thanks,
~Nick Desaulniers

[Fangrui Song]

The proposed LLVM=1 + PATH scheme looks good to me.


There seems to be one issue.
OBJSIZE=llvm-objsize added in
commit fcf1b6a35c16ac500fa908a4022238e5d666eabf "Documentation/llvm: add documentation on building w/ Clang/LLVM"
is wrong.

The tool is named llvm-size. OBJSIZE is only used once:

arch/s390/scripts/Makefile.chkbss
14: if ! $(OBJSIZE) --common $< | $(AWK) 'END { if ($$3) exit 1 }'; then \

[Masahiro Yamada]

Good catch.

Could you send a patch, please?