[PATCH] D143673: [lld][RISCV] Implement GP relaxation for R_RISCV_HI20/R_RISCV_LO12_I/R_RISCV_LO12_S.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a subscriber: lazyparser.
MaskRay added a comment.

In D143673#4159666 <https://reviews.llvm.org/D143673#4159666>, @craig.topper wrote:

> Ping

This patch is on my radar. I have been asking OSDT folks (@lazyparser's colleagues) to provide performance and code size analysis. So far the performance analysis says there is only noise (or oddly GP relaxation may negatively affect the performance by 0.x%...). I am still waiting for code size results...

In D143673#4162412 <https://reviews.llvm.org/D143673#4162412>, @asb wrote:

> FWIW I'm in favour of support for this, even if it shows very marginal benefit. We're going to get a constant stream of people asking about lack of support or wondering if this feature would improve code size / performance for them.

Honestly I do not know whether we have a constant stream of people...



================
Comment at: lld/ELF/Arch/RISCV.cpp:638
+  int64_t displace = r.sym->getVA(r.addend) - gp->getVA();
+
+  if (!isInt<12>(displace))
----------------
remove blank line


================
Comment at: lld/ELF/Writer.cpp:1870
       OutputSection *sec = findSection(".sdata");
-      addOptionalRegular("__global_pointer$", sec ? sec : Out::elfHeader, 0x800,
-                         STV_DEFAULT);
+      ElfSym::riscvGlobalPointer = addOptionalRegular(
+          "__global_pointer$", sec ? sec : Out::elfHeader, 0x800, STV_DEFAULT);
----------------
We can also set `riscvGlobalPointer` only if `config->relaxGp`, then we can remove `config->relaxGp` test in RISCV.cpp. In RISCV.cpp, we test both `config->relaxGp` and `riscvGlobalPointer`. I think it'd be good to reduce the number of tests.


================
Comment at: lld/test/ELF/riscv-relax-hi20-lo12.s:6
+# RUN: llvm-mc -filetype=obj -triple=riscv32-unknown-elf -mattr=+c,+relax %s -o %t.rv32c.o
+# RUN: llvm-mc -filetype=obj -triple=riscv64-unknown-elf -mattr=+c,+relax %s -o %t.rv64c.o
+
----------------
We may need a test for `-pie`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143673/new/

https://reviews.llvm.org/D143673