* Re: [PATCH v2 01/39] kbuild: Fix clang build
[not found] ` <20220224151322.072632223@infradead.org>
@ 2022-03-01 21:16 ` Nick Desaulniers
2022-03-02 0:47 ` Kees Cook
2022-03-02 16:37 ` Nathan Chancellor
0 siblings, 2 replies; 10+ messages in thread
From: Nick Desaulniers @ 2022-03-01 21:16 UTC (permalink / raw)
To: Peter Zijlstra
Cc: x86, joao, hjl.tools, jpoimboe, andrew.cooper3, linux-kernel,
keescook, samitolvanen, mark.rutland, alyssa.milburn, mbenes,
rostedt, mhiramat, alexei.starovoitov, Masahiro Yamada,
Linux Kbuild mailing list, llvm, Nathan Chancellor
On Thu, Feb 24, 2022 at 7:17 AM Peter Zijlstra <peterz@infradead.org> 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) <peterz@infradead.org>
> ---
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-01 21:16 ` [PATCH v2 01/39] kbuild: Fix clang build Nick Desaulniers
@ 2022-03-02 0:47 ` Kees Cook
2022-03-02 0:53 ` Fangrui Song
2022-03-02 16:37 ` Nathan Chancellor
1 sibling, 1 reply; 10+ messages in thread
From: Kees Cook @ 2022-03-02 0:47 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Peter Zijlstra, x86, joao, hjl.tools, jpoimboe, andrew.cooper3,
linux-kernel, samitolvanen, mark.rutland, alyssa.milburn, mbenes,
rostedt, mhiramat, alexei.starovoitov, Masahiro Yamada,
Linux Kbuild mailing list, llvm, Nathan Chancellor
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-02 0:47 ` Kees Cook
@ 2022-03-02 0:53 ` Fangrui Song
0 siblings, 0 replies; 10+ messages in thread
From: Fangrui Song @ 2022-03-02 0:53 UTC (permalink / raw)
To: Kees Cook
Cc: Nick Desaulniers, Peter Zijlstra, x86, joao, hjl.tools, jpoimboe,
andrew.cooper3, linux-kernel, samitolvanen, mark.rutland,
alyssa.milburn, mbenes, rostedt, mhiramat, alexei.starovoitov,
Masahiro Yamada, Linux Kbuild mailing list, llvm,
Nathan Chancellor
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-01 21:16 ` [PATCH v2 01/39] kbuild: Fix clang build Nick Desaulniers
2022-03-02 0:47 ` Kees Cook
@ 2022-03-02 16:37 ` Nathan Chancellor
2022-03-02 18:40 ` Kees Cook
2022-03-02 19:18 ` Nick Desaulniers
1 sibling, 2 replies; 10+ messages in thread
From: Nathan Chancellor @ 2022-03-02 16:37 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Peter Zijlstra, x86, joao, hjl.tools, jpoimboe, andrew.cooper3,
linux-kernel, keescook, samitolvanen, mark.rutland,
alyssa.milburn, mbenes, rostedt, mhiramat, alexei.starovoitov,
Masahiro Yamada, Linux Kbuild mailing list, llvm
On Tue, Mar 01, 2022 at 01:16:04PM -0800, Nick Desaulniers wrote:
> On Thu, Feb 24, 2022 at 7:17 AM Peter Zijlstra <peterz@infradead.org> 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/YXqpFHeY26sEbort@hirez.programming.kicks-ass.net/
Cheers,
Nathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-02 16:37 ` Nathan Chancellor
@ 2022-03-02 18:40 ` Kees Cook
2022-03-02 19:18 ` Nick Desaulniers
1 sibling, 0 replies; 10+ messages in thread
From: Kees Cook @ 2022-03-02 18:40 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nick Desaulniers, Peter Zijlstra, x86, joao, hjl.tools, jpoimboe,
andrew.cooper3, linux-kernel, samitolvanen, mark.rutland,
alyssa.milburn, mbenes, rostedt, mhiramat, alexei.starovoitov,
Masahiro Yamada, Linux Kbuild mailing list, llvm
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-02 16:37 ` Nathan Chancellor
2022-03-02 18:40 ` Kees Cook
@ 2022-03-02 19:18 ` Nick Desaulniers
2022-03-02 21:15 ` Nathan Chancellor
1 sibling, 1 reply; 10+ messages in thread
From: Nick Desaulniers @ 2022-03-02 19:18 UTC (permalink / raw)
To: Nathan Chancellor, Peter Zijlstra, keescook
Cc: x86, joao, hjl.tools, jpoimboe, andrew.cooper3, linux-kernel,
samitolvanen, mark.rutland, alyssa.milburn, mbenes, rostedt,
mhiramat, alexei.starovoitov, Masahiro Yamada,
Linux Kbuild mailing list, llvm
On Wed, Mar 2, 2022 at 8:37 AM Nathan Chancellor <nathan@kernel.org> 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/YXqpFHeY26sEbort@hirez.programming.kicks-ass.net/
I'd be much more amenable to that approach.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-02 19:18 ` Nick Desaulniers
@ 2022-03-02 21:15 ` Nathan Chancellor
2022-03-02 22:07 ` Nick Desaulniers
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Nathan Chancellor @ 2022-03-02 21:15 UTC (permalink / raw)
To: Nick Desaulniers, Peter Zijlstra
Cc: keescook, x86, joao, hjl.tools, jpoimboe, andrew.cooper3,
linux-kernel, samitolvanen, mark.rutland, alyssa.milburn, mbenes,
rostedt, mhiramat, alexei.starovoitov, Masahiro Yamada,
Linux Kbuild mailing list, llvm
[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]
On Wed, Mar 02, 2022 at 11:18:07AM -0800, Nick Desaulniers wrote:
> On Wed, Mar 2, 2022 at 8:37 AM Nathan Chancellor <nathan@kernel.org> 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/YXqpFHeY26sEbort@hirez.programming.kicks-ass.net/
>
> 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
[-- Attachment #2: 0001-kbuild-Allow-a-suffix-with-LLVM.patch --]
[-- Type: text/plain, Size: 4675 bytes --]
From 83219caafbb7dbc2e41e3888ba5079d342aff633 Mon Sep 17 00:00:00 2001
From: Nathan Chancellor <nathan@kernel.org>
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 <nathan@kernel.org>
---
Documentation/kbuild/llvm.rst | 7 +++++++
Makefile | 24 ++++++++++++++----------
tools/scripts/Makefile.include | 20 ++++++++++++--------
3 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
index d32616891dcf..5805a8473a36 100644
--- a/Documentation/kbuild/llvm.rst
+++ b/Documentation/kbuild/llvm.rst
@@ -60,6 +60,13 @@ They can be enabled individually. The full list of the parameters: ::
OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
+If your LLVM tools have a suffix and you prefer to test an explicit version rather
+than the unsuffixed executables, use ``LLVM=<suffix>``. For example: ::
+
+ make LLVM=-14
+
+will use ``clang-14``, ``ld.lld-14``, etc.
+
The integrated assembler is enabled by default. You can pass ``LLVM_IAS=0`` to
disable it.
diff --git a/Makefile b/Makefile
index daeb5c88b50b..89b61e693258 100644
--- a/Makefile
+++ b/Makefile
@@ -424,8 +424,12 @@ 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++
+ifneq ($(LLVM),1)
+LLVM_SFX := $(LLVM)
+endif
+
+HOSTCC = clang$(LLVM_SFX)
+HOSTCXX = clang++$(LLVM_SFX)
else
HOSTCC = gcc
HOSTCXX = g++
@@ -443,14 +447,14 @@ KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
# 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
+CC = clang$(LLVM_SFX)
+LD = ld.lld$(LLVM_SFX)
+AR = llvm-ar$(LLVM_SFX)
+NM = llvm-nm$(LLVM_SFX)
+OBJCOPY = llvm-objcopy$(LLVM_SFX)
+OBJDUMP = llvm-objdump$(LLVM_SFX)
+READELF = llvm-readelf$(LLVM_SFX)
+STRIP = llvm-strip$(LLVM_SFX)
else
CC = $(CROSS_COMPILE)gcc
LD = $(CROSS_COMPILE)ld
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index 79d102304470..ab3b2a7dcc94 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -52,11 +52,15 @@ define allow-override
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)
+ifneq ($(LLVM),1)
+LLVM_SFX := $(LLVM)
+endif
+
+$(call allow-override,CC,clang$(LLVM_SFX))
+$(call allow-override,AR,llvm-ar$(LLVM_SFX))
+$(call allow-override,LD,ld.lld$(LLVM_SFX))
+$(call allow-override,CXX,clang++$(LLVM_SFX))
+$(call allow-override,STRIP,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)
@@ -69,9 +73,9 @@ 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
+HOSTAR ?= llvm-ar$(LLVM_SFX)
+HOSTCC ?= clang$(LLVM_SFX)
+HOSTLD ?= ld.lld$(LLVM_SFX)
else
HOSTAR ?= ar
HOSTCC ?= gcc
base-commit: fb184c4af9b9f4563e7a126219389986a71d5b5b
--
2.35.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-02 21:15 ` Nathan Chancellor
@ 2022-03-02 22:07 ` Nick Desaulniers
2022-03-02 23:00 ` Kees Cook
2022-03-02 23:10 ` Peter Zijlstra
2 siblings, 0 replies; 10+ messages in thread
From: Nick Desaulniers @ 2022-03-02 22:07 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Peter Zijlstra, keescook, x86, joao, hjl.tools, jpoimboe,
andrew.cooper3, linux-kernel, samitolvanen, mark.rutland,
alyssa.milburn, mbenes, rostedt, mhiramat, alexei.starovoitov,
Masahiro Yamada, Linux Kbuild mailing list, llvm
On Wed, Mar 2, 2022 at 1:15 PM Nathan Chancellor <nathan@kernel.org> 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 <ndesaulniers@google.com>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-02 21:15 ` Nathan Chancellor
2022-03-02 22:07 ` Nick Desaulniers
@ 2022-03-02 23:00 ` Kees Cook
2022-03-02 23:10 ` Peter Zijlstra
2 siblings, 0 replies; 10+ messages in thread
From: Kees Cook @ 2022-03-02 23:00 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nick Desaulniers, Peter Zijlstra, x86, joao, hjl.tools, jpoimboe,
andrew.cooper3, linux-kernel, samitolvanen, mark.rutland,
alyssa.milburn, mbenes, rostedt, mhiramat, alexei.starovoitov,
Masahiro Yamada, Linux Kbuild mailing list, llvm
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 <nathan@kernel.org>
> 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 <nathan@kernel.org>
I like it!
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 01/39] kbuild: Fix clang build
2022-03-02 21:15 ` Nathan Chancellor
2022-03-02 22:07 ` Nick Desaulniers
2022-03-02 23:00 ` Kees Cook
@ 2022-03-02 23:10 ` Peter Zijlstra
2 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2022-03-02 23:10 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nick Desaulniers, keescook, x86, joao, hjl.tools, jpoimboe,
andrew.cooper3, linux-kernel, samitolvanen, mark.rutland,
alyssa.milburn, mbenes, rostedt, mhiramat, alexei.starovoitov,
Masahiro Yamada, Linux Kbuild mailing list, llvm
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!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-03-03 0:10 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220224145138.952963315@infradead.org>
[not found] ` <20220224151322.072632223@infradead.org>
2022-03-01 21:16 ` [PATCH v2 01/39] kbuild: Fix clang build Nick Desaulniers
2022-03-02 0:47 ` Kees Cook
2022-03-02 0:53 ` Fangrui Song
2022-03-02 16:37 ` Nathan Chancellor
2022-03-02 18:40 ` Kees Cook
2022-03-02 19:18 ` Nick Desaulniers
2022-03-02 21:15 ` Nathan Chancellor
2022-03-02 22:07 ` Nick Desaulniers
2022-03-02 23:00 ` Kees Cook
2022-03-02 23:10 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox