[PATCH v4] RISC-V: build: Allow LTO to be selected

[Nathan Chancellor]

On Fri, Oct 27, 2023 at 02:45:27PM +0100, Conor Dooley wrote:
> Hey Nathan,
> 
> On Tue, Oct 17, 2023 at 03:21:04PM -0700, Nathan Chancellor wrote:
> > From: Wende Tan <twd2.me at gmail.com>
> > 
> > Allow LTO to be selected for RISC-V, only when LLD >= 14, since there is
> > an issue [1] in prior LLD versions that prevents LLD to generate proper
> > machine code for RISC-V when writing `nop`s.
> > 
> > To avoid boot failures in QEMU [2], '-mattr=+c' and '-mattr=+relax'
> > need to be passed via '-mllvm' to ld.lld, as there appears to be an
> > issue with LLVM's target-features and LTO [3], which can result in
> > incorrect relocations to branch targets [4]. Once this is fixed in LLVM,
> > it can be made conditional on affected ld.lld versions.
> > 
> > Disable LTO for arch/riscv/kernel/pi, as llvm-objcopy expects an ELF
> > object file when manipulating the files in that subfolder, rather than
> > LLVM bitcode.
> > 
> > [1] https://github.com/llvm/llvm-project/issues/50505, resolved by LLVM
> >     commit e63455d5e0e5 ("[MC] Use local MCSubtargetInfo in writeNops")
> > [2] https://github.com/ClangBuiltLinux/linux/issues/1942
> > [3] https://github.com/llvm/llvm-project/issues/59350
> > [4] https://github.com/llvm/llvm-project/issues/65090
> > 
> > Tested-by: Wende Tan <twd2.me at gmail.com>
> > Signed-off-by: Wende Tan <twd2.me at gmail.com>
> > Co-developed-by: Nathan Chancellor <nathan at kernel.org>
> > Signed-off-by: Nathan Chancellor <nathan at kernel.org>
> > ---
> > Testing notes from Nathan:
> > 
> > I tested LLVM 14 through 18 with defconfig + full/thin LTO and
> > allmodconfig + thin LTO. allmodconfig + thin LTO with LLVM 15 and 16
> > shows
> > 
> >   ld.lld: error: section size decrease is too large
> > 
> > when linking vmlinux. This appears to be resolved in LLVM 17 with
> > https://github.com/llvm/llvm-project/commit/9d37ea95df1b84cca9b5e954d8964c976a5e303e
> > (I did not bisect but the commit message lines up with the issue). I
> > kept the existing version check because defconfig worked fine but we may
> > want to bump it to 17.0.0 if randconfigs trip over this.
> > 
> > v4 now boots in QEMU for me with both full and thin LTO.
> > 
> > Testing notes from Wende:
> > 
> > I have tested enabling LTO for `defconfig`. The LLD took ~2m21s and ~3GiB
> > on our Intel Xeon Gold 6140 server and produced an 18MiB Image. The image
> > can boot to shell using an archriscv rootfs on QEMU.
> > 
> > I have also tested it for `allyesconfig` without COMPILE_TEST, FTRACE,
> > KASAN, and GCOV. The LLD took ~7h03m and ~335GiB on the server,
> > successfully producing a 1.7GiB Image. Unfortunately, we cannot boot this
> > image because the `create_kernel_page_table()` -> `alloc_pmd_early()` ->
> > `BUG_ON()` logic limits the image to be < 1GiB. Maybe we can fix it in a
> > separate patch further.
> > 
> > Changes in v4:
> > - Pass '-mattr=+c' and '-mattr=+relax' to ld.lld via '-mllvm' flags when
> >   doing LTO due to LLVM not appearing to honor target-features when
> >   performing LTO, which can result in incorrect branch targets. This
> >   fixes boot with CONFIG_LTO_CLANG=y with LLVM 15.0.0+.
> > - Tidy up commit message and move testing notes to below the fold, as it
> >   is not too relevant to the git history and it makes the rest of the
> >   message more of an imperative mood.
> > - Link to v3: https://lore.kernel.org/r/20231003-riscv-lto-v3-1-8aca61a4ecb4@kernel.org
> > 
> > Changes in v3:
> > - Disable LTO in arch/riscv/kernel/pi/Makefile, which was added to the
> >   kernel after the submission of v2. This change matches arm64.
> > - Link to v2: https://lore.kernel.org/r/20220512205545.992288-1-twd2.me@gmail.com/
> > 
> > Changes in v2:
> > - Some textual changes suggested by Nick.
> > - Drop the changes to `arch/riscv/Makefile`, since the LLVM issue is
> >   filed and resolved.
> > - Drop the unnecessary changes to `arch/riscv/kernel/vdso/Makefile`.
> > - Link to v1: https://lore.kernel.org/r/20210719205208.1023221-1-twd2.me@gmail.com/
> > ---
> >  arch/riscv/Kconfig            | 3 +++
> >  arch/riscv/Makefile           | 5 +++++
> >  arch/riscv/kernel/pi/Makefile | 3 +++
> >  3 files changed, 11 insertions(+)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index d607ab0f7c6d..523640f7441e 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -46,6 +46,9 @@ config RISCV
> >  	select ARCH_SUPPORTS_CFI_CLANG
> >  	select ARCH_SUPPORTS_DEBUG_PAGEALLOC if MMU
> >  	select ARCH_SUPPORTS_HUGETLBFS if MMU
> > +	# LLD >= 14: https://github.com/llvm/llvm-project/issues/50505
> > +	select ARCH_SUPPORTS_LTO_CLANG if LLD_VERSION >= 140000
> > +	select ARCH_SUPPORTS_LTO_CLANG_THIN if LLD_VERSION >= 140000
> >  	select ARCH_SUPPORTS_PAGE_TABLE_CHECK if MMU
> >  	select ARCH_SUPPORTS_PER_VMA_LOCK if MMU
> >  	select ARCH_USE_MEMTEST
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index 1329e060c548..709400ceac60 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -52,6 +52,11 @@ ifndef CONFIG_AS_IS_LLVM
> >  	KBUILD_CFLAGS += -Wa,-mno-relax
> >  	KBUILD_AFLAGS += -Wa,-mno-relax
> >  endif
> > +# LLVM has an issue with target-features and LTO: https://github.com/llvm/llvm-project/issues/59350
> > +# Ensure it is aware of linker relaxation with LTO, otherwise relocations may
> > +# be incorrect: https://github.com/llvm/llvm-project/issues/65090
> > +else ifeq ($(CONFIG_LTO_CLANG),y)
> > +	KBUILD_LDFLAGS += -mllvm -mattr=+c -mllvm -mattr=+relax
> >  endif
> >  endif
> >  
> > diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
> > index 07915dc9279e..b75f150b923d 100644
> > --- a/arch/riscv/kernel/pi/Makefile
> > +++ b/arch/riscv/kernel/pi/Makefile
> > @@ -9,6 +9,9 @@ KBUILD_CFLAGS	:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) -fpie \
> >  		   -fno-asynchronous-unwind-tables -fno-unwind-tables \
> >  		   $(call cc-option,-fno-addrsig)
> >  
> 
> > +# Disable LTO
> 
> FWIW, this is a bit of a statement of the obvious, rather than being
> helpful. If this ends up being respun, I think it'd be good to add the
> context from the commit message.

Heh, good point. I had copied this from arm64 IIRC so I was not paying
much attention. If I have to respin, I will update this to:

# Disable LTO, as objcopy needs to work on ELF files, rather than LLVM
# bitcode.

> Otherwise,
> 
> Reviewed-by: Conor Dooley <conor.dooley at microchip.com>

Thanks for the review!

> > +KBUILD_CFLAGS	:= $(filter-out $(CC_FLAGS_LTO), $(KBUILD_CFLAGS))
> > +
> >  KBUILD_CFLAGS	+= -mcmodel=medany
> >  
> >  CFLAGS_cmdline_early.o += -D__NO_FORTIFY
> > 
> > ---
> > base-commit: 8a749fd1a8720d4619c91c8b6e7528c0a355c0aa
> > change-id: 20231003-riscv-lto-f013beed8587
> > 
> > Best regards,
> > -- 
> > Nathan Chancellor <nathan at kernel.org>
> >