LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Masahiro Yamada <masahiroy@kernel.org>
To: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
	Miguel Ojeda <ojeda@kernel.org>,
	Fangrui Song <maskray@google.com>,
	Michal Marek <michal.lkml@markovi.net>,
	Arnd Bergmann <arnd@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Christoph Hellwig <hch@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v3 2/2] Makefile: infer CROSS_COMPILE from SRCARCH for CC=clang LLVM_IAS=1
Date: Sat, 31 Jul 2021 00:10:18 +0900	[thread overview]
Message-ID: <CAK7LNAQvGK6pKXpE9=P-BXK5GHmLLVJRnxq84VOVz_1bm72FBg@mail.gmail.com> (raw)
In-Reply-To: <44117d0c-51b7-1f68-f752-ba53de503b14@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

      parent reply	other threads:[~2021-07-30 15:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 16:50 [PATCH v3 0/2] " Nick Desaulniers
2021-07-29 16:50 ` [PATCH v3 1/2] Makefile: move initial clang flag handling into scripts/Makefile.clang Nick Desaulniers
2021-07-29 16:50 ` [PATCH v3 2/2] Makefile: infer CROSS_COMPILE from SRCARCH for CC=clang LLVM_IAS=1 Nick Desaulniers
2021-07-29 19:40   ` Arnd Bergmann
2021-07-29 21:00   ` Nathan Chancellor
2021-07-30  0:19     ` Nick Desaulniers
2021-07-30  6:50       ` Miguel Ojeda
2021-07-30 15:15       ` Masahiro Yamada
2021-07-30 15:10     ` Masahiro Yamada [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAK7LNAQvGK6pKXpE9=P-BXK5GHmLLVJRnxq84VOVz_1bm72FBg@mail.gmail.com' \
    --to=masahiroy@kernel.org \
    --cc=arnd@kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=geert@linux-m68k.org \
    --cc=hch@infradead.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maskray@google.com \
    --cc=michal.lkml@markovi.net \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=ojeda@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH v3 2/2] Makefile: infer CROSS_COMPILE from SRCARCH for CC=clang LLVM_IAS=1' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).