[PATCH] D125036: [RISCV] Alignment relaxation
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat May 14 16:39:16 PDT 2022
MaskRay added a comment.
Clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git , apply this patch:
--- i/arch/riscv/Makefile
+++ w/arch/riscv/Makefile
@@ -38,9 +38,9 @@ endif
ifeq ($(CONFIG_LD_IS_LLD),y)
- KBUILD_CFLAGS += -mno-relax
- KBUILD_AFLAGS += -mno-relax
+ #KBUILD_CFLAGS += -mno-relax
+ #KBUILD_AFLAGS += -mno-relax
ifndef CONFIG_AS_IS_LLVM
- KBUILD_CFLAGS += -Wa,-mno-relax
- KBUILD_AFLAGS += -Wa,-mno-relax
+ #KBUILD_CFLAGS += -Wa,-mno-relax
+ #KBUILD_AFLAGS += -Wa,-mno-relax
endif
endif
(My llvm build directory is /tmp/RelA)
ninja -C /tmp/RelA clang lld llvm-{nm,size,strings,objcopy,objdump,readelf,ar,strip}
cd ~/Dev/linux
PATH=/tmp/RelA/bin:$PATH make O=/tmp/linux/riscv64 ARCH=riscv CROSS_COMPILE=riscv64-linux-gnu- LLVM=1 -j60 defconfig all
...
I see a segfault
LD .tmp_vmlinux.kallsyms1
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0. Program arguments: ld.lld -melf64lriscv --build-id=sha1 --script=./arch/riscv/kernel/vmlinux.lds --strip-debug -o .tmp_vmlinux.kallsyms1 --whole-archive arch/riscv/kernel/head.o init/built-in.a usr/built-in.a arch/riscv/built-in.a arch/riscv/errata/built-in.a kernel/built-in.a certs/built-in.a mm/built-in.a fs/built-in.a ipc/built-in.a security/built-in.a crypto/built-in.a block/built-in.a lib/built-in.a arch/riscv/lib/built-in.a lib/lib.a arch/riscv/lib/lib.a drivers/built-in.a sound/built-in.a net/built-in.a virt/built-in.a --no-whole-archive --start-group ./drivers/firmware/efi/libstub/lib.a --end-group
#0 0x000056142291b4d3 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/tmp/RelA/bin/lld+0x387c4d3)
#1 0x000056142291940e llvm::sys::RunSignalHandlers() (/tmp/RelA/bin/lld+0x387a40e)
#2 0x000056142291bada SignalHandler(int) Signals.cpp:0:0
#3 0x00007f8197d53200 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12200)
#4 0x0000561422a7ad01 void std::__introsort_loop<SymbolAddr*, long, __gnu_cxx::__ops::_Iter_comp_iter<lld::elf::InputSectionBase::adjustRanges(llvm::ArrayRef<lld::elf::InputSectionBase::AdjustRange>)::$_0>>(SymbolAddr*, SymbolAddr*, long, __gnu_cxx::__ops::_Iter_comp_iter<lld::elf::InputSectionBase::adjustRanges(llvm::ArrayRef<lld::elf::InputSectionBase::AdjustRange>)::$_0>) InputSection.cpp:0:0
#5 0x0000561422a6d458 lld::elf::InputSectionBase::adjustRanges(llvm::ArrayRef<lld::elf::InputSectionBase::AdjustRange>) (/tmp/RelA/bin/lld+0x39ce458)
#6 0x0000561422b61a7c (anonymous namespace)::RISCV::finalizeSections() const RISCV.cpp:0:0
#7 0x0000561422ba4223 (anonymous namespace)::Writer<llvm::object::ELFType<(llvm::support::endianness)1, true>>::finalizeSections() Writer.cpp:0:0
#8 0x0000561422b827dc void lld::elf::writeResult<llvm::object::ELFType<(llvm::support::endianness)1, true>>() (/tmp/RelA/bin/lld+0x3ae37dc)
#9 0x00005614229f170f lld::elf::LinkerDriver::link(llvm::opt::InputArgList&) (/tmp/RelA/bin/lld+0x395270f)
#10 0x00005614229e3ab4 lld::elf::LinkerDriver::linkerMain(llvm::ArrayRef<char const*>) (/tmp/RelA/bin/lld+0x3944ab4)
#11 0x00005614229e242d lld::elf::link(llvm::ArrayRef<char const*>, llvm::raw_ostream&, llvm::raw_ostream&, bool, bool) (/tmp/RelA/bin/lld+0x394342d)
#12 0x000056142286f488 lldMain(int, char const**, llvm::raw_ostream&, llvm::raw_ostream&, bool) lld.cpp:0:0
#13 0x000056142286ec62 main (/tmp/RelA/bin/lld+0x37cfc62)
#14 0x00007f81975fa7fd __libc_start_main ./csu/../csu/libc-start.c:332:16
#15 0x000056142286e7aa _start (/tmp/RelA/bin/lld+0x37cf7aa)
================
Comment at: lld/ELF/Arch/RISCV.cpp:476
+using AdjustRanges = std::vector<InputSectionBase::AdjustRange>;
+
----------------
Consider `SmallVector<InputSectionBase::AdjustRange, 0>`
================
Comment at: lld/ELF/Arch/RISCV.cpp:488
+
+const int alignNopBytesWidth = 16;
+const int alignNopBytesMask = ((1 << alignNopBytesWidth) - 1);
----------------
constexpr
================
Comment at: lld/ELF/Arch/RISCV.cpp:491
+
+void setAlignBoundary(Relocation &r, unsigned n) {
+ assert((r.addend & ~alignNopBytesMask) == 0);
----------------
static
================
Comment at: lld/ELF/Arch/RISCV.cpp:496
+
+unsigned getAlignBoundary(Relocation &r) {
+ return r.addend >> alignNopBytesWidth;
----------------
static
Actually, avoid defining such a short function.
================
Comment at: lld/ELF/Arch/RISCV.cpp:501
+void setAlignNopBytes(Relocation &r, unsigned n) {
+ r.addend = (r.addend & ~alignNopBytesMask) | n;
+}
----------------
avoid defining such a short function.
================
Comment at: lld/ELF/Arch/RISCV.cpp:514
+static void setAlignBoundaries() {
+ for (OutputSection *os : outputSections)
+ for (InputSection *is : getInputSections(*os))
----------------
Newer code uses `osec` instead of `os`. `os` is usually for `raw_ostream`.
================
Comment at: lld/ELF/Arch/RISCV.cpp:525
+ AdjustRanges &adjustRanges) {
+ uint64_t pc = is->getVA(r.offset) + delta;
+ uint64_t oldNopBytes = getAlignNopBytes(r);
----------------
Add some `const`
================
Comment at: lld/ELF/Arch/RISCV.cpp:548
+void fillAlignNops() {
+ for (OutputSection *os : outputSections)
+ for (InputSection *is : getInputSections(*os))
----------------
================
Comment at: lld/ELF/Arch/RISCV.cpp:550
+ for (InputSection *is : getInputSections(*os))
+ if (is->flags & SHF_EXECINSTR)
+ for (Relocation &r : is->relocations)
----------------
Use
```
if (!(is->flags & SHF_EXECINSTR))
continue;
```
to decrease indentation.
================
Comment at: lld/ELF/Arch/RISCV.cpp:519
+
+ // Rewrite the NOP sequence
+ uint8_t *buf = is->mutableData().data() + rel.offset;
----------------
reames wrote:
> Extend this comment to indicate why this is required - i.e. split a 4 byte into a 2-byte remainder case + not wanting to assume the compiler emitted nops at all.
+2 needs a comment.
================
Comment at: lld/ELF/InputSection.cpp:189
+ SmallVector<SymbolAddr, 0> symbolAddrs;
+ for (Symbol *b : file->getSymbols())
+ if (Defined *d = dyn_cast<Defined>(b))
----------------
Use `sym` for `Symbol *`.
Avoid `Symbol *b` from some old code.
================
Comment at: lld/ELF/InputSection.cpp:1100
const Relocation &rel = relocations[i];
- if (rel.expr == R_NONE)
+ if (rel.expr == R_NONE || rel.expr == R_RELAX_HINT)
continue;
----------------
Prefer moving R_RELAX_HINT to the switch below to not penalize other architectures. They don't want to take a comparison overhead here.
================
Comment at: lld/test/ELF/riscv-relax-align-rvc.s:11
+# RUN: ld.lld %t/rv64.o -o %t/relax.rv64
+# RUN: llvm-objdump -d -M no-aliases %t/relax.rv32 > %t/relax.rv32.dis
+# RUN: llvm-objdump -d -M no-aliases %t/relax.rv64 > %t/relax.rv64.dis
----------------
Avoid a temporary file for FIleCheck input.
================
Comment at: lld/test/ELF/riscv-relax-align.s:39
+.balign 4
+ add t0, t1, t2
----------------
Add a few more text sections to demonstrate that multiple sections can be handled.
================
Comment at: lld/test/ELF/riscv-relax-syms.s:2
+# REQUIRES: riscv
+# RUN: rm -rf %t && mkdir -p %t
+
----------------
Avoid `-p` if the directory is known to not exist. Use `cd %t` if you need to refer to `%t/` many times.
================
Comment at: lld/test/ELF/riscv-relax-syms.s:4
+
+// Check that relaxation correctly adjusts symbol addresses and sizes.
+
----------------
`## `
================
Comment at: lld/test/ELF/riscv-relax-syms.s:17
+# CHECK: 100000 4 NOTYPE LOCAL DEFAULT 1 a
+# CHECK: 100000 12 NOTYPE LOCAL DEFAULT 1 b
+# CHECK: 100004 8 NOTYPE LOCAL DEFAULT 1 c
----------------
`CHECK-DAG: `
================
Comment at: lld/test/ELF/riscv-relax-syms.s:29
+b:
+ add a0, a1, a2 # [0..4)
+.size a, .-a # 4
----------------
Add a comment somewhere to explain what the notation `[0..4)` means.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125036/new/
https://reviews.llvm.org/D125036
More information about the llvm-commits
mailing list