Debian (and derived) distros ship their compilers as -$ver suffixed
binaries. For gcc it is sufficent to use:
$ make CC=gcc-12
However, clang builds (esp. clang-lto) need a whole array of tools to be
exactly right, leading to unweildy stuff like:
$ make CC=clang-13 LD=ld.lld-13 AR=llvm-ar-13 NM=llvm-nm-13 OBJCOPY=llvm-objcopy-13 OBJDUMP=llvm-objdump-13 READELF=llvm-readelf-13 STRIP=llvm-strip-13 LLVM=1
which is, quite franktly, totally insane and unusable. Instead make
the CC variable DTRT, enabling one such as myself to use:
$ make CC=clang-13
This also lets one quickly test different clang versions.
Additionally, also support path based LLVM suites like:
$ make CC=/opt/llvm/bin/clang
This changes the default to LLVM=1 when CC is clang, mixing toolchains
is still possible by explicitly adding LLVM=0.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
Makefile | 45 +++++++++++++++++++++++++++---------
tools/scripts/Makefile.include | 50 ++++++++++++++++++++++++++++-------------
2 files changed, 68 insertions(+), 27 deletions(-)
--- a/Makefile
+++ b/Makefile
@@ -423,9 +423,29 @@ HOST_LFS_CFLAGS := $(shell getconf LFS_C
HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS 2>/dev/null)
HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null)
-ifneq ($(LLVM),)
-HOSTCC = clang
-HOSTCXX = clang++
+# powerpc and s390 don't yet work with LLVM as a whole
+ifeq ($(ARCH),powerpc)
+LLVM = 0
+endif
+ifeq ($(ARCH),s390)
+LLVM = 0
+endif
+
+# otherwise, if CC=clang, default to using LLVM to enable LTO
+CC_BASE := $(shell echo $(CC) | sed 's/.*\///')
+CC_NAME := $(shell echo $(CC_BASE) | cut -b "1-5")
+ifeq ($(shell test "$(CC_NAME)" = "clang"; echo $$?),0)
+LLVM ?= 1
+LLVM_PFX := $(shell echo $(CC) | sed 's/\(.*\/\)\?.*/\1/')
+LLVM_SFX := $(shell echo $(CC_BASE) | cut -b "6-")
+endif
+
+# if not set by now, do not use LLVM
+LLVM ?= 0
+
+ifneq ($(LLVM),0)
+HOSTCC = $(LLVM_PFX)clang$(LLVM_SFX)
+HOSTCXX = $(LLVM_PFX)clang++$(LLVM_SFX)
else
HOSTCC = gcc
HOSTCXX = g++
@@ -442,15 +462,15 @@ KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS)
# Make variables (CC, etc...)
CPP = $(CC) -E
-ifneq ($(LLVM),)
-CC = clang
-LD = ld.lld
-AR = llvm-ar
-NM = llvm-nm
-OBJCOPY = llvm-objcopy
-OBJDUMP = llvm-objdump
-READELF = llvm-readelf
-STRIP = llvm-strip
+ifneq ($(LLVM),0)
+CC = $(LLVM_PFX)clang$(LLVM_SFX)
+LD = $(LLVM_PFX)ld.lld$(LLVM_SFX)
+AR = $(LLVM_PFX)llvm-ar$(LLVM_SFX)
+NM = $(LLVM_PFX)llvm-nm$(LLVM_SFX)
+OBJCOPY = $(LLVM_PFX)llvm-objcopy$(LLVM_SFX)
+OBJDUMP = $(LLVM_PFX)llvm-objdump$(LLVM_SFX)
+READELF = $(LLVM_PFX)llvm-readelf$(LLVM_SFX)
+STRIP = $(LLVM_PFX)llvm-strip$(LLVM_SFX)
else
CC = $(CROSS_COMPILE)gcc
LD = $(CROSS_COMPILE)ld
@@ -461,6 +481,7 @@ OBJDUMP = $(CROSS_COMPILE)objdump
READELF = $(CROSS_COMPILE)readelf
STRIP = $(CROSS_COMPILE)strip
endif
+
PAHOLE = pahole
RESOLVE_BTFIDS = $(objtree)/tools/bpf/resolve_btfids/resolve_btfids
LEX = flex
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -51,12 +51,32 @@ define allow-override
$(eval $(1) = $(2)))
endef
-ifneq ($(LLVM),)
-$(call allow-override,CC,clang)
-$(call allow-override,AR,llvm-ar)
-$(call allow-override,LD,ld.lld)
-$(call allow-override,CXX,clang++)
-$(call allow-override,STRIP,llvm-strip)
+# powerpc and s390 don't yet work with LLVM as a whole
+ifeq ($(ARCH),powerpc)
+LLVM = 0
+endif
+ifeq ($(ARCH),s390)
+LLVM = 0
+endif
+
+# otherwise, if CC=clang, default to using LLVM to enable LTO
+CC_BASE := $(shell echo $(CC) | sed 's/.*\///')
+CC_NAME := $(shell echo $(CC_BASE) | cut -b "1-5")
+ifeq ($(shell test "$(CC_NAME)" = "clang"; echo $$?),0)
+LLVM ?= 1
+LLVM_PFX := $(shell echo $(CC) | sed 's/\(.*\/\)\?.*/\1/')
+LLVM_SFX := $(shell echo $(CC_BASE) | cut -b "6-")
+endif
+
+# if not set by now, do not use LLVM
+LLVM ?= 0
+
+ifneq ($(LLVM),0)
+$(call allow-override,CC,$(LLVM_PFX)clang$(LLVM_SFX))
+$(call allow-override,AR,$(LLVM_PFX)llvm-ar$(LLVM_SFX))
+$(call allow-override,LD,$(LLVM_PFX)ld.lld$(LLVM_SFX))
+$(call allow-override,CXX,$(LLVM_PFX)clang++$(LLVM_SFX))
+$(call allow-override,STRIP,$(LLVM_PFX)llvm-strip$(LLVM_SFX))
else
# Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
$(call allow-override,CC,$(CROSS_COMPILE)gcc)
@@ -68,10 +88,10 @@ endif
CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
-ifneq ($(LLVM),)
-HOSTAR ?= llvm-ar
-HOSTCC ?= clang
-HOSTLD ?= ld.lld
+ifneq ($(LLVM),0)
+HOSTAR ?= $(LLVM_PFX)llvm-ar$(LLVM_SFX)
+HOSTCC ?= $(LLVM_PFX)clang$(LLVM_SFX)
+HOSTLD ?= $(LLVM_PFX)ld.lld$(LLVM_SFX)
else
HOSTAR ?= ar
HOSTCC ?= gcc
@@ -79,11 +99,11 @@ HOSTLD ?= ld
endif
# Some tools require Clang, LLC and/or LLVM utils
-CLANG ?= clang
-LLC ?= llc
-LLVM_CONFIG ?= llvm-config
-LLVM_OBJCOPY ?= llvm-objcopy
-LLVM_STRIP ?= llvm-strip
+CLANG ?= $(LLVM_PFX)clang$(LLVM_SFX)
+LLC ?= $(LLVM_PFX)llc$(LLVM_SFX)
+LLVM_CONFIG ?= $(LLVM_PFX)llvm-config$(LLVM_SFX)
+LLVM_OBJCOPY ?= $(LLVM_PFX)llvm-objcopy$(LLVM_SFX)
+LLVM_STRIP ?= $(LLVM_PFX)llvm-strip$(LLVM_SFX)
ifeq ($(CC_NO_CLANG), 1)
EXTRA_WARNINGS += -Wstrict-aliasing=3
nit: Subject maybe could be something more specific like:
"kbuild: Detect and apply clang version suffix to defaults"
On Thu, Feb 24, 2022 at 03:51:39PM +0100, Peter Zijlstra wrote:
> Debian (and derived) distros ship their compilers as -$ver suffixed
> binaries. For gcc it is sufficent to use:
>
> $ make CC=gcc-12
>
> However, clang builds (esp. clang-lto) need a whole array of tools to be
> exactly right, leading to unweildy stuff like:
Yeah, I have had this problem with gcc versions too (when trying to
select older compilers when cross compiling).
> [...]
> which is, quite franktly, totally insane and unusable. Instead make
> the CC variable DTRT, enabling one such as myself to use:
>
> $ make CC=clang-13
This form is intended to mix clang and binutils, as there is a long tail
of (usually architecture-specific) issues with ld.lld and Clang's
assembler. It's only relatively recently that clang + ld.lld works
happily on x86_64. Growing a way to split that logic by architecture
might be interesting, but also confusing...
> This also lets one quickly test different clang versions.
> Additionally, also support path based LLVM suites like:
>
> $ make CC=/opt/llvm/bin/clang
>
> This changes the default to LLVM=1 when CC is clang, mixing toolchains
> is still possible by explicitly adding LLVM=0.
I do like the path idea -- this is much cleaner in Clang's case than the
gcc/binutils mixture where a versioned binutils might not even exist in
a given environment. I've been fighting versioned cross compilers with
gcc and ld.bfd. It's almost worse. ;)
> [...]
> --- a/Makefile
> +++ b/Makefile
> @@ -423,9 +423,29 @@ HOST_LFS_CFLAGS := $(shell getconf LFS_C
> HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS 2>/dev/null)
> HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null)
>
> -ifneq ($(LLVM),)
> -HOSTCC = clang
> -HOSTCXX = clang++
> +# powerpc and s390 don't yet work with LLVM as a whole
Er, this, uh, doesn't really capture the matrix. ;) See
https://clangbuiltlinux.github.io/
> +ifeq ($(ARCH),powerpc)
> +LLVM = 0
> +endif
> +ifeq ($(ARCH),s390)
> +LLVM = 0
> +endif
> +
> +# otherwise, if CC=clang, default to using LLVM to enable LTO
I find this comment confusing: using LLVM=1 lets LTO be possible,
strictly speaking, it doesn't enable it.
> +CC_BASE := $(shell echo $(CC) | sed 's/.*\///')
I would expect $(shell basename $(CC)) (or similarly "dirname") for
these sorts of path manipulations, but I'll bet there's some Makefile
string syntax to do this too, to avoid $(shell) entirely...
> +CC_NAME := $(shell echo $(CC_BASE) | cut -b "1-5")
O_o cut -d- -f1
> +ifeq ($(shell test "$(CC_NAME)" = "clang"; echo $$?),0)
> +LLVM ?= 1
> +LLVM_PFX := $(shell echo $(CC) | sed 's/\(.*\/\)\?.*/\1/')
dirname
> +LLVM_SFX := $(shell echo $(CC_BASE) | cut -b "6-")
cut -d- -f2-
> +endif
This versioned suffix logic I'm fine with, though I'd prefer we gain
this for both Clang and GCC, as I've needed it there too, specifically
with the handling of CROSS_COMPILE:
$ make CROSS_COMPILE=/path/to/abi- CC=/path/to/abi-gcc-10 LD=/path/to/abi-ld-binutilsversion
Anyway, I guess that could be a separate patch.
> +# if not set by now, do not use LLVM
> +LLVM ?= 0
I think, however, the LLVM state needs to stay default=0. The "how to
build with Clang" already details the "how" on this:
https://www.kernel.org/doc/html/latest/kbuild/llvm.html
But I do agree: extracting the version suffix would make things much
easier.
Regardless, even if LLVM=1 is made the default, I think that should be
separate from the path and suffix logic.
Also, please CC the "CLANG/LLVM BUILD SUPPORT" maintainers in the
future:
M: Nathan Chancellor <[email protected]>
M: Nick Desaulniers <[email protected]>
L: [email protected]
S: Supported
W: https://clangbuiltlinux.github.io/
B: https://github.com/ClangBuiltLinux/linux/issues
C: irc://irc.libera.chat/clangbuiltlinux
F: Documentation/kbuild/llvm.rst
F: include/linux/compiler-clang.h
F: scripts/Makefile.clang
F: scripts/clang-tools/
K: \b(?i:clang|llvm)\b
--
Kees Cook
On 2022-03-01, Kees Cook wrote:
>On Tue, Mar 01, 2022 at 01:16:04PM -0800, Nick Desaulniers wrote:
>> Also, Kees mentions this is an issue for testing multiple different
>> versions of gcc, too. There perhaps is a way to simplify the builds
>> for BOTH toolchains; i.e. a yet-to-be-created shared variable denoting
>> the suffix for binaries? The primary pain point seems to be Debian's
>> suffixing scheme; it will suffix GCC, clang, and lld, but not GNU
>> binutils IIUC.
>
>Right. Though I think auto-detection still makes sense.
>
>If I do:
>
> make CC=clang-12 LLVM=1
>
>it'd be nice if it also used ld.lld-12.
This transformation may be a bit magical.
On Debian, /usr/bin/clang-13 is a symlink to /usr/lib/llvm-13/bin/clang .
Will it be fine for the user to provide correct feasible PATH?
>> [...]
>> Just curious, what prefixes have you observed in the wild?
>
>For me, where ever I built clang, and "/usr/bin"
>
>--
>Kees Cook
>
On Thu, Feb 24, 2022 at 7:17 AM Peter Zijlstra <[email protected]> wrote:
>
> Debian (and derived) distros ship their compilers as -$ver suffixed
> binaries. For gcc it is sufficent to use:
>
> $ make CC=gcc-12
>
> However, clang builds (esp. clang-lto) need a whole array of tools to be
> exactly right, leading to unweildy stuff like:
>
> $ make CC=clang-13 LD=ld.lld-13 AR=llvm-ar-13 NM=llvm-nm-13 OBJCOPY=llvm-objcopy-13 OBJDUMP=llvm-objdump-13 READELF=llvm-readelf-13 STRIP=llvm-strip-13 LLVM=1
>
> which is, quite franktly, totally insane and unusable. Instead make
> the CC variable DTRT, enabling one such as myself to use:
>
> $ make CC=clang-13
>
> This also lets one quickly test different clang versions.
> Additionally, also support path based LLVM suites like:
>
> $ make CC=/opt/llvm/bin/clang
>
> This changes the default to LLVM=1 when CC is clang, mixing toolchains
No, nack, we definitely do not want CC=clang to set LLVM=1. Those are
distinctly two different things for testing JUST the compiler
(CC=clang) vs the whole toolchain suite (LLVM=1). I do not wish to
change the semantics of those, and only for LLVM.
LLVM=1 means test clang, lld, llvm-objcopy, etc..
CC=clang means test clang, bfd, GNU objcopy, etc..
https://docs.kernel.org/kbuild/llvm.html#llvm-utilities
I don't wish to see the behavior of CC=clang change based on LLVM=0 being set.
> is still possible by explicitly adding LLVM=0.
Thanks for testing with LLVM, and even multiple versions of LLVM.
I'm still sympathetic, but only up to a point. A change like this MUST
CC the kbuild AND LLVM maintainers AND respective lists though. It
also has little to do with the rest of the series.
As per our previous discussion
https://lore.kernel.org/linux-kbuild/CAKwvOd=x9E=7WcCiieso-CDiiU-wMFcXL4W3V5j8dq7BL5QT+w@mail.gmail.com/
I'm still of the opionion that this should be solved by modifications
(permanent or one off) to one's $PATH.
To see what that would look like, let's test that out:
$ sudo apt install clang-11 lld-11
$ cd linux
$ dirname $(readlink -f $(which clang-11))
$ PATH=$(dirname $(readlink -f $(which clang-11))):$PATH make LLVM=1
-j72 -s allnoconfig all
$ llvm-readelf -p .comment vmlinux
String dump of section '.comment':
[ 0] Linker: LLD 11.1.0
[ 14] Debian clang version 11.1.0-4+build3
If that's too much for the command line, then add a shell function to
your shell's .rc file:
$ which make_clang
make_clang () {
ver=$1
shift
if ! [[ -n $(command -v clang-$ver) ]]
then
echo "clang-$ver not installed"
return 1
fi
PATH=$(dirname $(readlink -f $(which clang-$ver))):$PATH make CC=clang $@
}
$ make_clang 11 -j72 -s clean allnoconfig all
$ llvm-readelf -p .comment vmlinux
String dump of section '.comment':
[ 0] Debian clang version 11.1.0-4+build3
Even stuffing the dirname+readlink+which in a short helper fn would let you do:
$ make CC=$(helper clang-11)
Also, Kees mentions this is an issue for testing multiple different
versions of gcc, too. There perhaps is a way to simplify the builds
for BOTH toolchains; i.e. a yet-to-be-created shared variable denoting
the suffix for binaries? The primary pain point seems to be Debian's
suffixing scheme; it will suffix GCC, clang, and lld, but not GNU
binutils IIUC.
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> Makefile | 45 +++++++++++++++++++++++++++---------
> tools/scripts/Makefile.include | 50 ++++++++++++++++++++++++++++-------------
> 2 files changed, 68 insertions(+), 27 deletions(-)
>
> --- a/Makefile
> +++ b/Makefile
> @@ -423,9 +423,29 @@ HOST_LFS_CFLAGS := $(shell getconf LFS_C
> HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS 2>/dev/null)
> HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null)
>
> -ifneq ($(LLVM),)
> -HOSTCC = clang
> -HOSTCXX = clang++
> +# powerpc and s390 don't yet work with LLVM as a whole
> +ifeq ($(ARCH),powerpc)
> +LLVM = 0
> +endif
> +ifeq ($(ARCH),s390)
> +LLVM = 0
> +endif
> +
> +# otherwise, if CC=clang, default to using LLVM to enable LTO
> +CC_BASE := $(shell echo $(CC) | sed 's/.*\///')
> +CC_NAME := $(shell echo $(CC_BASE) | cut -b "1-5")
> +ifeq ($(shell test "$(CC_NAME)" = "clang"; echo $$?),0)
> +LLVM ?= 1
> +LLVM_PFX := $(shell echo $(CC) | sed 's/\(.*\/\)\?.*/\1/')
Just curious, what prefixes have you observed in the wild?
> +LLVM_SFX := $(shell echo $(CC_BASE) | cut -b "6-")
> +endif
> +
> +# if not set by now, do not use LLVM
> +LLVM ?= 0
> +
> +ifneq ($(LLVM),0)
> +HOSTCC = $(LLVM_PFX)clang$(LLVM_SFX)
> +HOSTCXX = $(LLVM_PFX)clang++$(LLVM_SFX)
> else
> HOSTCC = gcc
> HOSTCXX = g++
> @@ -442,15 +462,15 @@ KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS)
>
> # Make variables (CC, etc...)
> CPP = $(CC) -E
> -ifneq ($(LLVM),)
> -CC = clang
> -LD = ld.lld
> -AR = llvm-ar
> -NM = llvm-nm
> -OBJCOPY = llvm-objcopy
> -OBJDUMP = llvm-objdump
> -READELF = llvm-readelf
> -STRIP = llvm-strip
> +ifneq ($(LLVM),0)
> +CC = $(LLVM_PFX)clang$(LLVM_SFX)
> +LD = $(LLVM_PFX)ld.lld$(LLVM_SFX)
> +AR = $(LLVM_PFX)llvm-ar$(LLVM_SFX)
> +NM = $(LLVM_PFX)llvm-nm$(LLVM_SFX)
> +OBJCOPY = $(LLVM_PFX)llvm-objcopy$(LLVM_SFX)
> +OBJDUMP = $(LLVM_PFX)llvm-objdump$(LLVM_SFX)
> +READELF = $(LLVM_PFX)llvm-readelf$(LLVM_SFX)
> +STRIP = $(LLVM_PFX)llvm-strip$(LLVM_SFX)
> else
> CC = $(CROSS_COMPILE)gcc
> LD = $(CROSS_COMPILE)ld
> @@ -461,6 +481,7 @@ OBJDUMP = $(CROSS_COMPILE)objdump
> READELF = $(CROSS_COMPILE)readelf
> STRIP = $(CROSS_COMPILE)strip
> endif
> +
> PAHOLE = pahole
> RESOLVE_BTFIDS = $(objtree)/tools/bpf/resolve_btfids/resolve_btfids
> LEX = flex
> --- a/tools/scripts/Makefile.include
> +++ b/tools/scripts/Makefile.include
> @@ -51,12 +51,32 @@ define allow-override
> $(eval $(1) = $(2)))
> endef
>
> -ifneq ($(LLVM),)
> -$(call allow-override,CC,clang)
> -$(call allow-override,AR,llvm-ar)
> -$(call allow-override,LD,ld.lld)
> -$(call allow-override,CXX,clang++)
> -$(call allow-override,STRIP,llvm-strip)
> +# powerpc and s390 don't yet work with LLVM as a whole
> +ifeq ($(ARCH),powerpc)
> +LLVM = 0
> +endif
> +ifeq ($(ARCH),s390)
> +LLVM = 0
> +endif
> +
> +# otherwise, if CC=clang, default to using LLVM to enable LTO
> +CC_BASE := $(shell echo $(CC) | sed 's/.*\///')
> +CC_NAME := $(shell echo $(CC_BASE) | cut -b "1-5")
> +ifeq ($(shell test "$(CC_NAME)" = "clang"; echo $$?),0)
> +LLVM ?= 1
> +LLVM_PFX := $(shell echo $(CC) | sed 's/\(.*\/\)\?.*/\1/')
> +LLVM_SFX := $(shell echo $(CC_BASE) | cut -b "6-")
> +endif
> +
> +# if not set by now, do not use LLVM
> +LLVM ?= 0
> +
> +ifneq ($(LLVM),0)
> +$(call allow-override,CC,$(LLVM_PFX)clang$(LLVM_SFX))
> +$(call allow-override,AR,$(LLVM_PFX)llvm-ar$(LLVM_SFX))
> +$(call allow-override,LD,$(LLVM_PFX)ld.lld$(LLVM_SFX))
> +$(call allow-override,CXX,$(LLVM_PFX)clang++$(LLVM_SFX))
> +$(call allow-override,STRIP,$(LLVM_PFX)llvm-strip$(LLVM_SFX))
> else
> # Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
> $(call allow-override,CC,$(CROSS_COMPILE)gcc)
> @@ -68,10 +88,10 @@ endif
>
> CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
>
> -ifneq ($(LLVM),)
> -HOSTAR ?= llvm-ar
> -HOSTCC ?= clang
> -HOSTLD ?= ld.lld
> +ifneq ($(LLVM),0)
> +HOSTAR ?= $(LLVM_PFX)llvm-ar$(LLVM_SFX)
> +HOSTCC ?= $(LLVM_PFX)clang$(LLVM_SFX)
> +HOSTLD ?= $(LLVM_PFX)ld.lld$(LLVM_SFX)
> else
> HOSTAR ?= ar
> HOSTCC ?= gcc
> @@ -79,11 +99,11 @@ HOSTLD ?= ld
> endif
>
> # Some tools require Clang, LLC and/or LLVM utils
> -CLANG ?= clang
> -LLC ?= llc
> -LLVM_CONFIG ?= llvm-config
> -LLVM_OBJCOPY ?= llvm-objcopy
> -LLVM_STRIP ?= llvm-strip
> +CLANG ?= $(LLVM_PFX)clang$(LLVM_SFX)
> +LLC ?= $(LLVM_PFX)llc$(LLVM_SFX)
> +LLVM_CONFIG ?= $(LLVM_PFX)llvm-config$(LLVM_SFX)
> +LLVM_OBJCOPY ?= $(LLVM_PFX)llvm-objcopy$(LLVM_SFX)
> +LLVM_STRIP ?= $(LLVM_PFX)llvm-strip$(LLVM_SFX)
>
> ifeq ($(CC_NO_CLANG), 1)
> EXTRA_WARNINGS += -Wstrict-aliasing=3
>
>
--
Thanks,
~Nick Desaulniers
On Wed, Mar 02, 2022 at 09:37:04AM -0700, Nathan Chancellor wrote:
> However, I think we could still address Peter's complaint of "there
> should be an easier way for me to use the tools that are already in my
> PATH" with his first iteration of this patch [1], which I feel is
> totally reasonable:
>
> $ make LLVM=-14
>
> It is still easy to use (in fact, it is shorter than 'CC=clang-14') and
> it does not change anything else about how we build with LLVM. We would
> just have to add something along the lines of
>
> "If your LLVM tools have a suffix like Debian's (clang-14, ld.lld-14,
> etc.), use LLVM=<suffix>.
>
> $ make LLVM=-14"
>
> to Documentation/kbuild/llvm.rst.
>
> I might change the patch not to be so clever though:
>
> ifneq ($(LLVM),)
> ifneq ($(LLVM),1)
> LLVM_SFX := $(LLVM)
> endif
> endif
I like this idea! I think it's much easier to control than PATH (though
I see the rationale there too).
--
Kees Cook
On Wed, Mar 2, 2022 at 1:15 PM Nathan Chancellor <[email protected]> wrote:
>
> On Wed, Mar 02, 2022 at 11:18:07AM -0800, Nick Desaulniers wrote:
> > I'd be much more amenable to that approach.
>
> Sounds good, tentative patch attached, it passes all of my testing.
> There is an instance of $(LLVM) in tools/testing/selftests/lib.mk that I
> did not touch, as that will presumably have to go through the selftests
> tree. I can send a separate patch for that later.
LGTM!
Reviewed-by: Nick Desaulniers <[email protected]>
--
Thanks,
~Nick Desaulniers
On Wed, Mar 2, 2022 at 8:37 AM Nathan Chancellor <[email protected]> wrote:
>
> On Tue, Mar 01, 2022 at 01:16:04PM -0800, Nick Desaulniers wrote:
> > As per our previous discussion
> > https://lore.kernel.org/linux-kbuild/CAKwvOd=x9E=7WcCiieso-CDiiU-wMFcXL4W3V5j8dq7BL5QT+w@mail.gmail.com/
> > I'm still of the opionion that this should be solved by modifications
> > (permanent or one off) to one's $PATH.
>
> However, I think we could still address Peter's complaint of "there
> should be an easier way for me to use the tools that are already in my
> PATH" with his first iteration of this patch [1], which I feel is
> totally reasonable:
>
> $ make LLVM=-14
>
> It is still easy to use (in fact, it is shorter than 'CC=clang-14') and
> it does not change anything else about how we build with LLVM. We would
> just have to add something along the lines of
>
> "If your LLVM tools have a suffix like Debian's (clang-14, ld.lld-14,
> etc.), use LLVM=<suffix>.
"If your LLVM tools have a suffix and you prefer to test an explicit
version rather than the unsuffixed executables ..."
>
> $ make LLVM=-14"
>
> to Documentation/kbuild/llvm.rst.
>
> I might change the patch not to be so clever though:
>
> ifneq ($(LLVM),)
> ifneq ($(LLVM),1)
> LLVM_SFX := $(LLVM)
> endif
> endif
>
> [1]: https://lore.kernel.org/r/[email protected]/
I'd be much more amenable to that approach.
--
Thanks,
~Nick Desaulniers
On Tue, Mar 01, 2022 at 01:16:04PM -0800, Nick Desaulniers wrote:
> On Thu, Feb 24, 2022 at 7:17 AM Peter Zijlstra <[email protected]> wrote:
> >
> > Debian (and derived) distros ship their compilers as -$ver suffixed
> > binaries. For gcc it is sufficent to use:
> >
> > $ make CC=gcc-12
> >
> > However, clang builds (esp. clang-lto) need a whole array of tools to be
> > exactly right, leading to unweildy stuff like:
> >
> > $ make CC=clang-13 LD=ld.lld-13 AR=llvm-ar-13 NM=llvm-nm-13 OBJCOPY=llvm-objcopy-13 OBJDUMP=llvm-objdump-13 READELF=llvm-readelf-13 STRIP=llvm-strip-13 LLVM=1
> >
> > which is, quite franktly, totally insane and unusable. Instead make
> > the CC variable DTRT, enabling one such as myself to use:
> >
> > $ make CC=clang-13
> >
> > This also lets one quickly test different clang versions.
> > Additionally, also support path based LLVM suites like:
> >
> > $ make CC=/opt/llvm/bin/clang
> >
> > This changes the default to LLVM=1 when CC is clang, mixing toolchains
>
> No, nack, we definitely do not want CC=clang to set LLVM=1. Those are
> distinctly two different things for testing JUST the compiler
> (CC=clang) vs the whole toolchain suite (LLVM=1). I do not wish to
> change the semantics of those, and only for LLVM.
I agree with this. CC is only changing the compiler, not any of the
other build utilities. CC=gcc-12 works for GCC because you are only
using a different compiler, not an entirely new toolchain (as binutils
will be the same as just CC=gcc).
> LLVM=1 means test clang, lld, llvm-objcopy, etc..
> CC=clang means test clang, bfd, GNU objcopy, etc..
> https://docs.kernel.org/kbuild/llvm.html#llvm-utilities
>
> I don't wish to see the behavior of CC=clang change based on LLVM=0 being set.
>
> > is still possible by explicitly adding LLVM=0.
>
> Thanks for testing with LLVM, and even multiple versions of LLVM.
>
> I'm still sympathetic, but only up to a point. A change like this MUST
> CC the kbuild AND LLVM maintainers AND respective lists though. It
> also has little to do with the rest of the series.
>
> As per our previous discussion
> https://lore.kernel.org/linux-kbuild/CAKwvOd=x9E=7WcCiieso-CDiiU-wMFcXL4W3V5j8dq7BL5QT+w@mail.gmail.com/
> I'm still of the opionion that this should be solved by modifications
> (permanent or one off) to one's $PATH.
However, I think we could still address Peter's complaint of "there
should be an easier way for me to use the tools that are already in my
PATH" with his first iteration of this patch [1], which I feel is
totally reasonable:
$ make LLVM=-14
It is still easy to use (in fact, it is shorter than 'CC=clang-14') and
it does not change anything else about how we build with LLVM. We would
just have to add something along the lines of
"If your LLVM tools have a suffix like Debian's (clang-14, ld.lld-14,
etc.), use LLVM=<suffix>.
$ make LLVM=-14"
to Documentation/kbuild/llvm.rst.
I might change the patch not to be so clever though:
ifneq ($(LLVM),)
ifneq ($(LLVM),1)
LLVM_SFX := $(LLVM)
endif
endif
[1]: https://lore.kernel.org/r/[email protected]/
Cheers,
Nathan
On Wed, Mar 02, 2022 at 11:18:07AM -0800, Nick Desaulniers wrote:
> On Wed, Mar 2, 2022 at 8:37 AM Nathan Chancellor <[email protected]> wrote:
> >
> > On Tue, Mar 01, 2022 at 01:16:04PM -0800, Nick Desaulniers wrote:
> > > As per our previous discussion
> > > https://lore.kernel.org/linux-kbuild/CAKwvOd=x9E=7WcCiieso-CDiiU-wMFcXL4W3V5j8dq7BL5QT+w@mail.gmail.com/
> > > I'm still of the opionion that this should be solved by modifications
> > > (permanent or one off) to one's $PATH.
> >
> > However, I think we could still address Peter's complaint of "there
> > should be an easier way for me to use the tools that are already in my
> > PATH" with his first iteration of this patch [1], which I feel is
> > totally reasonable:
> >
> > $ make LLVM=-14
> >
> > It is still easy to use (in fact, it is shorter than 'CC=clang-14') and
> > it does not change anything else about how we build with LLVM. We would
> > just have to add something along the lines of
> >
> > "If your LLVM tools have a suffix like Debian's (clang-14, ld.lld-14,
> > etc.), use LLVM=<suffix>.
>
> "If your LLVM tools have a suffix and you prefer to test an explicit
> version rather than the unsuffixed executables ..."
Ack, I included this.
> > $ make LLVM=-14"
> >
> > to Documentation/kbuild/llvm.rst.
> >
> > I might change the patch not to be so clever though:
> >
> > ifneq ($(LLVM),)
> > ifneq ($(LLVM),1)
> > LLVM_SFX := $(LLVM)
> > endif
> > endif
> >
> > [1]: https://lore.kernel.org/r/[email protected]/
>
> I'd be much more amenable to that approach.
Sounds good, tentative patch attached, it passes all of my testing.
There is an instance of $(LLVM) in tools/testing/selftests/lib.mk that I
did not touch, as that will presumably have to go through the selftests
tree. I can send a separate patch for that later.
Peter, is this approach okay with you? If so, would you like to be
co-author or should I use a suggested-by tag?
Cheers,
Nathan
On Tue, Mar 01, 2022 at 01:16:04PM -0800, Nick Desaulniers wrote:
> Also, Kees mentions this is an issue for testing multiple different
> versions of gcc, too. There perhaps is a way to simplify the builds
> for BOTH toolchains; i.e. a yet-to-be-created shared variable denoting
> the suffix for binaries? The primary pain point seems to be Debian's
> suffixing scheme; it will suffix GCC, clang, and lld, but not GNU
> binutils IIUC.
Right. Though I think auto-detection still makes sense.
If I do:
make CC=clang-12 LLVM=1
it'd be nice if it also used ld.lld-12.
> [...]
> Just curious, what prefixes have you observed in the wild?
For me, where ever I built clang, and "/usr/bin"
--
Kees Cook
On Wed, Mar 02, 2022 at 02:15:45PM -0700, Nathan Chancellor wrote:
> Sounds good, tentative patch attached, it passes all of my testing.
> There is an instance of $(LLVM) in tools/testing/selftests/lib.mk that I
> did not touch, as that will presumably have to go through the selftests
> tree. I can send a separate patch for that later.
I think it's fine to include that here, just to keep the logic together.
> Peter, is this approach okay with you? If so, would you like to be
> co-author or should I use a suggested-by tag?
>
> Cheers,
> Nathan
> From 83219caafbb7dbc2e41e3888ba5079d342aff633 Mon Sep 17 00:00:00 2001
> From: Nathan Chancellor <[email protected]>
> Date: Wed, 2 Mar 2022 13:28:14 -0700
> Subject: [PATCH] kbuild: Allow a suffix with $(LLVM)
>
> The LLVM variable allows a developer to quickly switch between the GNU
> and LLVM tools. However, it does not handle versioned binaries, such as
> the ones shipped by Debian, as LLVM=1 just defines the build variables
> with the unversioned binaries.
>
> There was some discussion during the review of the patch that introduces
> LLVM=1 around this, ultimately coming to the conclusion that developers
> can just add the folder that contains the unversioned binaries to their
> PATH, as Debian's versioned suffixed binaries are really just symlinks
> to the unversioned binaries in /usr/lib/llvm-#/bin:
>
> $ realpath /usr/bin/clang-14
> /usr/lib/llvm-14/bin/clang
>
> $ PATH=/usr/lib/llvm-14/bin:$PATH make ... LLVM=1
>
> However, it is simple enough to support this scheme directly in the
> Kbuild system by allowing the developer to specify the version suffix
> with LLVM=, which is shorter than the above suggestion:
>
> $ make ... LLVM=-14
>
> It does not change the meaning of LLVM=1 (which will continue to use
> unversioned binaries) and it does not add too much additional complexity
> to the existing $(LLVM) code, while allowing developers to quickly test
> their series with different versions of the whole LLVM suite of tools.
>
> Signed-off-by: Nathan Chancellor <[email protected]>
I like it!
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Wed, Mar 02, 2022 at 02:15:45PM -0700, Nathan Chancellor wrote:
> Peter, is this approach okay with you? If so, would you like to be
> co-author or should I use a suggested-by tag?
Yeah, this helps lots. Suggested-by seems fine. Thanks!