https://github.com/lenary updated
https://github.com/llvm/llvm-project/pull/153670
>From 1c327ec7190870a947792d4aaba02bd52a4b7bf4 Mon Sep 17 00:00:00 2001
From: Sam Elliott <aelli...@qti.qualcomm.com>
Date: Tue, 12 Aug 2025 09:02:48 +0100
Subject: [PATCH 1/2] [RISCV] Track Linker Relaxable through Assembly
Relaxation (#152602)
Span-dependent instructions on RISC-V interact in a complex manner with
linker relaxation. The span-dependent assembler algorithm implemented in
LLVM has to start with the smallest version of an instruction and then
only make it larger, so we compress instructions before emitting them to
the streamer.
When the instruction is streamed, the information that the instruction
(or rather, the fixup on the instruction) is linker relaxable must be
accurate, even though the assembler relaxation process may transform a
not-linker-relaxable instruction/fixup into one that that is linker
relaxable, for instance `c.jal` becoming `qc.e.jal`, or `bne` getting
turned into `beq; jal` (the `jal` is linker relaxable).
In order for this to work, the following things have to happen:
- Any instruction/fixup which might be relaxed to a linker-relaxable
instruction/fixup, gets marked as `RelaxCandidate = true` in
RISCVMCCodeEmitter.
- In RISCVAsmBackend, when emitting the `R_RISCV_RELAX` relocation, we
have to check that the relocation/fixup kind is one that may need a
relax relocation, as well as that it is marked as linker relaxable (the
latter will not be set if relaxation is disabled).
- Linker Relaxable instructions streamed to a Relaxable fragment need to
mark the fragment and its section as linker relaxable.
I also added more debug output for Sections/Fixups which are marked
Linker Relaxable.
This results in more relocations, when these PC-relative fixups cross an
instruction with a fixup that is resolved as not linker-relaxable but
caused the fragment to be marked linker relaxable at streaming time
(i.e. `c.j`).
(cherry picked from commit 9e8f7acd2b3a71dad473565a6a6f3ba51a3e6bca)
Fixes: #150071
---
llvm/lib/MC/MCExpr.cpp | 4 +
llvm/lib/MC/MCFragment.cpp | 4 +
llvm/lib/MC/MCObjectStreamer.cpp | 7 ++
llvm/lib/MC/MCSection.cpp | 2 +
.../RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 42 ++++++++--
.../RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp | 29 ++++++-
llvm/test/MC/RISCV/Relocations/mc-dump.s | 4 +-
llvm/test/MC/RISCV/xqcibi-linker-relaxation.s | 84 +++++++++++++++++++
8 files changed, 161 insertions(+), 15 deletions(-)
create mode 100644 llvm/test/MC/RISCV/xqcibi-linker-relaxation.s
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index a196591744099..243704c68000a 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -361,6 +361,10 @@ static void attemptToFoldSymbolOffsetDifference(const
MCAssembler *Asm,
if (BBeforeRelax && AAfterRelax)
return;
}
+ const auto *RF = dyn_cast<MCRelaxableFragment>(F);
+ if (RF && RF->isLinkerRelaxable()) {
+ return;
+ }
if (&*F == FA) {
// If FA and FB belong to the same subsection, the loop will find FA
and
// we can resolve the difference.
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index c59fabe5df1b9..28cddb9396467 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -70,6 +70,8 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
OS << "\n Fixup @" << F.getOffset() << " Value:";
F.getValue()->print(OS, nullptr);
OS << " Kind:" << F.getKind();
+ if (F.isLinkerRelaxable())
+ OS << " LinkerRelaxable";
}
};
@@ -113,6 +115,8 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
}
case MCFragment::FT_Relaxable: {
const auto *F = cast<MCRelaxableFragment>(this);
+ if (F->isLinkerRelaxable())
+ OS << " LinkerRelaxable";
OS << " Size:" << F->getContents().size() << ' ';
F->getInst().dump_pretty(OS);
printFixups(F->getFixups());
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 44a82f75576b6..d9c39bbedf37c 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -408,6 +408,13 @@ void MCObjectStreamer::emitInstToFragment(const MCInst
&Inst,
Inst, IF->getContentsForAppending(), Fixups, STI);
IF->doneAppending();
IF->appendFixups(Fixups);
+
+ for (auto &Fixup : Fixups) {
+ if (Fixup.isLinkerRelaxable()) {
+ IF->setLinkerRelaxable();
+ getCurrentSectionOnly()->setLinkerRelaxable();
+ }
+ }
}
#ifndef NDEBUG
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index beb472b7c7deb..a7330692571de 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -63,6 +63,8 @@ LLVM_DUMP_METHOD void MCSection::dump(
raw_ostream &OS = errs();
OS << "MCSection Name:" << getName();
+ if (isLinkerRelaxable())
+ OS << " LinkerRelaxable";
for (auto &F : *this) {
OS << '\n';
F.dump();
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index e42d6c539a349..df3028e051399 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -792,6 +792,23 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment
&F,
Asm->getWriter().recordRelocation(F, VendorFixup, VendorTarget, VendorValue);
}
+static bool relaxableFixupNeedsRelocation(const MCFixupKind Kind) {
+ // Some Fixups are marked as LinkerRelaxable by
+ // `RISCVMCCodeEmitter::getImmOpValue` only because they may be
+ // (assembly-)relaxed into a linker-relaxable instruction. This function
+ // should return `false` for those fixups so they do not get a
`R_RISCV_RELAX`
+ // relocation emitted in addition to the relocation.
+ switch (Kind) {
+ default:
+ break;
+ case RISCV::fixup_riscv_rvc_jump:
+ case RISCV::fixup_riscv_rvc_branch:
+ case RISCV::fixup_riscv_jal:
+ return false;
+ }
+ return true;
+}
+
bool RISCVAsmBackend::addReloc(const MCFragment &F, const MCFixup &Fixup,
const MCValue &Target, uint64_t &FixedValue,
bool IsResolved) {
@@ -834,25 +851,32 @@ bool RISCVAsmBackend::addReloc(const MCFragment &F, const
MCFixup &Fixup,
return false;
}
- // If linker relaxation is enabled and supported by the current relocation,
- // generate a relocation and then append a RELAX.
- if (Fixup.isLinkerRelaxable())
+ // If linker relaxation is enabled and supported by the current fixup, then
we
+ // always want to generate a relocation.
+ bool NeedsRelax = Fixup.isLinkerRelaxable() &&
+ relaxableFixupNeedsRelocation(Fixup.getKind());
+ if (NeedsRelax)
IsResolved = false;
+
if (IsResolved && Fixup.isPCRel())
IsResolved = isPCRelFixupResolved(Target.getAddSym(), F);
if (!IsResolved) {
- // Some Fixups require a vendor relocation, record it (directly) before we
+ // Some Fixups require a VENDOR relocation, record it (directly) before we
// add the relocation.
maybeAddVendorReloc(F, Fixup);
Asm->getWriter().recordRelocation(F, Fixup, Target, FixedValue);
- }
- if (Fixup.isLinkerRelaxable()) {
- auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX);
- Asm->getWriter().recordRelocation(F, FA, MCValue::get(nullptr),
- FixedValueA);
+ if (NeedsRelax) {
+ // Some Fixups get a RELAX relocation, record it (directly) after we add
+ // the relocation.
+ MCFixup RelaxFixup =
+ MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX);
+ MCValue RelaxTarget = MCValue::get(nullptr);
+ uint64_t RelaxValue;
+ Asm->getWriter().recordRelocation(F, RelaxFixup, RelaxTarget,
RelaxValue);
+ }
}
return false;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index cbeabdddb9371..717fba68b48ed 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -576,8 +576,21 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst
&MI, unsigned OpNo,
"getImmOpValue expects only expressions or immediates");
const MCExpr *Expr = MO.getExpr();
MCExpr::ExprKind Kind = Expr->getKind();
- unsigned FixupKind = RISCV::fixup_riscv_invalid;
+
+ // `RelaxCandidate` must be set to `true` in two cases:
+ // - The fixup's relocation gets a R_RISCV_RELAX relocation
+ // - The underlying instruction may be relaxed to an instruction that gets a
+ // `R_RISCV_RELAX` relocation.
+ //
+ // The actual emission of `R_RISCV_RELAX` will be handled in
+ // `RISCVAsmBackend::applyFixup`.
bool RelaxCandidate = false;
+ auto AsmRelaxToLinkerRelaxableWithFeature = [&](unsigned Feature) -> void {
+ if (!STI.hasFeature(RISCV::FeatureExactAssembly) &&
STI.hasFeature(Feature))
+ RelaxCandidate = true;
+ };
+
+ unsigned FixupKind = RISCV::fixup_riscv_invalid;
if (Kind == MCExpr::Specifier) {
const auto *RVExpr = cast<MCSpecifierExpr>(Expr);
FixupKind = RVExpr->getSpecifier();
@@ -644,18 +657,26 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst
&MI, unsigned OpNo,
// FIXME: Sub kind binary exprs have chance of underflow.
if (MIFrm == RISCVII::InstFormatJ) {
FixupKind = RISCV::fixup_riscv_jal;
+ AsmRelaxToLinkerRelaxableWithFeature(RISCV::FeatureVendorXqcilb);
} else if (MIFrm == RISCVII::InstFormatB) {
FixupKind = RISCV::fixup_riscv_branch;
+ // This might be assembler relaxed to `b<cc>; jal` but we cannot relax
+ // the `jal` again in the assembler.
} else if (MIFrm == RISCVII::InstFormatCJ) {
FixupKind = RISCV::fixup_riscv_rvc_jump;
+ AsmRelaxToLinkerRelaxableWithFeature(RISCV::FeatureVendorXqcilb);
} else if (MIFrm == RISCVII::InstFormatCB) {
FixupKind = RISCV::fixup_riscv_rvc_branch;
+ // This might be assembler relaxed to `b<cc>; jal` but we cannot relax
+ // the `jal` again in the assembler.
} else if (MIFrm == RISCVII::InstFormatCI) {
FixupKind = RISCV::fixup_riscv_rvc_imm;
} else if (MIFrm == RISCVII::InstFormatI) {
FixupKind = RISCV::fixup_riscv_12_i;
} else if (MIFrm == RISCVII::InstFormatQC_EB) {
FixupKind = RISCV::fixup_riscv_qc_e_branch;
+ // This might be assembler relaxed to `qc.e.b<cc>; jal` but we cannot
+ // relax the `jal` again in the assembler.
} else if (MIFrm == RISCVII::InstFormatQC_EAI) {
FixupKind = RISCV::fixup_riscv_qc_e_32;
RelaxCandidate = true;
@@ -670,9 +691,9 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst
&MI, unsigned OpNo,
assert(FixupKind != RISCV::fixup_riscv_invalid && "Unhandled expression!");
addFixup(Fixups, 0, Expr, FixupKind);
- // If linker relaxation is enabled and supported by this relocation, set
- // a bit so that if fixup is unresolved, a R_RISCV_RELAX relocation will be
- // appended.
+ // If linker relaxation is enabled and supported by this relocation, set a
bit
+ // so that the assembler knows the size of the instruction is not
fixed/known,
+ // and the relocation will need a R_RISCV_RELAX relocation.
if (EnableRelax && RelaxCandidate)
Fixups.back().setLinkerRelaxable();
++MCNumFixups;
diff --git a/llvm/test/MC/RISCV/Relocations/mc-dump.s
b/llvm/test/MC/RISCV/Relocations/mc-dump.s
index 24f3e67ebbdda..c646e6f54b261 100644
--- a/llvm/test/MC/RISCV/Relocations/mc-dump.s
+++ b/llvm/test/MC/RISCV/Relocations/mc-dump.s
@@ -2,12 +2,12 @@
# RUN: llvm-mc -filetype=obj --triple=riscv64 --mattr=+relax %s
-debug-only=mc-dump -o /dev/null 2>&1 | FileCheck %s
# CHECK:Sections:[
-# CHECK-NEXT:MCSection Name:.text
+# CHECK-NEXT:MCSection Name:.text LinkerRelaxable
# CHECK-NEXT:0 Data Size:0 []
# CHECK-NEXT: Symbol @0 .text
# CHECK-NEXT:0 Align Align:4 Fill:0 FillLen:1 MaxBytesToEmit:4 Nops
# CHECK-NEXT:0 Data LinkerRelaxable Size:8 [97,00,00,00,e7,80,00,00]
-# CHECK-NEXT: Fixup @0 Value:specifier(19,ext) Kind:4023
+# CHECK-NEXT: Fixup @0 Value:specifier(19,ext) Kind:4023 LinkerRelaxable
# CHECK-NEXT: Symbol @0 $x
# CHECK-NEXT:8 Align Align:8 Fill:0 FillLen:1 MaxBytesToEmit:8 Nops
# CHECK-NEXT:12 Data Size:4 [13,05,30,00]
diff --git a/llvm/test/MC/RISCV/xqcibi-linker-relaxation.s
b/llvm/test/MC/RISCV/xqcibi-linker-relaxation.s
new file mode 100644
index 0000000000000..b12585265bf1d
--- /dev/null
+++ b/llvm/test/MC/RISCV/xqcibi-linker-relaxation.s
@@ -0,0 +1,84 @@
+# RUN: llvm-mc --triple=riscv32
-mattr=+relax,+experimental-xqcilb,+experimental-xqcibi \
+# RUN: %s -filetype=obj -o - -riscv-add-build-attributes \
+# RUN: | llvm-objdump -dr -M no-aliases - \
+# RUN: | FileCheck %s
+
+## This tests that we correctly emit relocations for linker relaxation when
+## relaxing `JAL` to `QC.E.JAL`.
+
+## PR150071
+
+.global foo
+
+# CHECK-LABEL: <branch_over_relaxable>:
+branch_over_relaxable:
+ jal x1, foo
+# CHECK: qc.e.jal 0x0 <branch_over_relaxable>
+# CHECK-NEXT: R_RISCV_VENDOR QUALCOMM
+# CHECK-NEXT: R_RISCV_CUSTOM195 foo
+# CHECK-NEXT: R_RISCV_RELAX *ABS*
+ bne a0, a1, branch_over_relaxable
+# CHECK-NEXT: bne a0, a1, 0x6 <branch_over_relaxable+0x6>
+# CHECK-NEXT: R_RISCV_BRANCH branch_over_relaxable
+# CHECK-NOT: R_RISCV_RELAX
+ qc.e.bnei a0, 0x21, branch_over_relaxable
+# CHECK-NEXT: qc.e.bnei a0, 0x21, 0xa <branch_over_relaxable+0xa>
+# CHECK-NEXT: R_RISCV_VENDOR QUALCOMM
+# CHECK-NEXT: R_RISCV_CUSTOM193 branch_over_relaxable
+# CHECK-NOT: R_RISCV_RELAX
+ ret
+# CHECK-NEXT: c.jr ra
+
+# CHECK-LABEL: <short_jump_over_fixed>:
+short_jump_over_fixed:
+ nop
+# CHECK: c.nop
+ j short_jump_over_fixed
+# CHECK-NEXT: c.j 0x14 <short_jump_over_fixed+0x2>
+# CHECK-NEXT: R_RISCV_RVC_JUMP short_jump_over_fixed
+# CHECK-NOT: R_RISCV_RELAX
+ ret
+# CHECK-NEXT: c.jr ra
+
+# CHECK-LABEL: <short_jump_over_relaxable>:
+short_jump_over_relaxable:
+ call foo
+# CHECK: auipc ra, 0x0
+# CHECK-NEXT: R_RISCV_CALL_PLT foo
+# CHECK-NEXT: R_RISCV_RELAX *ABS*
+# CHECK-NEXT: jalr ra, 0x0(ra) <short_jump_over_relaxable>
+ j short_jump_over_relaxable
+# CHECK-NEXT: c.j 0x20 <short_jump_over_relaxable+0x8>
+# CHECK-NEXT: R_RISCV_RVC_JUMP short_jump_over_relaxable
+# CHECK-NOT: R_RISCV_RELAX
+ ret
+# CHECK-NEXT: c.jr ra
+
+# CHECK-LABEL: <mid_jump_over_fixed>:
+mid_jump_over_fixed:
+ nop
+# CHECK: c.nop
+ .space 0x1000
+# CHECK-NEXT: ...
+ j mid_jump_over_fixed
+# CHECK-NEXT: jal zero, 0x1026 <mid_jump_over_fixed+0x1002>
+# CHECK-NEXT: R_RISCV_JAL mid_jump_over_fixed
+# CHECK-NOT: R_RISCV_RELAX
+ ret
+# CHECK-NEXT: c.jr ra
+
+# CHECK-LABEL: <mid_jump_over_relaxable>:
+mid_jump_over_relaxable:
+ call foo
+# CHECK: auipc ra, 0x0
+# CHECK-NEXT: R_RISCV_CALL_PLT foo
+# CHECK-NEXT: R_RISCV_RELAX *ABS*
+# CHECK-NEXT: jalr ra, 0x0(ra) <mid_jump_over_relaxable>
+ .space 0x1000
+# CHECK-NEXT: ...
+ j mid_jump_over_relaxable
+# CHECK-NEXT: jal zero, 0x2034 <mid_jump_over_relaxable+0x1008>
+# CHECK-NEXT: R_RISCV_JAL mid_jump_over_relaxable
+# CHECK-NOT: R_RISCV_RELAX
+ ret
+# CHECK-NEXT: c.jr ra
>From b7280ff377111ead35a06e11017e9b286f3bd832 Mon Sep 17 00:00:00 2001
From: Sam Elliott <aelli...@qti.qualcomm.com>
Date: Thu, 14 Aug 2025 21:55:12 -0700
Subject: [PATCH 2/2] code formatting
---
llvm/lib/MC/MCExpr.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index 243704c68000a..a877eb7938ebf 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -362,9 +362,8 @@ static void attemptToFoldSymbolOffsetDifference(const
MCAssembler *Asm,
return;
}
const auto *RF = dyn_cast<MCRelaxableFragment>(F);
- if (RF && RF->isLinkerRelaxable()) {
+ if (RF && RF->isLinkerRelaxable())
return;
- }
if (&*F == FA) {
// If FA and FB belong to the same subsection, the loop will find FA
and
// we can resolve the difference.
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
