[lld] [RISC-V][lld] Fix divergent relaxation issue (PR #73624)

via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 28 01:05:39 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lld-elf

Author: Nemanja Ivanovic (nemanjai)

<details>
<summary>Changes</summary>

Linker relaxations in different sections can always have circular dependencies. For example, removing 4 bytes in one section can allow the linker to relax an instrucion in a later section and remove 4 bytes. Doing so however, now moves the first instruction far enough away that the first relaxation is no longer valid. Not performing the first relaxation then makes the second one invalid. Removing it makes the first one valid again, and so on indefinitely.

This patch tracks the number of times the relaxation decision for a specific relocation changes. When some threshold is reached, relaxation for that relocation is no longer attempted. While it is not guaranteed that any number of flip-flops in the decision to relax or not for a specific relocation is an indication of such a circular dependency, it is a likely indication that it is so. Missing a few relaxation opportunities is clearly preferable over the linker failing to link the binary.

---
Full diff: https://github.com/llvm/llvm-project/pull/73624.diff


2 Files Affected:

- (modified) lld/ELF/Arch/RISCV.cpp (+42-2) 
- (added) lld/test/ELF/riscv-divergent-relaxation.s (+59) 


``````````diff
diff --git a/lld/ELF/Arch/RISCV.cpp b/lld/ELF/Arch/RISCV.cpp
index a556d89c36400d3..5b338e153c7c7b7 100644
--- a/lld/ELF/Arch/RISCV.cpp
+++ b/lld/ELF/Arch/RISCV.cpp
@@ -530,6 +530,7 @@ struct elf::RISCVRelaxAux {
   // For relocations[i], the actual offset is r_offset - (i ? relocDeltas[i-1] :
   // 0).
   std::unique_ptr<uint32_t[]> relocDeltas;
+  std::unique_ptr<uint32_t[]> relaxDecisions;
   // For relocations[i], the actual type is relocTypes[i].
   std::unique_ptr<RelType[]> relocTypes;
   SmallVector<uint32_t, 0> writes;
@@ -545,6 +546,8 @@ static void initSymbolAnchors() {
       if (sec->relocs().size()) {
         sec->relaxAux->relocDeltas =
             std::make_unique<uint32_t[]>(sec->relocs().size());
+        sec->relaxAux->relaxDecisions =
+            std::make_unique<uint32_t[]>(sec->relocs().size());
         sec->relaxAux->relocTypes =
             std::make_unique<RelType[]>(sec->relocs().size());
       }
@@ -588,6 +591,26 @@ static void initSymbolAnchors() {
   }
 }
 
+// Track the number of state changes between relax and norelax have occurred
+// for a particular relocation. If hist is odd, last decision was relax, even,
+// last decision was norelax.
+static void trackRelaxedDecision(uint32_t &hist, bool relaxed) {
+  bool lastRelaxed = (bool)(hist & 0x1);
+  if (relaxed != lastRelaxed)
+    hist++;
+}
+
+// Once the decision about relaxing a specific relocation has completed
+// stopRelaxingAfter state changes and reached "norelax", we will stop
+// attempting to relax this relocation. This is an indicator that there
+// is a good chance that the ability to relax relocations in two or more
+// sections have a circular dependency.
+static bool giveUpRelaxing(const InputSection &sec, size_t i) {
+  static constexpr int stopRelaxingAfter = 6;
+  return sec.relaxAux->relaxDecisions[i] >= stopRelaxingAfter &&
+         (sec.relaxAux->relaxDecisions[i] & 0x1) == 0;
+}
+
 // Relax R_RISCV_CALL/R_RISCV_CALL_PLT auipc+jalr to c.j, c.jal, or jal.
 static void relaxCall(const InputSection &sec, size_t i, uint64_t loc,
                       Relocation &r, uint32_t &remove) {
@@ -599,6 +622,9 @@ static void relaxCall(const InputSection &sec, size_t i, uint64_t loc,
       (r.expr == R_PLT_PC ? sym.getPltVA() : sym.getVA()) + r.addend;
   const int64_t displace = dest - loc;
 
+  if (giveUpRelaxing(sec, i))
+    return;
+
   if (rvc && isInt<12>(displace) && rd == 0) {
     sec.relaxAux->relocTypes[i] = R_RISCV_RVC_JUMP;
     sec.relaxAux->writes.push_back(0xa001); // c.j
@@ -613,14 +639,20 @@ static void relaxCall(const InputSection &sec, size_t i, uint64_t loc,
     sec.relaxAux->writes.push_back(0x6f | rd << 7); // jal
     remove = 4;
   }
+  // If remove is non-zero, this relocation was relaxed.
+  trackRelaxedDecision(sec.relaxAux->relaxDecisions[i], remove != 0);
 }
 
 // Relax local-exec TLS when hi20 is zero.
 static void relaxTlsLe(const InputSection &sec, size_t i, uint64_t loc,
                        Relocation &r, uint32_t &remove) {
   uint64_t val = r.sym->getVA(r.addend);
-  if (hi20(val) != 0)
+  if (giveUpRelaxing(sec, i))
+    return;
+  if (hi20(val) != 0) {
+    trackRelaxedDecision(sec.relaxAux->relaxDecisions[i], false);
     return;
+  }
   uint32_t insn = read32le(sec.content().data() + r.offset);
   switch (r.type) {
   case R_RISCV_TPREL_HI20:
@@ -642,6 +674,8 @@ static void relaxTlsLe(const InputSection &sec, size_t i, uint64_t loc,
     sec.relaxAux->writes.push_back(setLO12_S(insn, val));
     break;
   }
+  // If remove is non-zero, this relocation was relaxed.
+  trackRelaxedDecision(sec.relaxAux->relaxDecisions[i], remove != 0);
 }
 
 static void relaxHi20Lo12(const InputSection &sec, size_t i, uint64_t loc,
@@ -650,14 +684,20 @@ static void relaxHi20Lo12(const InputSection &sec, size_t i, uint64_t loc,
   if (!gp)
     return;
 
-  if (!isInt<12>(r.sym->getVA(r.addend) - gp->getVA()))
+  if (giveUpRelaxing(sec, i))
     return;
 
+  if (!isInt<12>(r.sym->getVA(r.addend) - gp->getVA())) {
+    trackRelaxedDecision(sec.relaxAux->relaxDecisions[i], false);
+    return;
+  }
+
   switch (r.type) {
   case R_RISCV_HI20:
     // Remove lui rd, %hi20(x).
     sec.relaxAux->relocTypes[i] = R_RISCV_RELAX;
     remove = 4;
+    trackRelaxedDecision(sec.relaxAux->relaxDecisions[i], true);
     break;
   case R_RISCV_LO12_I:
     sec.relaxAux->relocTypes[i] = INTERNAL_R_RISCV_GPREL_I;
diff --git a/lld/test/ELF/riscv-divergent-relaxation.s b/lld/test/ELF/riscv-divergent-relaxation.s
new file mode 100644
index 000000000000000..224c9efd57ec243
--- /dev/null
+++ b/lld/test/ELF/riscv-divergent-relaxation.s
@@ -0,0 +1,59 @@
+# REQUIRES: riscv
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32-unknown-elf -mattr=+relax a.s -o rv32.o
+# RUN: llvm-mc -filetype=obj -triple=riscv64-unknown-elf -mattr=+relax a.s -o rv64.o
+
+# RUN: ld.lld --relax rv32.o lds -o rv32
+# RUN: ld.lld --relax rv64.o lds -o rv64
+# RUN: llvm-objdump -td -M no-aliases --no-show-raw-insn rv32 | FileCheck %s
+# RUN: llvm-objdump -td -M no-aliases --no-show-raw-insn rv64 | FileCheck %s
+
+# CHECK: 00101004 l       .secf1 {{0+}} f1
+# CHECK: 00101004 l       .secf2 {{0+}} f2
+# CHECK: 00101008 l       .secf2 {{0+}} f3
+# CHECK: 00001000 g       .text  {{0+}} _start
+
+# CHECK: <.callf1>
+# CHECK-NEXT: auipc   ra, 256
+# CHECK-NEXT: jalr    ra, 4(ra)
+
+# CHECK: <.callf2>:
+# CHECK-NEXT: auipc   ra, 256
+# CHECK-NEXT: jalr    ra, -4(ra)
+
+# CHECK: <.callf3>:
+# CHECK-NEXT: auipc   ra, 256
+# CHECK-NEXT: jalr    ra, -8(ra)
+
+#--- a.s
+.global _start
+_start:
+.section .callf1,"ax"
+  call f1       # relax if there is a relaxation after
+
+.section .callf2,"ax"
+  call f2       # relax if there is no relaxation before
+.section .callf3,"ax"
+  call f3       # relax if there is no relaxation before
+  .space 1048556
+
+.section .secf1,"ax"
+f1:
+#  lui a0, 11
+
+.section .secf2,"ax"
+f2:
+  lui a0, 12
+f3:
+  lui a0, 10
+
+#--- lds
+SECTIONS {
+  .text 0x1000    : { }
+  .callf1         : { }
+  .callf2         : { }
+  .callf3         : { }
+  .secf1          : { }
+  .secf2 0x101004 : { }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/73624


More information about the llvm-commits mailing list