riscv: Enable linker relaxation with lld 15 and later
Needs ReviewPublic
Actions

Authored by rick_sloservers.com on Sat, Oct 3, 12:56 PM.

Details

Reviewers

mhorne
jrtc27
dim
imp
emaste
markj
manu

Summary

ld.lld gained RISC-V linker relaxation in LLVM 15, but bsd.linker.mk
only reports riscv-relaxations for GNU ld, so we still build with
-mno-relax.

Report it for lld 15 and later, and pass -mrelax explicitly since our
clang defaults to -mno-relax (389f88cffd0d).

Test Plan

Built world and a QEMU kernel for riscv64 with lld 19 and compared them with an unrelaxed build of the same tree, verified relaxation took effect: every call in libc.so.7 is now a jal, and so are about half of the calls in the kernel.

The kernel boots to multi-user in qemu-system-riscv64, with and without loader.efi. Modules work too, including a VNET jail with epair and pf, which is what relaxation broke on arm64 (PR 264094).

kyua results for lib/libc, lib/msun, bin/sh and sys/net match the unrelaxed build.

Tested only in QEMU, I lack RISCV hardware for testing on real hw.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

rick_sloservers.com created this revision.Sat, Oct 3, 12:56 PM

Herald added a reviewer: manu. · View Herald TranscriptSat, Oct 3, 12:56 PM

Herald added subscribers: dab, bdrewery. · View Herald Transcript

rick_sloservers.com requested review of this revision.Sat, Oct 3, 12:56 PM

Did you use LLMs to generate this?

In D60278#1383212, @jrtc27 wrote:

Did you use LLMs to generate this?

Yes

If you’re to use them, at least have the decency to remove all the slop and write a proper commit message. As it stands this is a very low-effort change that feels extractive.

Yes, no problem. Sorry, it's not lack of effort on my part, just ignorance. I'm new to this system and didn't know that the summary becomes the commit message, I can understand now why that would be irritating. Give a few moments, please, and I will rewrite.

rick_sloservers.com retitled this revision from riscv: Use linker relaxation with lld 15 and later to riscv: Enable linker relaxation with lld 15 and later.Sat, Oct 3, 1:42 PM

rick_sloservers.com edited the summary of this revision. (Show Details)

rick_sloservers.com edited the test plan for this revision. (Show Details)

mhorne added inline comments.Sun, Oct 4, 2:28 PM

stand/defs.mk
186–188	What's going on here that you didn't add the `-mrelax` flag? Are we opting to never enable relaxations for the bootloader?

rick_sloservers.com added inline comments.Sun, Oct 4, 2:53 PM

stand/defs.mk
186–188	No, the loader gets relaxed too. stand/efi/loader and libsa include bsd.prog.mk and bsd.lib.mk, which now add -mrelax when the linker supports relaxations, so this block only matters for a linker that can't relax. With lld 15+ it doesn't apply. I only changed its comment, since it said lld can't relax. I can add a .else with -mrelax here too if you'd rather it be explicit.

mhorne added inline comments.Wed, Oct 7, 4:16 PM

stand/defs.mk
186–188	Yes, please add the `.else` case, for consistency. Given the inclusion of `bsd.prog.mk` or `bsd.lib.mk`, I wonder about the necessity of this block at all (it seems I am the original culprit...). I don't think it's necessary to chase it down.

Looking at this again, I think your instinct is right and the block in defs.mk can just go. It came in with the riscv EFI loader (2192efc03bc4), but 79f5991559ce had already moved -mno-relax into bsd.prog.mk and bsd.lib.mk a few months before, so the loader gets the flag from there. I removed it and checked every riscv dir under stand, they all still get -mrelax. I think an .else would also have to go under a riscv check, or else the amd64 and arm64 loaders would get -mrelax. Removing the whole block seems cleaner. What do you think?