[RISCV] Track Linker Relaxable through Assembly Relaxation #152602
+169
−18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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). I do not fully understand why the linker relaxation information has to be accurate at streaming-time if the assembler knows it may be relaxing the instruction (i.e., potentially making it longer), but I have been told this is the case. 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 `R_RISCV_BRANCH` relocations, but I do not think there is anything we can do about that. Fixes: llvm#150071
|
@llvm/pr-subscribers-backend-risc-v Author: Sam Elliott (lenary) ChangesSpan-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 I do not fully understand why the linker relaxation information has to be accurate at streaming-time if the assembler knows it may be relaxing the instruction (i.e., potentially making it longer), but I have been told this is the case. In order for this to work, the following things have to happen:
I also added more debug output for Sections/Fixups which are marked Linker Relaxable. This results in more Fixes: #150071 Full diff: https://github.com/llvm/llvm-project/pull/152602.diff 9 Files Affected:
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index 6cbdf749a6943..21da79bb0aa30 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -68,6 +68,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";
}
};
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 8c279586bb4d3..f18cbf218570d 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -461,11 +461,21 @@ void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
getAssembler().getEmitter().encodeInstruction(Inst, Data, Fixups, STI);
F->Kind = MCFragment::FT_Relaxable;
- F->STI = &STI;
- F->HasInstructions = true;
+ F->setHasInstructions(STI);
+
F->setVarContents(Data);
- F->setVarFixups(Fixups);
F->setInst(Inst);
+
+ for (auto &Fixup : Fixups) {
+ if (!Fixup.isLinkerRelaxable())
+ continue;
+ auto *Sec = F->getParent();
+ if (!Sec->isLinkerRelaxable())
+ Sec->setLinkerRelaxable();
+ F->setLinkerRelaxable();
+ }
+ F->setVarFixups(Fixups);
+
newFragment();
}
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 27ca1314074fe..7d251be29c23a 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -39,6 +39,8 @@ LLVM_DUMP_METHOD void MCSection::dump(
raw_ostream &OS = errs();
OS << "MCSection Name:" << getName();
+ if (LinkerRelaxable)
+ 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 95ec42f960105..db5a7d68b3252 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -803,6 +803,40 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F,
Asm->getWriter().recordRelocation(F, VendorFixup, VendorTarget, VendorValue);
}
+static bool fixupGetsRelaxRelocation(const MCFixupKind Kind) {
+ switch (Kind) {
+ default:
+ break;
+ case RISCV::fixup_riscv_lo12_i:
+ case RISCV::fixup_riscv_lo12_s:
+ case RISCV::fixup_riscv_hi20:
+ case RISCV::fixup_riscv_pcrel_lo12_i:
+ case RISCV::fixup_riscv_pcrel_lo12_s:
+ case RISCV::fixup_riscv_pcrel_hi20:
+ case RISCV::fixup_riscv_call_plt:
+ case RISCV::fixup_riscv_qc_abs20_u:
+ case RISCV::fixup_riscv_qc_e_32:
+ case RISCV::fixup_riscv_qc_e_call_plt:
+ case ELF::R_RISCV_TPREL_LO12_I:
+ case ELF::R_RISCV_TPREL_LO12_S:
+ case ELF::R_RISCV_TPREL_HI20:
+ return true;
+ }
+ return false;
+}
+
+void RISCVAsmBackend::maybeAddRelaxReloc(const MCFragment &F,
+ const MCFixup &Fixup) {
+ if (!Fixup.isLinkerRelaxable() || !fixupGetsRelaxRelocation(Fixup.getKind()))
+ return;
+
+ 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);
+}
+
bool RISCVAsmBackend::addReloc(const MCFragment &F, const MCFixup &Fixup,
const MCValue &Target, uint64_t &FixedValue,
bool IsResolved) {
@@ -845,25 +879,24 @@ 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.
+ if (Fixup.isLinkerRelaxable() && fixupGetsRelaxRelocation(Fixup.getKind()))
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);
+ // Some Fixups may get a RELAX relocation, record it (directly) after we
+ // add the relocation.
+ maybeAddRelaxReloc(F, Fixup);
}
return false;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index adec1ec699da0..8caee6c9ba187 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -44,6 +44,7 @@ class RISCVAsmBackend : public MCAsmBackend {
uint64_t &FixedValue, bool IsResolved);
void maybeAddVendorReloc(const MCFragment &, const MCFixup &);
+ void maybeAddRelaxReloc(const MCFragment &, const MCFixup &);
void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
uint8_t *Data, uint64_t Value, bool IsResolved) override;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index cbeabdddb9371..6adb58d0dd9e4 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -576,8 +576,25 @@ 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`.
bool RelaxCandidate = false;
+ auto AsmRelaxToLinkerRelaxable = [&]() -> void {
+ if (!STI.hasFeature(RISCV::FeatureExactAssembly))
+ RelaxCandidate = true;
+ };
+ 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 +661,23 @@ 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;
+ AsmRelaxToLinkerRelaxable();
} 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;
+ AsmRelaxToLinkerRelaxable();
} 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;
+ AsmRelaxToLinkerRelaxable();
} else if (MIFrm == RISCVII::InstFormatQC_EAI) {
FixupKind = RISCV::fixup_riscv_qc_e_32;
RelaxCandidate = true;
@@ -670,9 +692,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 ddc0c7d2fe6b1..e5481bc3d822d 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 Align Size:0+0 []
# CHECK-NEXT: Align:4 Fill:0 FillLen:1 MaxBytesToEmit:4 Nops
# CHECK-NEXT: Symbol @0 .text
# 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 LinkerRelaxable Size:0+4 []
# CHECK-NEXT: Align:8 Fill:0 FillLen:1 MaxBytesToEmit:8 Nops
diff --git a/llvm/test/MC/RISCV/align.s b/llvm/test/MC/RISCV/align.s
index da3b1aa9b637a..93e54a3c61d96 100644
--- a/llvm/test/MC/RISCV/align.s
+++ b/llvm/test/MC/RISCV/align.s
@@ -137,14 +137,18 @@ data2:
## Branches crossing the linker-relaxable R_RISCV_ALIGN need relocations.
# RELAX-RELOC: .rela.text3 {
-# RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
-# RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
-# RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x0 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
+# RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x10 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: }
# C-OR-ZCA-EXT-RELAX-RELOC: .rela.text3 {
-# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
-# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
-# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x0 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x10 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: }
.section .text3, "ax"
bnez t1, 1f
@@ -161,9 +165,11 @@ data2:
# RELAX-RELOC: .rela.text3a {
# RELAX-RELOC-NEXT: 0x0 R_RISCV_CALL_PLT foo 0x0
# RELAX-RELOC-NEXT: 0x0 R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: 0x8 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0x10 R_RISCV_ALIGN - 0x4
# RELAX-RELOC-NEXT: 0x14 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x18 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: }
.section .text3a, "ax"
call foo
@@ -177,9 +183,11 @@ bnez t1, 2b
## .text3 with a call at the end
# RELAX-RELOC: .rela.text3b {
+# RELAX-RELOC-NEXT: 0x0 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
# RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x10 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0x14 R_RISCV_CALL_PLT foo 0x0
# RELAX-RELOC-NEXT: 0x14 R_RISCV_RELAX - 0x0
# RELAX-RELOC-NEXT: }
diff --git a/llvm/test/MC/RISCV/linker-relaxation-pr150071.s b/llvm/test/MC/RISCV/linker-relaxation-pr150071.s
new file mode 100644
index 0000000000000..7d5297fbaa937
--- /dev/null
+++ b/llvm/test/MC/RISCV/linker-relaxation-pr150071.s
@@ -0,0 +1,18 @@
+# RUN: llvm-mc --triple=riscv32 -mattr=+relax,+experimental-xqcilb \
+# RUN: %s -filetype=obj -o - -riscv-add-build-attributes \
+# RUN: | llvm-objdump -dr - \
+# RUN: | FileCheck %s
+
+.global foo
+
+bar:
+ jal x1, foo
+# CHECK: qc.e.jal 0x0 <bar>
+# CHECK-NEXT: R_RISCV_VENDOR QUALCOMM
+# CHECK-NEXT: R_RISCV_CUSTOM195 foo
+# CHECK-NEXT: R_RISCV_RELAX *ABS*
+ bne a0, a1, bar
+# CHECK-NEXT: bne a0, a1, 0x6 <bar+0x6>
+# CHECK-NEXT: R_RISCV_BRANCH bar
+ ret
+# CHECK-NEXT: ret
|
|
@llvm/pr-subscribers-mc Author: Sam Elliott (lenary) ChangesSpan-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 I do not fully understand why the linker relaxation information has to be accurate at streaming-time if the assembler knows it may be relaxing the instruction (i.e., potentially making it longer), but I have been told this is the case. In order for this to work, the following things have to happen:
I also added more debug output for Sections/Fixups which are marked Linker Relaxable. This results in more Fixes: #150071 Full diff: https://github.com/llvm/llvm-project/pull/152602.diff 9 Files Affected:
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index 6cbdf749a6943..21da79bb0aa30 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -68,6 +68,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";
}
};
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 8c279586bb4d3..f18cbf218570d 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -461,11 +461,21 @@ void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
getAssembler().getEmitter().encodeInstruction(Inst, Data, Fixups, STI);
F->Kind = MCFragment::FT_Relaxable;
- F->STI = &STI;
- F->HasInstructions = true;
+ F->setHasInstructions(STI);
+
F->setVarContents(Data);
- F->setVarFixups(Fixups);
F->setInst(Inst);
+
+ for (auto &Fixup : Fixups) {
+ if (!Fixup.isLinkerRelaxable())
+ continue;
+ auto *Sec = F->getParent();
+ if (!Sec->isLinkerRelaxable())
+ Sec->setLinkerRelaxable();
+ F->setLinkerRelaxable();
+ }
+ F->setVarFixups(Fixups);
+
newFragment();
}
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 27ca1314074fe..7d251be29c23a 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -39,6 +39,8 @@ LLVM_DUMP_METHOD void MCSection::dump(
raw_ostream &OS = errs();
OS << "MCSection Name:" << getName();
+ if (LinkerRelaxable)
+ 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 95ec42f960105..db5a7d68b3252 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -803,6 +803,40 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F,
Asm->getWriter().recordRelocation(F, VendorFixup, VendorTarget, VendorValue);
}
+static bool fixupGetsRelaxRelocation(const MCFixupKind Kind) {
+ switch (Kind) {
+ default:
+ break;
+ case RISCV::fixup_riscv_lo12_i:
+ case RISCV::fixup_riscv_lo12_s:
+ case RISCV::fixup_riscv_hi20:
+ case RISCV::fixup_riscv_pcrel_lo12_i:
+ case RISCV::fixup_riscv_pcrel_lo12_s:
+ case RISCV::fixup_riscv_pcrel_hi20:
+ case RISCV::fixup_riscv_call_plt:
+ case RISCV::fixup_riscv_qc_abs20_u:
+ case RISCV::fixup_riscv_qc_e_32:
+ case RISCV::fixup_riscv_qc_e_call_plt:
+ case ELF::R_RISCV_TPREL_LO12_I:
+ case ELF::R_RISCV_TPREL_LO12_S:
+ case ELF::R_RISCV_TPREL_HI20:
+ return true;
+ }
+ return false;
+}
+
+void RISCVAsmBackend::maybeAddRelaxReloc(const MCFragment &F,
+ const MCFixup &Fixup) {
+ if (!Fixup.isLinkerRelaxable() || !fixupGetsRelaxRelocation(Fixup.getKind()))
+ return;
+
+ 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);
+}
+
bool RISCVAsmBackend::addReloc(const MCFragment &F, const MCFixup &Fixup,
const MCValue &Target, uint64_t &FixedValue,
bool IsResolved) {
@@ -845,25 +879,24 @@ 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.
+ if (Fixup.isLinkerRelaxable() && fixupGetsRelaxRelocation(Fixup.getKind()))
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);
+ // Some Fixups may get a RELAX relocation, record it (directly) after we
+ // add the relocation.
+ maybeAddRelaxReloc(F, Fixup);
}
return false;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index adec1ec699da0..8caee6c9ba187 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -44,6 +44,7 @@ class RISCVAsmBackend : public MCAsmBackend {
uint64_t &FixedValue, bool IsResolved);
void maybeAddVendorReloc(const MCFragment &, const MCFixup &);
+ void maybeAddRelaxReloc(const MCFragment &, const MCFixup &);
void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
uint8_t *Data, uint64_t Value, bool IsResolved) override;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index cbeabdddb9371..6adb58d0dd9e4 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -576,8 +576,25 @@ 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`.
bool RelaxCandidate = false;
+ auto AsmRelaxToLinkerRelaxable = [&]() -> void {
+ if (!STI.hasFeature(RISCV::FeatureExactAssembly))
+ RelaxCandidate = true;
+ };
+ 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 +661,23 @@ 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;
+ AsmRelaxToLinkerRelaxable();
} 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;
+ AsmRelaxToLinkerRelaxable();
} 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;
+ AsmRelaxToLinkerRelaxable();
} else if (MIFrm == RISCVII::InstFormatQC_EAI) {
FixupKind = RISCV::fixup_riscv_qc_e_32;
RelaxCandidate = true;
@@ -670,9 +692,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 ddc0c7d2fe6b1..e5481bc3d822d 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 Align Size:0+0 []
# CHECK-NEXT: Align:4 Fill:0 FillLen:1 MaxBytesToEmit:4 Nops
# CHECK-NEXT: Symbol @0 .text
# 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 LinkerRelaxable Size:0+4 []
# CHECK-NEXT: Align:8 Fill:0 FillLen:1 MaxBytesToEmit:8 Nops
diff --git a/llvm/test/MC/RISCV/align.s b/llvm/test/MC/RISCV/align.s
index da3b1aa9b637a..93e54a3c61d96 100644
--- a/llvm/test/MC/RISCV/align.s
+++ b/llvm/test/MC/RISCV/align.s
@@ -137,14 +137,18 @@ data2:
## Branches crossing the linker-relaxable R_RISCV_ALIGN need relocations.
# RELAX-RELOC: .rela.text3 {
-# RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
-# RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
-# RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x0 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
+# RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x10 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: }
# C-OR-ZCA-EXT-RELAX-RELOC: .rela.text3 {
-# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
-# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
-# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x0 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: 0x10 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# C-OR-ZCA-EXT-RELAX-RELOC-NEXT: }
.section .text3, "ax"
bnez t1, 1f
@@ -161,9 +165,11 @@ data2:
# RELAX-RELOC: .rela.text3a {
# RELAX-RELOC-NEXT: 0x0 R_RISCV_CALL_PLT foo 0x0
# RELAX-RELOC-NEXT: 0x0 R_RISCV_RELAX - 0x0
+# RELAX-RELOC-NEXT: 0x8 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0x10 R_RISCV_ALIGN - 0x4
# RELAX-RELOC-NEXT: 0x14 R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x18 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: }
.section .text3a, "ax"
call foo
@@ -177,9 +183,11 @@ bnez t1, 2b
## .text3 with a call at the end
# RELAX-RELOC: .rela.text3b {
+# RELAX-RELOC-NEXT: 0x0 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0x4 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0x8 R_RISCV_ALIGN - 0x4
# RELAX-RELOC-NEXT: 0xC R_RISCV_BRANCH .Ltmp[[#]] 0x0
+# RELAX-RELOC-NEXT: 0x10 R_RISCV_BRANCH .Ltmp[[#]] 0x0
# RELAX-RELOC-NEXT: 0x14 R_RISCV_CALL_PLT foo 0x0
# RELAX-RELOC-NEXT: 0x14 R_RISCV_RELAX - 0x0
# RELAX-RELOC-NEXT: }
diff --git a/llvm/test/MC/RISCV/linker-relaxation-pr150071.s b/llvm/test/MC/RISCV/linker-relaxation-pr150071.s
new file mode 100644
index 0000000000000..7d5297fbaa937
--- /dev/null
+++ b/llvm/test/MC/RISCV/linker-relaxation-pr150071.s
@@ -0,0 +1,18 @@
+# RUN: llvm-mc --triple=riscv32 -mattr=+relax,+experimental-xqcilb \
+# RUN: %s -filetype=obj -o - -riscv-add-build-attributes \
+# RUN: | llvm-objdump -dr - \
+# RUN: | FileCheck %s
+
+.global foo
+
+bar:
+ jal x1, foo
+# CHECK: qc.e.jal 0x0 <bar>
+# CHECK-NEXT: R_RISCV_VENDOR QUALCOMM
+# CHECK-NEXT: R_RISCV_CUSTOM195 foo
+# CHECK-NEXT: R_RISCV_RELAX *ABS*
+ bne a0, a1, bar
+# CHECK-NEXT: bne a0, a1, 0x6 <bar+0x6>
+# CHECK-NEXT: R_RISCV_BRANCH bar
+ ret
+# CHECK-NEXT: ret
|
MaskRay
reviewed
Aug 8, 2025
| @@ -803,6 +803,41 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, | |||
| Asm->getWriter().recordRelocation(F, VendorFixup, VendorTarget, VendorValue); | |||
| } | |||
|
|
|||
| static bool fixupGetsRelaxRelocation(const MCFixupKind Kind) { | |||
| switch (Kind) { | |||
There was a problem hiding this comment.
Why does any fixup need special treatment? check-llvm-mc still passes if fixupGetsRelaxRelocation returns false.
There was a problem hiding this comment.
if we don't have this, we'll end up with R_RISCV_RELAX on compressed instructions which are in-range but require a relocation (i.e. reference something close but the other side of something relaxable). I'll make a testcase for this tomorrow.
There was a problem hiding this comment.
I added more to the linker-relaxation-pr150071.s test which will fail if this either always returns true or false.
The problem is about the over-approximation of RelaxCandidate = true in the MCCodeEmitter. Now, many more relocations are marked as linkerrelaxable, which is good, but those might be pc-relative resolvable, like a jump over non-relaxable data. In these cases we want to allow the fixup itself to resolve if possible (these will only be left if they are in range, when xqcilb is enabled).
And then we want to make sure, if a fixup is not resolved, that only fixups expecting R_RISCV_RELAX get that relocation, even if the fixup is marked linker relaxable.
There was a problem hiding this comment.
Encoding the complement set is clearer.
static bool fixupGetsRelaxRelocation(const MCFixupKind Kind) {
switch (Kind) {
case RISCV::fixup_riscv_rvc_jump:
case RISCV::fixup_riscv_rvc_branch:
case RISCV::fixup_riscv_jal:
return false;
}
return true;
}We are essentially listing short-range fixups that suppress RELAX relocations. Perhaps a function name like fixupSuppressRelax is clearer.
MaskRay
reviewed
Aug 8, 2025
| @@ -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 | |||
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
MaskRay
reviewed
Aug 9, 2025
| // `R_RISCV_RELAX` relocation. | ||
| // | ||
| // The actual emission of `R_RISCV_RELAX` will be handled in | ||
| // `RISCVAsmBackend`. |
There was a problem hiding this comment.
RISCVAsmBackend::applyFixup
(make it clearer)
| FixedValueA); | ||
| // Some Fixups may get a RELAX relocation, record it (directly) after we | ||
| // add the relocation. | ||
| maybeAddRelaxReloc(F, Fixup); |
There was a problem hiding this comment.
The outlined function seems unnecessary. The previous open coding seems clearer.
// If linker relaxation is enabled and supported by the current fixup, then we
// always want to generate a relocation.
bool needsRelax = Fixup.isLinkerRelaxable() && fixupGetsRelaxRelocation(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
// add the relocation.
maybeAddVendorReloc(F, Fixup);
Asm->getWriter().recordRelocation(F, Fixup, Target, FixedValue);
if (needsRelax) {
...
}
}| @@ -803,6 +803,41 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, | |||
| Asm->getWriter().recordRelocation(F, VendorFixup, VendorTarget, VendorValue); | |||
| } | |||
|
|
|||
| static bool fixupGetsRelaxRelocation(const MCFixupKind Kind) { | |||
| switch (Kind) { | |||
There was a problem hiding this comment.
Encoding the complement set is clearer.
static bool fixupGetsRelaxRelocation(const MCFixupKind Kind) {
switch (Kind) {
case RISCV::fixup_riscv_rvc_jump:
case RISCV::fixup_riscv_rvc_branch:
case RISCV::fixup_riscv_jal:
return false;
}
return true;
}We are essentially listing short-range fixups that suppress RELAX relocations. Perhaps a function name like fixupSuppressRelax is clearer.
| @@ -0,0 +1,79 @@ | |||
| # RUN: llvm-mc --triple=riscv32 -mattr=+relax,+experimental-xqcilb,+experimental-xqcibi \ | |||
There was a problem hiding this comment.
Suggest that we don't use pr... in the test filename. Use linker-relaxation-xqcibi.s or xqcibi-linker-relaxation.s
We can specify "## PR150071" as a comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.jalbecomingqc.e.jal, orbnegetting turned intobeq; jal(thejalis linker relaxable).I do not fully understand why the linker relaxation information has to be accurate at streaming-time if the assembler knows it may be relaxing the instruction (i.e., potentially making it longer), but I have been told this is the case.
In order for this to work, the following things have to happen:
RelaxCandidate = truein RISCVMCCodeEmitter.R_RISCV_RELAXrelocation, 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).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).Fixes: #150071