On Fri, Mar 04, 2022 at 08:16:00PM +0900, Masahiro Yamada wrote: > On Thu, Mar 3, 2022 at 8:47 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > > > > 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. > > > > Link: https://lore.kernel.org/r/20200317215515.226917-1-ndesaulniers@xxxxxxxxxx/ > > Link: https://lore.kernel.org/r/20220224151322.072632223@xxxxxxxxxxxxx/ > > Suggested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > > Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> > > Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > > Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx> > > --- > > > > 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 | 7 +++++++ > > Makefile | 24 ++++++++++++++---------- > > tools/scripts/Makefile.include | 20 ++++++++++++-------- > > tools/testing/selftests/lib.mk | 6 +++++- > > 4 files changed, 38 insertions(+), 19 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. > > > Perhaps, it might be worth mentioning the difference between > LLVM=1 and LLVM=<suffix> > > The current behavior is, > any value other than '1' is regarded as a suffix. Maybe just adding something like: "... prefer to test an explicit version rather than the unsuffixed executables like above, ..." ? I'll try to come up with a clearer way to word everything for v2. > > diff --git a/Makefile b/Makefile > > index a82095c69fdd..963840c00eae 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 > > I am OK with this, but please note LLVM=0 > uses 'clang0', 'ld.lld0' instead of disabling > LLVM explicitly. > > This might be a small surprise because LLVM_IAS=0 > is used to disable the integrated assembler. Right, but we have that problem right now, as LLVM=0 is currently treated like LLVM=1. I suppose I could add a line to the documentation in a follow up change to clarify this. > If you want handle LLVM=<suffix> > only when <suffix> start with a hyphen, > you can do like this: > > ifneq ($(filter -%, $(LLVM)),) > LLVM_SFX := $(LLVM) > endif I did think about this. I guess the only reason I did not do that in this version is someone might have a different suffix scheme than Debian's but it is probably better to be a little bit more precise based on what we know in this moment. I will change it to that and update the documentation. > In the future, If somebody requests to support > make LLVM=/path/to/my/own/llvm/dir/ > to use llvm tools in that path, > we can expand the code like this: > > > > # "LLVM=foo/bar/" is a syntax sugar of "LLVM=1 LLVM_PFX=foo/bar" > # "LLVM=-foo" is a syntax sugar of "LLVM=1 LLVM_SFX=-foo" > > ifneq ($(filter %/, $(LLVM)),) > LLVM_PFX := $(LLVM) > else ifneq ($(filter -%, $(LLVM)),) > LLVM_SFX := $(LLVM) > endif I know I personally I would use the prefix form when testing with LLVM=1 so I think I will just go ahead and support that now, especially since Peter had aimed to support a full path with his CC patch that we NAK'd. > Lastly, I personally prefer to fully spell LLVM_SUFFIX > as Nick originally suggested: > https://lkml.org/lkml/2020/3/17/1477 Ack, I have changed this locally and I'll send a v2 along shortly once I have written some documentation to codify these suggested changes. Thank you for the comments and review, cheers! Nathan
