On Thu, Aug 24, 2023 at 11:49 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > > On Thu, Aug 24, 2023 at 11:03:17AM -0700, ndesaulniers@xxxxxxxxxx wrote: > > Link: https://github.com/ClangBuiltLinux/linux/issues/1907 [0] > > --- > > > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > > Nit: Your signed-off-by ended up below the fold, was it in your cover > letter commit rather than your actual commit? Heh, no. I'm restoring a machine after suffering drive corruption last week. I just got patatt and friends re-set up, but I forgot to `git commit --amend -s` for this patch once everything was working again. Thanks for catching this! > > Aside from the relatively minor comments below, this looks like a really > good improvement to the documentation to me. It feels like it is more > targeting users or non-kbuild folks now, which I think is great. > > I trust you to address my comments as you see fit, so please carry > forward: > > Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx> Thanks for the review! > I see a few new kernel-doc warnings from not adjusting the underlines to > match the new length of the title: > > Documentation/kbuild/llvm.rst:40: WARNING: Title underline too short. > > The LLVM= argument > -------------- > Documentation/kbuild/llvm.rst:40: WARNING: Title underline too short. > > The LLVM= argument > -------------- > Documentation/kbuild/llvm.rst:102: WARNING: Title underline too short. > > The LLVM_IAS= argument > ----------------- > Documentation/kbuild/llvm.rst:102: WARNING: Title underline too short. > > The LLVM_IAS= argument > ----------------- oops! remind me of the make target to observe these? > > -For example, to cross-compile the arm64 kernel:: > > + make LLVM=1 LD=ld.bfd CROSS_COMPILE=s390x-linux-gnu- > > This should probably have ARCH=s390? Oops! Good catch! > > > - make ARCH=arm64 LLVM=1 > > +``CROSS_COMPILE`` is not used to prefix the Clang compiler binary (or > > +corresponding LLVM utilities), but it will be for any GNU toolchain utilities. > > +This example will invoke ``s390x-linux-gnu-ld.bfd`` as the linker, so ensure > > +that is reachable in your ``$PATH``. > > I like the example as I feel like it addresses some of the fear I have had > around recommending LLVM=1 as the initial build suggestion but 'LLVM=1 > LD=ld.bfd CROSS_COMPILE=s390x-linux-gnu-' does not compose as you describe here > because $(LD) is not prefixed with $(CROSS_COMPILE) anywhere in Makefile. The > non-$(LLVM) default assignment of $(LD) is '$(CROSS_COMPILE)ld' and that is > overridden by 'LD=ld.bfd' on the command line. > > In other words, this should be > > make ARCH=s390 LLVM=1 LD=s390x-linux-gnu-ld.bfd > > and have the note about CROSS_COMPILE prefixing any GNU toolchain utilities > removed. It should problably have OBJCOPY and OBJDUMP too, as those are > required due to https://github.com/ClangBuiltLinux/linux/issues/859 and > https://github.com/ClangBuiltLinux/linux/issues/1530. Ah right; I'm no longer able to repro the build failure with OBJDUMP, though I still see warnings. I kind of feel like we should bring back CROSS_COMPILE in some form; having to respecify the triple is still sub-optimal. i.e. today: $ make LLVM=1 ARCH=s390 LD=s390x-linux-gnu-ld.bfd OBJCOPY=s390x-linux-gnu-objcopy OBJDUMP=s390x-linux-gnu-objdump vs $ make LLVM=1 ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- LD=ld.bfd OBJCOPY=objcopy OBJDUMP=objdump but perhaps that's a change for another day. > > > -If ``LLVM_IAS=0`` is specified, ``CROSS_COMPILE`` is also used to derive > > -``--prefix=<path>`` to search for the GNU assembler and linker. :: > > +The LLVM_IAS= argument > > +----------------- > > > > - make ARCH=arm64 LLVM=1 LLVM_IAS=0 CROSS_COMPILE=aarch64-linux-gnu- > > +Clang can assemble assembler code. You can pass ``LLVM_IAS=0`` to disable this > > +behavior and have Clang invoke the system assembler instead (or the assembler > > +based on ``CROSS_COMPILE``). ``CROSS_COMPILE`` is necessary when ``LLVM_IAS=0`` > > +is set when cross compiling in order to set ``--prefix=`` for the compiler to > > +find the corresponding non-integrated assembler. > > Thanks a lot for documenting this behavior, it is one of the most common > issues I run into myself (adding LLVM_IAS=0 without CROSS_COMPILE) and > maybe this note will be what I need in order to remember :) Ah right, I'll make that clearer. > > > > -We provide prebuilt stable versions of LLVM on `kernel.org <https://kernel.org/pub/tools/llvm/>`_. > > -Below are links that may be useful for building LLVM from source or procuring > > -it through a distribution's package manager. > > +We provide prebuilt stable versions of LLVM on `kernel.org > > +<https://kernel.org/pub/tools/llvm/>`_. These have been optimized with profile > > +data for building Linux kernels. Below are links that may be useful for > > Maybe make a note of why this matters? ", which should lower kernel > build times compared to non-optimized LLVM toolchains."? Ack -- Thanks, ~Nick Desaulniers
