Re: [PATCH v2 01/39] kbuild: Fix clang build

[Nathan Chancellor <nathan@kernel.org>]

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