[llvm] RISCV: Remove shouldForceRelocation and unneeded relocations (PR #140692)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Fri May 23 09:28:49 PDT 2025
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/140692
>From 1b5b5a6df9b18deae38d712e26cb30ceb6872a8c Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Tue, 20 May 2025 00:48:13 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
=?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.5-bogner
---
llvm/include/llvm/MC/MCAssembler.h | 2 ++
llvm/lib/MC/MCAssembler.cpp | 4 +++
llvm/lib/MC/MCExpr.cpp | 6 ++++
.../Target/RISCV/AsmParser/RISCVAsmParser.cpp | 15 ----------
.../RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 28 +++++++++++++++----
.../RISCV/MCTargetDesc/RISCVAsmBackend.h | 8 ++++--
.../RISCV/MCTargetDesc/RISCVELFStreamer.cpp | 7 -----
.../CodeGen/RISCV/option-relax-relocation.ll | 10 +++++--
llvm/test/MC/RISCV/option-relax.s | 10 +------
9 files changed, 48 insertions(+), 42 deletions(-)
diff --git a/llvm/include/llvm/MC/MCAssembler.h b/llvm/include/llvm/MC/MCAssembler.h
index d463b40cbd142..becd7cf6e8d1b 100644
--- a/llvm/include/llvm/MC/MCAssembler.h
+++ b/llvm/include/llvm/MC/MCAssembler.h
@@ -64,6 +64,7 @@ class MCAssembler {
std::unique_ptr<MCObjectWriter> Writer;
bool HasLayout = false;
+ bool HasFinalLayout = false;
bool RelaxAll = false;
SectionListType Sections;
@@ -197,6 +198,7 @@ class MCAssembler {
void layout();
bool hasLayout() const { return HasLayout; }
+ bool hasFinalLayout() const { return HasFinalLayout; }
bool getRelaxAll() const { return RelaxAll; }
void setRelaxAll(bool Value) { RelaxAll = Value; }
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 3a974f3591631..c2e775389735a 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -897,6 +897,10 @@ void MCAssembler::layout() {
// example, to set the index fields in the symbol data).
getWriter().executePostLayoutBinding(*this);
+ // Fragments sizes are final. RISC-V style linker relaxation determines
+ // whether a PC-relative fixup is truly resolved with this flag.
+ this->HasFinalLayout = true;
+
// Evaluate and apply the fixups, generating relocation entries as necessary.
for (MCSection &Sec : *this) {
for (MCFragment &Frag : Sec) {
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index 4c159feea48f8..bfde050b5b5ed 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -390,6 +390,12 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
unsigned Count;
if (DF) {
Displacement += DF->getContents().size();
+ } else if (auto *RF = dyn_cast<MCRelaxableFragment>(FI);
+ RF && Asm->hasFinalLayout()) {
+ // A relaxable fragment has indeterminate size before finishLayout.
+ // Afterwards (during relocation generation), it can be treated as a
+ // data fragment.
+ Displacement += RF->getContents().size();
} else if (auto *AF = dyn_cast<MCAlignFragment>(FI);
AF && Layout && AF->hasEmitNops() &&
!Asm->getBackend().shouldInsertExtraNopBytesForCodeAlign(
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index f64307babab07..01704e69ddd0b 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2820,21 +2820,6 @@ bool RISCVAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
bool RISCVAsmParser::parseInstruction(ParseInstructionInfo &Info,
StringRef Name, SMLoc NameLoc,
OperandVector &Operands) {
- // Ensure that if the instruction occurs when relaxation is enabled,
- // relocations are forced for the file. Ideally this would be done when there
- // is enough information to reliably determine if the instruction itself may
- // cause relaxations. Unfortunately instruction processing stage occurs in the
- // same pass as relocation emission, so it's too late to set a 'sticky bit'
- // for the entire file.
- if (getSTI().hasFeature(RISCV::FeatureRelax)) {
- auto *Assembler = getTargetStreamer().getStreamer().getAssemblerPtr();
- if (Assembler != nullptr) {
- RISCVAsmBackend &MAB =
- static_cast<RISCVAsmBackend &>(Assembler->getBackend());
- MAB.setForceRelocs();
- }
- }
-
// Apply mnemonic aliases because the destination mnemonic may have require
// custom operand parsing. The generic tblgen'erated code does this later, at
// the start of MatchInstructionImpl(), but that's too late for custom
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index bcab114c0ecf0..aaef249976845 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -104,9 +104,8 @@ MCFixupKindInfo RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
return Infos[Kind - FirstTargetFixupKind];
}
-// If linker relaxation is enabled, or the relax option had previously been
-// enabled, always emit relocations even if the fixup can be resolved. This is
-// necessary for correctness as offsets may change during relaxation.
+// If linker relaxation is enabled, emit relocation to mark the instruction as
+// shrinkable by the linker.
bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
const MCFixup &Fixup,
const MCValue &Target,
@@ -124,7 +123,7 @@ bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
break;
}
- return STI->hasFeature(RISCV::FeatureRelax) || ForceRelocs;
+ return STI->hasFeature(RISCV::FeatureRelax);
}
bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCAssembler &,
@@ -570,6 +569,18 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
}
}
+bool RISCVAsmBackend::isPCRelFixupResolved(const MCAssembler &Asm,
+ const MCSymbol *SymA,
+ const MCFragment &F) {
+ if (!PCRelTemp)
+ PCRelTemp = Asm.getContext().createTempSymbol();
+ PCRelTemp->setFragment(const_cast<MCFragment *>(&F));
+ MCValue Res;
+ MCExpr::evaluateSymbolicAdd(&Asm, false, MCValue::get(SymA),
+ MCValue::get(nullptr, PCRelTemp), Res);
+ return !Res.getSubSym();
+}
+
bool RISCVAsmBackend::evaluateTargetFixup(
const MCAssembler &Asm, const MCFixup &Fixup, const MCFragment *DF,
const MCValue &Target, const MCSubtargetInfo *STI, uint64_t &Value) {
@@ -613,7 +624,9 @@ bool RISCVAsmBackend::evaluateTargetFixup(
Value = Asm.getSymbolOffset(SA) + AUIPCTarget.getConstant();
Value -= Asm.getFragmentOffset(*AUIPCDF) + AUIPCFixup->getOffset();
- return AUIPCFixup->getTargetKind() == RISCV::fixup_riscv_pcrel_hi20;
+ if (AUIPCFixup->getTargetKind() != RISCV::fixup_riscv_pcrel_hi20)
+ return false;
+ return isPCRelFixupResolved(Asm, AUIPCTarget.getAddSym(), *AUIPCDF);
}
bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
@@ -659,11 +672,14 @@ bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
return false;
}
+ if (IsResolved &&
+ (getFixupKindInfo(Fixup.getKind()).Flags & MCFixupKindInfo::FKF_IsPCRel))
+ IsResolved = isPCRelFixupResolved(Asm, Target.getAddSym(), F);
IsResolved = MCAsmBackend::addReloc(Asm, F, Fixup, Target, FixedValue,
IsResolved, STI);
// If linker relaxation is enabled and supported by the current relocation,
// append a RELAX relocation.
- if (Fixup.needsRelax()) {
+ if (!IsResolved && Fixup.needsRelax()) {
auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX);
Asm.getWriter().recordRelocation(Asm, &F, FA, MCValue::get(nullptr),
FixedValueA);
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index 874cf654e7eef..e9de9ac642b11 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -25,16 +25,18 @@ class RISCVAsmBackend : public MCAsmBackend {
const MCSubtargetInfo &STI;
uint8_t OSABI;
bool Is64Bit;
- bool ForceRelocs = false;
const MCTargetOptions &TargetOptions;
+ // Temporary symbol used to check whether a PC-relative fixup is resolved.
+ MCSymbol *PCRelTemp = nullptr;
+
+ bool isPCRelFixupResolved(const MCAssembler &Asm, const MCSymbol *SymA,
+ const MCFragment &F);
public:
RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit,
const MCTargetOptions &Options);
~RISCVAsmBackend() override = default;
- void setForceRelocs() { ForceRelocs = true; }
-
// Return Size with extra Nop Bytes for alignment directive in code section.
bool shouldInsertExtraNopBytesForCodeAlign(const MCAlignFragment &AF,
unsigned &Size) override;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
index 5a81c829e3a89..c654fd2b5cbe0 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
@@ -34,13 +34,6 @@ RISCVTargetELFStreamer::RISCVTargetELFStreamer(MCStreamer &S,
setTargetABI(RISCVABI::computeTargetABI(STI.getTargetTriple(), Features,
MAB.getTargetOptions().getABIName()));
setFlagsFromFeatures(STI);
- // `j label` in `.option norelax; j label; .option relax; ...; label:` needs a
- // relocation to ensure the jump target is correct after linking. This is due
- // to a limitation that shouldForceRelocation has to make the decision upfront
- // without knowing a possibly future .option relax. When RISCVAsmParser is used,
- // its ParseInstruction may call setForceRelocs as well.
- if (STI.hasFeature(RISCV::FeatureRelax))
- static_cast<RISCVAsmBackend &>(MAB).setForceRelocs();
}
RISCVELFStreamer &RISCVTargetELFStreamer::getStreamer() {
diff --git a/llvm/test/CodeGen/RISCV/option-relax-relocation.ll b/llvm/test/CodeGen/RISCV/option-relax-relocation.ll
index 3dc5aa64bb368..e4071d16fef67 100644
--- a/llvm/test/CodeGen/RISCV/option-relax-relocation.ll
+++ b/llvm/test/CodeGen/RISCV/option-relax-relocation.ll
@@ -2,7 +2,7 @@
;; after linker relaxation. See https://github.com/ClangBuiltLinux/linux/issues/1965
; RUN: llc -mtriple=riscv64 -mattr=-relax -filetype=obj < %s \
-; RUN: | llvm-objdump -d -r - | FileCheck %s
+; RUN: | llvm-objdump -d -r - | FileCheck %s --check-prefixes=CHECK,NORELAX
; RUN: llc -mtriple=riscv64 -mattr=+relax -filetype=obj < %s \
; RUN: | llvm-objdump -d -r - | FileCheck %s --check-prefixes=CHECK,RELAX
@@ -12,14 +12,20 @@
; CHECK-NEXT: R_RISCV_CALL_PLT f
; RELAX-NEXT: R_RISCV_RELAX *ABS*
; CHECK-NEXT: jalr ra
+; CHECK-NEXT: j {{.*}}
+; CHECK-NEXT: j {{.*}}
+; NORELAX-NEXT: li a0, 0x0
+; RELAX-NEXT: R_RISCV_JAL .L0
define dso_local noundef signext i32 @main() local_unnamed_addr #0 {
entry:
- callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"() #2
+ callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"()
to label %asm.fallthrough [label %label]
asm.fallthrough: ; preds = %entry
tail call void @f()
+ callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"()
+ to label %asm.fallthrough [label %label]
br label %label
label: ; preds = %asm.fallthrough, %entry
diff --git a/llvm/test/MC/RISCV/option-relax.s b/llvm/test/MC/RISCV/option-relax.s
index 8a6a929ad0241..7b27293fe727b 100644
--- a/llvm/test/MC/RISCV/option-relax.s
+++ b/llvm/test/MC/RISCV/option-relax.s
@@ -21,15 +21,11 @@
# CHECK-INST: call foo
# CHECK-RELOC: R_RISCV_CALL_PLT foo 0x0
-# CHECK-RELOC-NOT: R_RISCV_RELAX - 0x0
+# CHECK-RELOC-NOT: R_RISCV
call foo
-# CHECK-RELOC-NEXT: R_RISCV_ADD64
-# CHECK-RELOC-NEXT: R_RISCV_SUB64
.dword .L2-.L1
-# CHECK-RELOC-NEXT: R_RISCV_JAL
jal zero, .L1
-# CHECK-RELOC-NEXT: R_RISCV_BRANCH
beq s1, s1, .L1
.L2:
@@ -41,8 +37,6 @@ beq s1, s1, .L1
# CHECK-RELOC-NEXT: R_RISCV_RELAX - 0x0
call bar
-# CHECK-RELOC-NEXT: R_RISCV_ADD64
-# CHECK-RELOC-NEXT: R_RISCV_SUB64
.dword .L2-.L1
# CHECK-RELOC-NEXT: R_RISCV_JAL
jal zero, .L1
@@ -57,8 +51,6 @@ beq s1, s1, .L1
# CHECK-RELOC-NOT: R_RISCV_RELAX - 0x0
call baz
-# CHECK-RELOC-NEXT: R_RISCV_ADD64
-# CHECK-RELOC-NEXT: R_RISCV_SUB64
.dword .L2-.L1
# CHECK-RELOC-NEXT: R_RISCV_JAL
jal zero, .L1
More information about the llvm-commits
mailing list