[PATCH v2] kbuild: Make $(LLVM) more flexible

[PATCH v2] kbuild: Make $(LLVM) more flexible

[Nathan Chancellor]

The LLVM make 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 tool variables
with the unversioned binaries.

There was some discussion during the review of the patch that introduces
LLVM=1 around versioned binaries, 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, that can be cumbersome to developers who are constantly testing
series with different toolchains and versions. It is simple enough to
support these versioned binaries 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.

Some developers may build LLVM from source but not add the binaries to
their PATH, as they may not want to use that toolchain systemwide.
Support those developers by allowing them to supply the directory that
the LLVM tools are available in, as it is no more complex to support
than the version suffix change above.

$ make ... LLVM=/path/to/llvm/

Update and reorder the documentation to reflect these new additions.
At the same time, notate that LLVM=0 is not the same as just omitting it
altogether, which has confused people in the past.

Link: https://lore.kernel.org/r/20200317215515.226917-1-ndesaulniers@google.com/
Link: https://lore.kernel.org/r/20220224151322.072632223@infradead.org/
Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---

v1 -> v2: https://lore.kernel.org/r/20220302234400.782002-1-nathan@kernel.org/

* Drop Kees and Nick's reviewed-by tags, as there is a substantial logic
  update.

* Only support dash-based suffixes (Masahiro).

* Support passing a path to LLVM, which will be prefixed onto the
  tools (Masahiro).

* Reorder and update documentation for LLVM (Masahiro).

* Reword commit message to reflect new changes.

RFC -> v1: https://lore.kernel.org/r/Yh%2FegU1LZudfrgVy@dev-arch.thelio-3990X/

* Tidy up commit message slightly.

* Add tags.

* Add links to prior discussions for context.

* Add change to tools/testing/selftests/lib.mk.

I would like for this to go through the Kbuild tree, please ack as
necessary.

 Documentation/kbuild/llvm.rst  | 31 +++++++++++++++++++++++++------
 Makefile                       | 26 ++++++++++++++++----------
 tools/scripts/Makefile.include | 22 ++++++++++++++--------
 tools/testing/selftests/lib.mk |  8 +++++++-
 4 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
index d32616891dcf..68b74416ec48 100644
--- a/Documentation/kbuild/llvm.rst
+++ b/Documentation/kbuild/llvm.rst
@@ -49,17 +49,36 @@ example: ::
 LLVM Utilities
 --------------
 
-LLVM has substitutes for GNU binutils utilities. Kbuild supports ``LLVM=1``
-to enable them. ::
-
-	make LLVM=1
-
-They can be enabled individually. The full list of the parameters: ::
+LLVM has substitutes for GNU binutils utilities. They can be enabled individually.
+The full list of supported make variables: ::
 
 	make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
 	  OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
 	  HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
 
+To simplify the above command, Kbuild supports the ``LLVM`` variable: ::
+
+	make LLVM=1
+
+If your LLVM tools are not available in your PATH, you can supply their
+location using the LLVM variable with a trailing slash: ::
+
+	make LLVM=/path/to/llvm/
+
+which will use ``/path/to/llvm/clang``, ``/path/to/llvm/ld.lld``, etc.
+
+If your LLVM tools have a version suffix and you want to test with that
+explicit version rather than the unsuffixed executables like ``LLVM=1``, you
+can pass the suffix using the ``LLVM`` variable: ::
+
+	make LLVM=-14
+
+which will use ``clang-14``, ``ld.lld-14``, etc.
+
+``LLVM=0`` is not the same as omitting ``LLVM`` altogether, it will behave like
+``LLVM=1``. If you only wish to use certain LLVM utilities, use their respective
+make variables.
+
 The integrated assembler is enabled by default. You can pass ``LLVM_IAS=0`` to
 disable it.
 
diff --git a/Makefile b/Makefile
index a82095c69fdd..b3fb32fd3906 100644
--- a/Makefile
+++ b/Makefile
@@ -424,8 +424,14 @@ 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 ($(filter %/,$(LLVM)),)
+LLVM_PREFIX := $(LLVM)
+else ifneq ($(filter -%,$(LLVM)),)
+LLVM_SUFFIX := $(LLVM)
+endif
+
+HOSTCC	= $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
+HOSTCXX	= $(LLVM_PREFIX)clang++$(LLVM_SUFFIX)
 else
 HOSTCC	= gcc
 HOSTCXX	= g++
@@ -444,14 +450,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		= $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
+LD		= $(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX)
+AR		= $(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX)
+NM		= $(LLVM_PREFIX)llvm-nm$(LLVM_SUFFIX)
+OBJCOPY		= $(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX)
+OBJDUMP		= $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
+READELF		= $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
+STRIP		= $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
 else
 CC		= $(CROSS_COMPILE)gcc
 LD		= $(CROSS_COMPILE)ld
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index 79d102304470..b507a6a733f5 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -52,11 +52,17 @@ 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 ($(filter %/,$(LLVM)),)
+LLVM_PREFIX := $(LLVM)
+else ifneq ($(filter -%,$(LLVM)),)
+LLVM_SUFFIX := $(LLVM)
+endif
+
+$(call allow-override,CC,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
+$(call allow-override,AR,$(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX))
+$(call allow-override,LD,$(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX))
+$(call allow-override,CXX,$(LLVM_PREFIX)clang++$(LLVM_SUFFIX))
+$(call allow-override,STRIP,$(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX))
 else
 # Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
 $(call allow-override,CC,$(CROSS_COMPILE)gcc)
@@ -69,9 +75,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_PREFIX)llvm-ar$(LLVM_SUFFIX)
+HOSTCC  ?= $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
+HOSTLD  ?= $(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX)
 else
 HOSTAR  ?= ar
 HOSTCC  ?= gcc
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index a40add31a2e3..2a2d240cdc1b 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -1,7 +1,13 @@
 # This mimics the top-level Makefile. We do it explicitly here so that this
 # Makefile can operate with or without the kbuild infrastructure.
 ifneq ($(LLVM),)
-CC := clang
+ifneq ($(filter %/,$(LLVM)),)
+LLVM_PREFIX := $(LLVM)
+else ifneq ($(filter -%,$(LLVM)),)
+LLVM_SUFFIX := $(LLVM)
+endif
+
+CC := $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
 else
 CC := $(CROSS_COMPILE)gcc
 endif

base-commit: d42118db5e7bea017cf9d7c122c7ca3b89a51421
-- 
2.35.1

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Kees Cook]

On Fri, Mar 04, 2022 at 10:08:14AM -0700, Nathan Chancellor wrote:
> [...]
> 
> Update and reorder the documentation to reflect these new additions.
> At the same time, notate that LLVM=0 is not the same as just omitting it
> altogether, which has confused people in the past.

Is it worth making LLVM=0 actually act the way it's expected to?

> Link: https://lore.kernel.org/r/20200317215515.226917-1-ndesaulniers@google.com/
> Link: https://lore.kernel.org/r/20220224151322.072632223@infradead.org/
> Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Looks good; minor .rst nit below...

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

> [...]
> -LLVM has substitutes for GNU binutils utilities. Kbuild supports ``LLVM=1``
> -to enable them. ::
> -
> -	make LLVM=1
> -
> -They can be enabled individually. The full list of the parameters: ::
> +LLVM has substitutes for GNU binutils utilities. They can be enabled individually.
> +The full list of supported make variables: ::

": ::" and "::" yield the same result. I think the latter is more
readable in non-rendered form. *shrug*

-Kees

-- 
Kees Cook

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Nathan Chancellor]

On Fri, Mar 04, 2022 at 10:09:03AM -0800, Kees Cook wrote:
> On Fri, Mar 04, 2022 at 10:08:14AM -0700, Nathan Chancellor wrote:
> > [...]
> > 
> > Update and reorder the documentation to reflect these new additions.
> > At the same time, notate that LLVM=0 is not the same as just omitting it
> > altogether, which has confused people in the past.
> 
> Is it worth making LLVM=0 actually act the way it's expected to?

I don't really see the point, omitting $(LLVM) altogether is simpler.
Why specify LLVM=0 if you want GNU tools, since it is the default?
However, I can look into changing that in a new revision or a follow up
if others disagree?

> > Link: https://lore.kernel.org/r/20200317215515.226917-1-ndesaulniers@google.com/
> > Link: https://lore.kernel.org/r/20220224151322.072632223@infradead.org/
> > Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> Looks good; minor .rst nit below...
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> > [...]
> > -LLVM has substitutes for GNU binutils utilities. Kbuild supports ``LLVM=1``
> > -to enable them. ::
> > -
> > -	make LLVM=1
> > -
> > -They can be enabled individually. The full list of the parameters: ::
> > +LLVM has substitutes for GNU binutils utilities. They can be enabled individually.
> > +The full list of supported make variables: ::
> 
> ": ::" and "::" yield the same result. I think the latter is more
> readable in non-rendered form. *shrug*

Ack, I'll wait for other feedback before sending v3, unless there is
none and Masahiro does not mind fixing it up during application.

Thanks for the review!
Nathan

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Masahiro Yamada]

On Sat, Mar 5, 2022 at 3:15 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Fri, Mar 04, 2022 at 10:09:03AM -0800, Kees Cook wrote:
> > On Fri, Mar 04, 2022 at 10:08:14AM -0700, Nathan Chancellor wrote:
> > > [...]
> > >
> > > Update and reorder the documentation to reflect these new additions.
> > > At the same time, notate that LLVM=0 is not the same as just omitting it
> > > altogether, which has confused people in the past.
> >
> > Is it worth making LLVM=0 actually act the way it's expected to?
>
> I don't really see the point, omitting $(LLVM) altogether is simpler.
> Why specify LLVM=0 if you want GNU tools, since it is the default?
> However, I can look into changing that in a new revision or a follow up
> if others disagree?


Changing the meaning of LLVM=0 is beyond the scope of what
we are trying to achieve now.

I think documenting it is enough.

(If we have a good reason to change it, we can. But, it should be
done in a separate patch, at least)






-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Nick Desaulniers]

On Fri, Mar 4, 2022 at 9:14 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> index d32616891dcf..68b74416ec48 100644
> --- a/Documentation/kbuild/llvm.rst
> +++ b/Documentation/kbuild/llvm.rst
> @@ -49,17 +49,36 @@ example: ::
>  LLVM Utilities
>  --------------
>
> -LLVM has substitutes for GNU binutils utilities. Kbuild supports ``LLVM=1``
> -to enable them. ::
> -
> -       make LLVM=1
> -
> -They can be enabled individually. The full list of the parameters: ::
> +LLVM has substitutes for GNU binutils utilities. They can be enabled individually.
> +The full list of supported make variables: ::
>
>         make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
>           OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
>           HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
>
> +To simplify the above command, Kbuild supports the ``LLVM`` variable: ::
> +
> +       make LLVM=1
> +
> +If your LLVM tools are not available in your PATH, you can supply their
> +location using the LLVM variable with a trailing slash: ::
> +
> +       make LLVM=/path/to/llvm/
> +
> +which will use ``/path/to/llvm/clang``, ``/path/to/llvm/ld.lld``, etc.

I don't think we should do this; `PATH=/path/to/llvm/ make LLVM=1`
works and (my interpretation of what) Masahiro said "if anyone asks
for this, here's how we could do that."  I don't think I've seen an
explicit ask for that. I'd rather LLVM= have 2 behaviors than 3, but I
won't hold this patch up over that.  Either way:

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> +
> +If your LLVM tools have a version suffix and you want to test with that
> +explicit version rather than the unsuffixed executables like ``LLVM=1``, you
> +can pass the suffix using the ``LLVM`` variable: ::
> +
> +       make LLVM=-14
> +
> +which will use ``clang-14``, ``ld.lld-14``, etc.
> +
> +``LLVM=0`` is not the same as omitting ``LLVM`` altogether, it will behave like
> +``LLVM=1``.

Hmm... I can see someone's build wrappers setting LLVM=1, then them
being surprised that appending LLVM=0 doesn't disable LLVM=1 as they
might expect.  But Masahiro says let's fix this later which is fine.

--
Thanks,
~Nick Desaulniers

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Nathan Chancellor]

On Mon, Mar 07, 2022 at 11:08:29AM -0800, Nick Desaulniers wrote:
> On Fri, Mar 4, 2022 at 9:14 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> > index d32616891dcf..68b74416ec48 100644
> > --- a/Documentation/kbuild/llvm.rst
> > +++ b/Documentation/kbuild/llvm.rst
> > @@ -49,17 +49,36 @@ example: ::
> >  LLVM Utilities
> >  --------------
> >
> > -LLVM has substitutes for GNU binutils utilities. Kbuild supports ``LLVM=1``
> > -to enable them. ::
> > -
> > -       make LLVM=1
> > -
> > -They can be enabled individually. The full list of the parameters: ::
> > +LLVM has substitutes for GNU binutils utilities. They can be enabled individually.
> > +The full list of supported make variables: ::
> >
> >         make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
> >           OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
> >           HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
> >
> > +To simplify the above command, Kbuild supports the ``LLVM`` variable: ::
> > +
> > +       make LLVM=1
> > +
> > +If your LLVM tools are not available in your PATH, you can supply their
> > +location using the LLVM variable with a trailing slash: ::
> > +
> > +       make LLVM=/path/to/llvm/
> > +
> > +which will use ``/path/to/llvm/clang``, ``/path/to/llvm/ld.lld``, etc.
> 
> I don't think we should do this; `PATH=/path/to/llvm/ make LLVM=1`
> works and (my interpretation of what) Masahiro said "if anyone asks
> for this, here's how we could do that."  I don't think I've seen an
> explicit ask for that. I'd rather LLVM= have 2 behaviors than 3, but I
> won't hold this patch up over that.  Either way:

Right, there has not been an explicit ask for the prefix support yet,
although I know I personally would use it, but I think that it is worth
doing now instead of later for a few reasons:

1. It makes path goofs easier to spot. If you do

     $ PATH=/path/to/llvm:$PATH make LLVM=1 ...

   with a path to LLVM that does not exist (maybe you are bisecting an
   issue and using a temporary build of LLVM and you forgot the path it
   was in), you fall back to the LLVM tools that are in other places in
   your PATH, which is not what the developer intended. I know that I
   have messed up bisects that way. If you did

     $ make LLVM=/path/to/llvm/

   with a path that does not exist, there will be an error much earlier:

     $ make LLVM=/this/path/does/not/exist/ defconfig
     /bin/sh: line 1: /this/path/does/not/exist/clang: No such file or directory

2. It does not take that much more code or documentation to support. It
   is the same amount of code as the suffix and the documentation is
   roughly the same amount of lines as well.

3. If we wait to implement the path-based use of $(LLVM), we have three
   "sequence" points: the initial support of $(LLVM), the suffix
   support, and the prefix support. As we are constantly working with
   various trees, it would make it harder to know what to use when. If
   we just do it in the same patch, we know 5.18+ can use both of these
   methods.

However, at the end of the day, we are a team and if you feel like we
should only have suffix support, I am more than happy to push a v3 that
does just that and we can revist prefix support in the future. Just let
me know!

> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> > +
> > +If your LLVM tools have a version suffix and you want to test with that
> > +explicit version rather than the unsuffixed executables like ``LLVM=1``, you
> > +can pass the suffix using the ``LLVM`` variable: ::
> > +
> > +       make LLVM=-14
> > +
> > +which will use ``clang-14``, ``ld.lld-14``, etc.
> > +
> > +``LLVM=0`` is not the same as omitting ``LLVM`` altogether, it will behave like
> > +``LLVM=1``.
> 
> Hmm... I can see someone's build wrappers setting LLVM=1, then them
> being surprised that appending LLVM=0 doesn't disable LLVM=1 as they
> might expect.  But Masahiro says let's fix this later which is fine.

Sure, I guess that is a reasonable case to support. I'll see if I can
come up with something that makes sense after this change lands.

Cheers,
Nathan

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Matthew Wilcox]

On Mon, Mar 07, 2022 at 11:08:29AM -0800, Nick Desaulniers wrote:
> > +``LLVM=0`` is not the same as omitting ``LLVM`` altogether, it will behave like
> > +``LLVM=1``.
> 
> Hmm... I can see someone's build wrappers setting LLVM=1, then them
> being surprised that appending LLVM=0 doesn't disable LLVM=1 as they
> might expect.  But Masahiro says let's fix this later which is fine.

What happens if you say LLVM= instead of LLVM=0 ?  Would that "undo"
a prior LLVM=1 and use GCC instead?

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Masahiro Yamada]

On Wed, Mar 9, 2022 at 1:36 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Mar 07, 2022 at 11:08:29AM -0800, Nick Desaulniers wrote:
> > > +``LLVM=0`` is not the same as omitting ``LLVM`` altogether, it will behave like
> > > +``LLVM=1``.
> >
> > Hmm... I can see someone's build wrappers setting LLVM=1, then them
> > being surprised that appending LLVM=0 doesn't disable LLVM=1 as they
> > might expect.  But Masahiro says let's fix this later which is fine.
>
> What happens if you say LLVM= instead of LLVM=0 ?  Would that "undo"
> a prior LLVM=1 and use GCC instead?

I think so.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Masahiro Yamada]

On Wed, Mar 9, 2022 at 12:47 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Mon, Mar 07, 2022 at 11:08:29AM -0800, Nick Desaulniers wrote:
> > On Fri, Mar 4, 2022 at 9:14 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > >
> > > diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> > > index d32616891dcf..68b74416ec48 100644
> > > --- a/Documentation/kbuild/llvm.rst
> > > +++ b/Documentation/kbuild/llvm.rst
> > > @@ -49,17 +49,36 @@ example: ::
> > >  LLVM Utilities
> > >  --------------
> > >
> > > -LLVM has substitutes for GNU binutils utilities. Kbuild supports ``LLVM=1``
> > > -to enable them. ::
> > > -
> > > -       make LLVM=1
> > > -
> > > -They can be enabled individually. The full list of the parameters: ::
> > > +LLVM has substitutes for GNU binutils utilities. They can be enabled individually.
> > > +The full list of supported make variables: ::
> > >
> > >         make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
> > >           OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
> > >           HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
> > >
> > > +To simplify the above command, Kbuild supports the ``LLVM`` variable: ::
> > > +
> > > +       make LLVM=1
> > > +
> > > +If your LLVM tools are not available in your PATH, you can supply their
> > > +location using the LLVM variable with a trailing slash: ::
> > > +
> > > +       make LLVM=/path/to/llvm/
> > > +
> > > +which will use ``/path/to/llvm/clang``, ``/path/to/llvm/ld.lld``, etc.
> >
> > I don't think we should do this; `PATH=/path/to/llvm/ make LLVM=1`
> > works and (my interpretation of what) Masahiro said "if anyone asks
> > for this, here's how we could do that."  I don't think I've seen an
> > explicit ask for that. I'd rather LLVM= have 2 behaviors than 3, but I
> > won't hold this patch up over that.  Either way:
>
> Right, there has not been an explicit ask for the prefix support yet,
> although I know I personally would use it, but I think that it is worth
> doing now instead of later for a few reasons:
>
> 1. It makes path goofs easier to spot. If you do
>
>      $ PATH=/path/to/llvm:$PATH make LLVM=1 ...
>
>    with a path to LLVM that does not exist (maybe you are bisecting an
>    issue and using a temporary build of LLVM and you forgot the path it
>    was in), you fall back to the LLVM tools that are in other places in
>    your PATH, which is not what the developer intended. I know that I
>    have messed up bisects that way. If you did
>
>      $ make LLVM=/path/to/llvm/
>
>    with a path that does not exist, there will be an error much earlier:
>
>      $ make LLVM=/this/path/does/not/exist/ defconfig
>      /bin/sh: line 1: /this/path/does/not/exist/clang: No such file or directory
>
> 2. It does not take that much more code or documentation to support. It
>    is the same amount of code as the suffix and the documentation is
>    roughly the same amount of lines as well.
>
> 3. If we wait to implement the path-based use of $(LLVM), we have three
>    "sequence" points: the initial support of $(LLVM), the suffix
>    support, and the prefix support. As we are constantly working with
>    various trees, it would make it harder to know what to use when. If
>    we just do it in the same patch, we know 5.18+ can use both of these
>    methods.
>
> However, at the end of the day, we are a team and if you feel like we
> should only have suffix support, I am more than happy to push a v3 that
> does just that and we can revist prefix support in the future. Just let
> me know!


I do not have a strong opinion about this.
(I just mentioned the LLVM=/path/to/llvm/ form because I guessed
somebody would request this sooner or later.)


If you want me to pick up this version, I will apply it with fixing up
a nit pointed out by Kees   (": ::" -> "::")

If you want to send v3, that is fine with me as well.

Please let me know your thoughts.








>
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> >
> > > +
> > > +If your LLVM tools have a version suffix and you want to test with that
> > > +explicit version rather than the unsuffixed executables like ``LLVM=1``, you
> > > +can pass the suffix using the ``LLVM`` variable: ::
> > > +
> > > +       make LLVM=-14
> > > +
> > > +which will use ``clang-14``, ``ld.lld-14``, etc.
> > > +
> > > +``LLVM=0`` is not the same as omitting ``LLVM`` altogether, it will behave like
> > > +``LLVM=1``.
> >
> > Hmm... I can see someone's build wrappers setting LLVM=1, then them
> > being surprised that appending LLVM=0 doesn't disable LLVM=1 as they
> > might expect.  But Masahiro says let's fix this later which is fine.
>
> Sure, I guess that is a reasonable case to support. I'll see if I can
> come up with something that makes sense after this change lands.
>
> Cheers,
> Nathan



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Nick Desaulniers]

On Tue, Mar 8, 2022 at 7:47 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Mon, Mar 07, 2022 at 11:08:29AM -0800, Nick Desaulniers wrote:
> > On Fri, Mar 4, 2022 at 9:14 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > >
> > > +If your LLVM tools are not available in your PATH, you can supply their
> > > +location using the LLVM variable with a trailing slash: ::
> > > +
> > > +       make LLVM=/path/to/llvm/
> > > +
> > > +which will use ``/path/to/llvm/clang``, ``/path/to/llvm/ld.lld``, etc.
> >
> > I don't think we should do this; `PATH=/path/to/llvm/ make LLVM=1`
> > works and (my interpretation of what) Masahiro said "if anyone asks
> > for this, here's how we could do that."  I don't think I've seen an
> > explicit ask for that. I'd rather LLVM= have 2 behaviors than 3, but I
> > won't hold this patch up over that.  Either way:
>
> Right, there has not been an explicit ask for the prefix support yet,
> although I know I personally would use it,

Then let that be reason enough. :)
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Nathan Chancellor]

On Wed, Mar 09, 2022 at 06:33:40PM +0900, Masahiro Yamada wrote:
> On Wed, Mar 9, 2022 at 12:47 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > On Mon, Mar 07, 2022 at 11:08:29AM -0800, Nick Desaulniers wrote:
> > > On Fri, Mar 4, 2022 at 9:14 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > > >
> > > > diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> > > > index d32616891dcf..68b74416ec48 100644
> > > > --- a/Documentation/kbuild/llvm.rst
> > > > +++ b/Documentation/kbuild/llvm.rst
> > > > @@ -49,17 +49,36 @@ example: ::
> > > >  LLVM Utilities
> > > >  --------------
> > > >
> > > > -LLVM has substitutes for GNU binutils utilities. Kbuild supports ``LLVM=1``
> > > > -to enable them. ::
> > > > -
> > > > -       make LLVM=1
> > > > -
> > > > -They can be enabled individually. The full list of the parameters: ::
> > > > +LLVM has substitutes for GNU binutils utilities. They can be enabled individually.
> > > > +The full list of supported make variables: ::
> > > >
> > > >         make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
> > > >           OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
> > > >           HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
> > > >
> > > > +To simplify the above command, Kbuild supports the ``LLVM`` variable: ::
> > > > +
> > > > +       make LLVM=1
> > > > +
> > > > +If your LLVM tools are not available in your PATH, you can supply their
> > > > +location using the LLVM variable with a trailing slash: ::
> > > > +
> > > > +       make LLVM=/path/to/llvm/
> > > > +
> > > > +which will use ``/path/to/llvm/clang``, ``/path/to/llvm/ld.lld``, etc.
> > >
> > > I don't think we should do this; `PATH=/path/to/llvm/ make LLVM=1`
> > > works and (my interpretation of what) Masahiro said "if anyone asks
> > > for this, here's how we could do that."  I don't think I've seen an
> > > explicit ask for that. I'd rather LLVM= have 2 behaviors than 3, but I
> > > won't hold this patch up over that.  Either way:
> >
> > Right, there has not been an explicit ask for the prefix support yet,
> > although I know I personally would use it, but I think that it is worth
> > doing now instead of later for a few reasons:
> >
> > 1. It makes path goofs easier to spot. If you do
> >
> >      $ PATH=/path/to/llvm:$PATH make LLVM=1 ...
> >
> >    with a path to LLVM that does not exist (maybe you are bisecting an
> >    issue and using a temporary build of LLVM and you forgot the path it
> >    was in), you fall back to the LLVM tools that are in other places in
> >    your PATH, which is not what the developer intended. I know that I
> >    have messed up bisects that way. If you did
> >
> >      $ make LLVM=/path/to/llvm/
> >
> >    with a path that does not exist, there will be an error much earlier:
> >
> >      $ make LLVM=/this/path/does/not/exist/ defconfig
> >      /bin/sh: line 1: /this/path/does/not/exist/clang: No such file or directory
> >
> > 2. It does not take that much more code or documentation to support. It
> >    is the same amount of code as the suffix and the documentation is
> >    roughly the same amount of lines as well.
> >
> > 3. If we wait to implement the path-based use of $(LLVM), we have three
> >    "sequence" points: the initial support of $(LLVM), the suffix
> >    support, and the prefix support. As we are constantly working with
> >    various trees, it would make it harder to know what to use when. If
> >    we just do it in the same patch, we know 5.18+ can use both of these
> >    methods.
> >
> > However, at the end of the day, we are a team and if you feel like we
> > should only have suffix support, I am more than happy to push a v3 that
> > does just that and we can revist prefix support in the future. Just let
> > me know!
> 
> 
> I do not have a strong opinion about this.
> (I just mentioned the LLVM=/path/to/llvm/ form because I guessed
> somebody would request this sooner or later.)
> 
> 
> If you want me to pick up this version, I will apply it with fixing up
> a nit pointed out by Kees   (": ::" -> "::")
> 
> If you want to send v3, that is fine with me as well.
> 
> Please let me know your thoughts.

Given Nick's response, please pick up this revision with Kees' nit.
Thank you!

Cheers,
Nathan

> > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > >
> > > > +
> > > > +If your LLVM tools have a version suffix and you want to test with that
> > > > +explicit version rather than the unsuffixed executables like ``LLVM=1``, you
> > > > +can pass the suffix using the ``LLVM`` variable: ::
> > > > +
> > > > +       make LLVM=-14
> > > > +
> > > > +which will use ``clang-14``, ``ld.lld-14``, etc.
> > > > +
> > > > +``LLVM=0`` is not the same as omitting ``LLVM`` altogether, it will behave like
> > > > +``LLVM=1``.
> > >
> > > Hmm... I can see someone's build wrappers setting LLVM=1, then them
> > > being surprised that appending LLVM=0 doesn't disable LLVM=1 as they
> > > might expect.  But Masahiro says let's fix this later which is fine.
> >
> > Sure, I guess that is a reasonable case to support. I'll see if I can
> > come up with something that makes sense after this change lands.
> >
> > Cheers,
> > Nathan
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

Re: [PATCH v2] kbuild: Make $(LLVM) more flexible

[Masahiro Yamada]

On Fri, Mar 11, 2022 at 2:36 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Wed, Mar 09, 2022 at 06:33:40PM +0900, Masahiro Yamada wrote:
> > On Wed, Mar 9, 2022 at 12:47 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > >
> > > On Mon, Mar 07, 2022 at 11:08:29AM -0800, Nick Desaulniers wrote:
> > > > On Fri, Mar 4, 2022 at 9:14 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > > > >
> > > > > diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> > > > > index d32616891dcf..68b74416ec48 100644
> > > > > --- a/Documentation/kbuild/llvm.rst
> > > > > +++ b/Documentation/kbuild/llvm.rst
> > > > > @@ -49,17 +49,36 @@ example: ::
> > > > >  LLVM Utilities
> > > > >  --------------
> > > > >
> > > > > -LLVM has substitutes for GNU binutils utilities. Kbuild supports ``LLVM=1``
> > > > > -to enable them. ::
> > > > > -
> > > > > -       make LLVM=1
> > > > > -
> > > > > -They can be enabled individually. The full list of the parameters: ::
> > > > > +LLVM has substitutes for GNU binutils utilities. They can be enabled individually.
> > > > > +The full list of supported make variables: ::
> > > > >
> > > > >         make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
> > > > >           OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
> > > > >           HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
> > > > >
> > > > > +To simplify the above command, Kbuild supports the ``LLVM`` variable: ::
> > > > > +
> > > > > +       make LLVM=1
> > > > > +
> > > > > +If your LLVM tools are not available in your PATH, you can supply their
> > > > > +location using the LLVM variable with a trailing slash: ::
> > > > > +
> > > > > +       make LLVM=/path/to/llvm/
> > > > > +
> > > > > +which will use ``/path/to/llvm/clang``, ``/path/to/llvm/ld.lld``, etc.
> > > >
> > > > I don't think we should do this; `PATH=/path/to/llvm/ make LLVM=1`
> > > > works and (my interpretation of what) Masahiro said "if anyone asks
> > > > for this, here's how we could do that."  I don't think I've seen an
> > > > explicit ask for that. I'd rather LLVM= have 2 behaviors than 3, but I
> > > > won't hold this patch up over that.  Either way:
> > >
> > > Right, there has not been an explicit ask for the prefix support yet,
> > > although I know I personally would use it, but I think that it is worth
> > > doing now instead of later for a few reasons:
> > >
> > > 1. It makes path goofs easier to spot. If you do
> > >
> > >      $ PATH=/path/to/llvm:$PATH make LLVM=1 ...
> > >
> > >    with a path to LLVM that does not exist (maybe you are bisecting an
> > >    issue and using a temporary build of LLVM and you forgot the path it
> > >    was in), you fall back to the LLVM tools that are in other places in
> > >    your PATH, which is not what the developer intended. I know that I
> > >    have messed up bisects that way. If you did
> > >
> > >      $ make LLVM=/path/to/llvm/
> > >
> > >    with a path that does not exist, there will be an error much earlier:
> > >
> > >      $ make LLVM=/this/path/does/not/exist/ defconfig
> > >      /bin/sh: line 1: /this/path/does/not/exist/clang: No such file or directory
> > >
> > > 2. It does not take that much more code or documentation to support. It
> > >    is the same amount of code as the suffix and the documentation is
> > >    roughly the same amount of lines as well.
> > >
> > > 3. If we wait to implement the path-based use of $(LLVM), we have three
> > >    "sequence" points: the initial support of $(LLVM), the suffix
> > >    support, and the prefix support. As we are constantly working with
> > >    various trees, it would make it harder to know what to use when. If
> > >    we just do it in the same patch, we know 5.18+ can use both of these
> > >    methods.
> > >
> > > However, at the end of the day, we are a team and if you feel like we
> > > should only have suffix support, I am more than happy to push a v3 that
> > > does just that and we can revist prefix support in the future. Just let
> > > me know!
> >
> >
> > I do not have a strong opinion about this.
> > (I just mentioned the LLVM=/path/to/llvm/ form because I guessed
> > somebody would request this sooner or later.)
> >
> >
> > If you want me to pick up this version, I will apply it with fixing up
> > a nit pointed out by Kees   (": ::" -> "::")
> >
> > If you want to send v3, that is fine with me as well.
> >
> > Please let me know your thoughts.
>
> Given Nick's response, please pick up this revision with Kees' nit.
> Thank you!


I fixed up ": ::" to "::",
and applied to linux-kbuild.  Thanks.






-- 
Best Regards
Masahiro Yamada