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:
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
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.
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.