Skip to content

Conversation

@lenary
Copy link
Member

@lenary lenary commented Jul 30, 2025

Since this code was last reviewed, more relaxations have been added to the psABI for existing standard relocations that LLVM didn't have marked as relaxable.

This change ensures that LLVM marks the following relocations (and their respective fixups) as relaxable:

  • R_RISCV_JAL
  • R_RISCV_GOT_HI20
  • R_RISCV_TPREL_HI20

This also updates the linker relaxation test to use -NEXT to check all the output lines.

There are lots more emitted relocations, not only due to JAL being relaxable, but branches are now also marked linker relaxable because they can be turned into b<cc>; jal during assembly relaxation, which may also be marked relaxable.

Since this code was last reviewed, more relaxations have been added for
existing standard relocations that LLVM didn't have marked as relaxable.
This code ensures that LLVM marks the following relocations (and their
respective fixups) as relaxable:
- `R_RISCV_JAL`
- `R_RISCV_GOT_HI20`
- `R_RISCV_TPREL_HI20`
- `R_RISCV_TLSDESC_HI20`
@llvmbot llvmbot added backend:RISC-V llvm:mc Machine (object) code labels Jul 30, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 30, 2025

@llvm/pr-subscribers-bolt
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-risc-v

Author: Sam Elliott (lenary)

Changes

Since this code was last reviewed, more relaxations have been added to the psABI for existing standard relocations that LLVM didn't have marked as relaxable.

This change ensures that LLVM marks the following relocations (and their respective fixups) as relaxable:

  • R_RISCV_JAL
  • R_RISCV_GOT_HI20
  • R_RISCV_TPREL_HI20
  • R_RISCV_TLSDESC_HI20

This also updates the linker relaxation test to use -NEXT to check all the output lines.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp (+6-3)
  • (modified) llvm/test/MC/RISCV/linker-relaxation.s (+72-40)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index cbeabdddb9371..c7135c8e7ab7f 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -628,9 +628,6 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
         llvm_unreachable("VK_TPREL_LO used with unexpected instruction format");
       RelaxCandidate = true;
       break;
-    case ELF::R_RISCV_TPREL_HI20:
-      RelaxCandidate = true;
-      break;
     case ELF::R_RISCV_CALL_PLT:
       FixupKind = RISCV::fixup_riscv_call_plt;
       RelaxCandidate = true;
@@ -639,11 +636,17 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
       FixupKind = RISCV::fixup_riscv_qc_abs20_u;
       RelaxCandidate = true;
       break;
+    case ELF::R_RISCV_GOT_HI20:
+    case ELF::R_RISCV_TPREL_HI20:
+    case ELF::R_RISCV_TLSDESC_HI20:
+      RelaxCandidate = true;
+      break;
     }
   } else if (Kind == MCExpr::SymbolRef || Kind == MCExpr::Binary) {
     // FIXME: Sub kind binary exprs have chance of underflow.
     if (MIFrm == RISCVII::InstFormatJ) {
       FixupKind = RISCV::fixup_riscv_jal;
+      RelaxCandidate = true;
     } else if (MIFrm == RISCVII::InstFormatB) {
       FixupKind = RISCV::fixup_riscv_branch;
     } else if (MIFrm == RISCVII::InstFormatCJ) {
diff --git a/llvm/test/MC/RISCV/linker-relaxation.s b/llvm/test/MC/RISCV/linker-relaxation.s
index 6b0685baaa69e..c5c4e4877ff2e 100644
--- a/llvm/test/MC/RISCV/linker-relaxation.s
+++ b/llvm/test/MC/RISCV/linker-relaxation.s
@@ -8,49 +8,57 @@
 # RUN:     | llvm-readobj -r - | FileCheck -check-prefix=NORELAX-RELOC %s
 
 .long foo
+# NORELAX-RELOC: R_RISCV_32 foo 0x0
+# RELAX-RELOC: R_RISCV_32 foo 0x0
 
 call foo
-# NORELAX-RELOC: R_RISCV_CALL_PLT foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_CALL_PLT foo 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_CALL_PLT foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_CALL_PLT foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+jal foo
+# NORELAX-RELOC-NEXT: R_RISCV_JAL foo 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_JAL foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 lui t1, %hi(foo)
-# NORELAX-RELOC: R_RISCV_HI20 foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_HI20 foo 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_HI20 foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_HI20 foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 addi t1, t1, %lo(foo)
-# NORELAX-RELOC: R_RISCV_LO12_I foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_LO12_I foo 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_LO12_I foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_LO12_I foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 sb t1, %lo(foo)(a2)
-# NORELAX-RELOC: R_RISCV_LO12_S foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_LO12_S foo 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_LO12_S foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_LO12_S foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 1:
 auipc t1, %pcrel_hi(foo)
-# NORELAX-RELOC: R_RISCV_PCREL_HI20 foo 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_PCREL_HI20 foo 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_HI20 foo 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_HI20 foo 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 addi t1, t1, %pcrel_lo(1b)
-# NORELAX-RELOC: R_RISCV_PCREL_LO12_I .Ltmp0 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_I .Ltmp0 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_I .Ltmp0 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_I .Ltmp0 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 sb t1, %pcrel_lo(1b)(a2)
-# NORELAX-RELOC: R_RISCV_PCREL_LO12_S .Ltmp0 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_S .Ltmp0 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_S .Ltmp0 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_S .Ltmp0 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 
 # Check behaviour when a locally defined symbol is referenced.
@@ -63,64 +71,88 @@ call bar
 # NORELAX-RELOC-NOT: R_RISCV_CALL
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
 # RELAX-RELOC-NEXT: R_RISCV_CALL_PLT bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+jal bar
+# NORELAX-RELOC-NOT: R_RISCV_JAL
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_JAL bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 beq s1, s1, bar
 # NORELAX-RELOC-NOT: R_RISCV_BRANCH
 # RELAX-RELOC-NEXT: R_RISCV_BRANCH bar 0x0
 
 lui t1, %hi(bar)
-# NORELAX-RELOC: R_RISCV_HI20 bar 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_HI20 bar 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_HI20 bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_HI20 bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 addi t1, t1, %lo(bar)
-# NORELAX-RELOC: R_RISCV_LO12_I bar 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_LO12_I bar 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_LO12_I bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_LO12_I bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 sb t1, %lo(bar)(a2)
-# NORELAX-RELOC: R_RISCV_LO12_S bar 0x0
+# NORELAX-RELOC-NEXT: R_RISCV_LO12_S bar 0x0
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_LO12_S bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_LO12_S bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 2:
 auipc t1, %pcrel_hi(bar)
 # NORELAX-RELOC-NOT: R_RISCV_PCREL_HI20
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_HI20 bar 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_HI20 bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 addi t1, t1, %pcrel_lo(2b)
 # NORELAX-RELOC-NOT: R_RISCV_PCREL_LO12_I
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_I .Ltmp1 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_I .Ltmp1 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 sb t1, %pcrel_lo(2b)(a2)
 # NORELAX-RELOC-NOT: R_RISCV_PCREL_LO12_S
 # NORELAX-RELOC-NOT: R_RISCV_RELAX
-# RELAX-RELOC: R_RISCV_PCREL_LO12_S .Ltmp1 0x0
-# RELAX-RELOC: R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: R_RISCV_PCREL_LO12_S .Ltmp1 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+auipc t1, %got_pcrel_hi(bar)
+# NORELAX-RELOC-NEXT: R_RISCV_GOT_HI20 bar 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_GOT_HI20 bar 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+lui t1, %tprel_hi(baz)
+# NORELAX-RELOC-NEXT: R_RISCV_TPREL_HI20 baz 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_TPREL_HI20 baz 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
+
+auipc t1, %tlsdesc_hi(baz)
+# NORELAX-RELOC-NEXT: R_RISCV_TLSDESC_HI20 baz 0x0
+# NORELAX-RELOC-NOT: R_RISCV_RELAX
+# RELAX-RELOC-NEXT: R_RISCV_TLSDESC_HI20 baz 0x0
+# RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 ## %hi/%lo on an absolute symbol (not yet defined) leads to relocations when relaxation is enabled.
 lui t2, %hi(abs)
 # NORELAX-RELOC-NOT: R_RISCV_
-# RELAX-RELOC:      R_RISCV_HI20 - 0x12345
+# RELAX-RELOC-NEXT:      R_RISCV_HI20 - 0x12345
 # RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 addi t2, t2, %lo(abs)
 # NORELAX-RELOC-NOT: R_RISCV_
-# RELAX-RELOC:      R_RISCV_LO12_I - 0x12345
+# RELAX-RELOC-NEXT:      R_RISCV_LO12_I - 0x12345
 # RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 .set abs, 0x12345
 
 lui t3, %hi(abs)
-# RELAX-RELOC:      R_RISCV_HI20 - 0x12345
+# RELAX-RELOC-NEXT:      R_RISCV_HI20 - 0x12345
 # RELAX-RELOC-NEXT: R_RISCV_RELAX - 0x0
 
 # Check that a relocation is not emitted for a symbol difference which has

; CHECK-NEXT: jalr ra
; CHECK-NEXT: j {{.*}}
; RELAX-NEXT: R_RISCV_JAL {{.*}}
; RELAX-NOT: R_RISCV_RELAX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe J is not supposed to be assembler- or linker- relaxable. Then we should not mark it as such.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Table Jump Relaxation applies to JAL and want the R_RISCV_JAL to be marked with R_RISCV_RELAX: https://riscv-non-isa.github.io/riscv-elf-psabi-doc/#_table_jump_relaxation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL :) Thanks

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even aside from that couldn't jal be relaxed to c.jal and j to c.j (with the former only being available on RV32)? I don't think that's a currently-implemented relaxation, it's only done if the original (pseudo)instruction is a call, but for those that believe in the fun that is linker relaxation it would seem an obvious one to implement.

@MaskRay
Copy link
Member

MaskRay commented Aug 2, 2025

I would prevent JAL from being assembler-relaxed to QC_E_JAL, resolving the issue. There is no interference for users who don't use the extension.

Instead, the compiler should emit qc.e.jal when the displacement might exceed JAL's range, and the linker should handles QC_E_JAL->JAL accordingly.

@lenary
Copy link
Member Author

lenary commented Aug 2, 2025

I would prevent JAL from being assembler-relaxed to QC_E_JAL, resolving the issue. There is no interference for users who don't use the extension.

Instead, the compiler should emit qc.e.jal when the displacement might exceed JAL's range, and the linker should handles QC_E_JAL->JAL accordingly.

I don't want to side-track this patch in discussions about Xqci, which is why this patch does not reference QC.E.JAL or Xqci at all. We can come back to those issues on relevant patches/issues, there are enough of them floating around.

The psABI specifically notes relaxations involving the relocations (or their respective fixups) updated in this patch.

Yes, this will expose a layout problem in the backend (#150071). That's part of the reason I want to get this landed now, so we have time to fix it before a release. This patch is not against the 21.x branch and I do not intend to backport it.

@lenary lenary requested review from asb, jrtc27 and kito-cheng August 2, 2025 07:08
@@ -1,32 +1,45 @@
;; With +relax, J below needs a relocation to ensure the target is correct
;; after linker relaxation. See https://github.com/ClangBuiltLinux/linux/issues/1965

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@lenary
Copy link
Member Author

lenary commented Aug 13, 2025

This needs quite a lot of updates, due to #152602 and riscv-non-isa/riscv-elf-psabi-doc#475 - I will get to that in the next few days, but not today.

@lenary
Copy link
Member Author

lenary commented Aug 23, 2025

This is now rebased and updated for the linker relaxable fixes recently.

This should bring us up to date with the psABI, under the assumption that riscv-non-isa/riscv-elf-psabi-doc#475 is accepted.

@lenary lenary requested a review from MaskRay August 23, 2025 00:11
lenary added 2 commits August 22, 2025 23:35
For the moment, while the abi discussion continues.
@llvmbot llvmbot added the BOLT label Aug 23, 2025
@lenary
Copy link
Member Author

lenary commented Sep 3, 2025

Ping?

The current failures are unrelated.

@lenary
Copy link
Member Author

lenary commented Oct 9, 2025

Ping

Copy link

@RobinKastberg RobinKastberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
This will help #163142 to be more useful, and also increase coverage of the testing of it.

@lenary
Copy link
Member Author

lenary commented Oct 16, 2025

Ping @MaskRay @jrtc27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants