Re: [PATCH v3 2/2] Makefile: infer CROSS_COMPILE from SRCARCH for CC=clang LLVM_IAS=1

[Masahiro Yamada <masahiroy@kernel.org>]

On Fri, Jul 30, 2021 at 6:00 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> I realized that the title of this commit is not really right. We are not
> inferring CROSS_COMPILE, we are inferring '--target='.
>
> On 7/29/2021 9:50 AM, Nick Desaulniers wrote:
> > We get constant feedback that the command line invocation of make is too
> > long. CROSS_COMPILE is helpful when a toolchain has a prefix of the
> > target triple, or is an absolute path outside of $PATH, but it's mostly
> > redundant for a given SRCARCH. SRCARCH itself is derived from ARCH
>
> I feel like the beginning of this needs a little work.
>
> 1. "...invocation of make is too long when compiling with LLVM" would be
> a little more accurate.
>
> 2. "it's mostly redundant for a given SRCARCH" is not quite true in my
> eyes. For example, you could have aarch64-linux-, aarch64-elf-, or
> aarch64-linux-gnu-, and to my knowledge, all of these can compile a
> working Linux kernel. Again, saying "with LLVM", even mentioning its
> multitargeted nature, might make it a little more accurate to the casual
> passerby.
>
> > (normalized for a few different targets).
> >
> > If CROSS_COMPILE is not set, simply set --target= for CLANG_FLAGS,
> > KBUILD_CFLAGS, and KBUILD_AFLAGS based on $SRCARCH.
> >
> > Previously, we'd cross compile via:
> > $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make LLVM=1 LLVM_IAS=1
> > Now:
> > $ ARCH=arm64 make LLVM=1 LLVM_IAS=1
> >
> > For native builds (not involving cross compilation) we now explicitly
> > specify a target triple rather than rely on the implicit host triple.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1399
> > Suggested-by: Arnd Bergmann <arnd@kernel.org>
> > Suggested-by: Nathan Chancellor <nathan@kernel.org>
> > Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
>
> > ---
> > Changes v2 -> v3:
> > * Drop check/requirement for LLVM=1, as per Masahiro.
> > * Change oneliner from LLVM=1 LLVM_IAS=1 to CC=clang LLVM_IAS=1.
> > * Don't carry forward Nathan's RB/TB tags. :( Sorry Nathan, but thank
> >    you for testing+reviewing v2.
> > * Update wording of docs slightly.
> >
> > Changes v1 -> v2:
> > * Fix typos in commit message as per Geert and Masahiro.
> > * Use SRCARCH instead of ARCH, simplifying x86 handling, as per
> >    Masahiro. Add his sugguested by tag.
> > * change commit oneline from 'drop' to 'infer.'
> > * Add detail about explicit host --target and relationship of ARCH to
> >    SRCARCH, as per Masahiro.
> >
> > Changes RFC -> v1:
> > * Rebase onto linux-kbuild/for-next
> > * Keep full target triples since missing the gnueabi suffix messes up
> >    32b ARM. Drop Fangrui's sugguested by tag. Update commit message to
> >    drop references to arm64.
> > * Flush out TODOS.
> > * Add note about -EL/-EB, -m32/-m64.
> > * Add note to Documentation/.
> >
> >   Documentation/kbuild/llvm.rst |  6 ++++++
> >   scripts/Makefile.clang        | 32 ++++++++++++++++++++++++++++++--
> >   2 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> > index b18401d2ba82..aef1587fc09b 100644
> > --- a/Documentation/kbuild/llvm.rst
> > +++ b/Documentation/kbuild/llvm.rst
> > @@ -46,6 +46,12 @@ example: ::
> >
> >       clang --target=aarch64-linux-gnu foo.c
> >
> > +When both ``CC=clang`` (set via ``LLVM=1``) and ``LLVM_IAS=1`` are used,
> > +``CROSS_COMPILE`` becomes unnecessary and can be inferred from ``ARCH``.
>
> I am not a fan of this sentence because it implies that something like
> 'make ARCH=arm64 CC=clang LLVM_IAS=1' will work fine, which is not true.
> We still need CROSS_COMPILE for binutils in this configuration.


Agree.  That sentence is misleading, and moreover, it is wrong.



>
> CROSS_COMPILE provides the value for '--target=' and the prefix for the
> GNU tools such as ld, objcopy, and readelf. I think this direction is a
> regression because we are just talking about the first use of
> CROSS_COMPILE rather than the second at the same time.
>
> With LLVM=1 LLVM_IAS=1, we KNOW that the user will be using all LLVM
> tools. Sure, they are free to override LD, OBJCOPY, READELF, etc with
> the GNU variants but they have to provide the prefix because LLVM=1
> overrides the $(CROSS_COMPILE)<tool> assignments so it is irrelevant to
> us. As Masahiro mentioned, the user is free to individually specify all
> the tools by their individual variables such as LD=ld.lld BUT at that
> point, the user should be aware of what they are doing and specify
> CROSS_COMPILE.
>
> While I understand that the LLVM=1 LLVM_IAS=1 case works perfectly fine
> with this series, I am of the belief that making it work for CC=clang
> LLVM_IAS=1 is a mistake because there is no way for that configuration
> to work for cross compiling without CROSS_COMPILE.



LLVM=1 is a too strong requirement.


LLVM=1 switches not only target tools
(CC=clang, LD=ld.lld, AR=llvm-ar...)
but also host tools
(HOSTCC=clang, HOSTCXX=g++...).

Obviously host-tools are don't-care here.



Specifying the target tools individually, as in

   make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
     OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf

... is a perfectly correct command that
makes CROSS_COMPILE unnecessary.

There is no reason to stop inferring --target for this case.


The problem is NOT removing the LLVM=1 check
but the wrong documentation.


Let's write a precise document.

For example, the following document exactly
explains what is happening in the code,
and is still clear.




diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
index b18401d2ba82..a0d862bd73ac 100644
--- a/Documentation/kbuild/llvm.rst
+++ b/Documentation/kbuild/llvm.rst
@@ -63,6 +63,26 @@ They can be enabled individually. The full list of
the parameters: ::
 Currently, the integrated assembler is disabled by default. You can pass
 ``LLVM_IAS=1`` to enable it.

+
+Omitting CROSS_COMPILE
+----------------------
+
+As explained above, ``CROSS_COMPILE`` is used to set ``--target=<triple>``.
+
+Unless ``LLVM_IAS=1`` is specified, ``CROSS_COMPILE`` is also used to derive
+``--prefix=<path>`` to search for the back-end GNU assembler.
+
+If CROSS_COMPILE is not specified, the ``--target=<triple>`` is inferred from
+``ARCH``.
+
+It means, if you use only LLVM tools, `CROSS_COMPILE`` becomes unnecessary.
+
+For example, to cross-compile the arm64 kernel::
+
+       ARCH=arm64 make LLVM=1 LLVM_IAS=1
+
+
 Supported Architectures
 -----------------------





BTW, I noticed LLVM_IAS=1 check is also unneeded
for the same reason.
So, it should go away.


-- 
Best Regards
Masahiro Yamada