[llvm] 5d87ebf - [MC] Refactor fixup evaluation and relocation generation
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sun Apr 13 16:22:25 PDT 2025
Author: Fangrui Song
Date: 2025-04-13T16:22:20-07:00
New Revision: 5d87ebf3ade73d43b2dc334e4d23bc86ddc47879
URL: https://github.com/llvm/llvm-project/commit/5d87ebf3ade73d43b2dc334e4d23bc86ddc47879
DIFF: https://github.com/llvm/llvm-project/commit/5d87ebf3ade73d43b2dc334e4d23bc86ddc47879.diff
LOG: [MC] Refactor fixup evaluation and relocation generation
Follow-up to commits 5710759eb390c0d5274c2a4d43967282d7df1993
and 634f9a981571eae000c1adc311014c5c64486187
- Integrate `evaluateFixup` into `recordRelocation` and inline code
within `MCAssembler::layout`, removing `handleFixup`.
- Update `fixupNeedsRelaxation` to bypass `shouldForceRelocation` when
calling `evaluateFixup`, eliminating the `WasForced` workaround for
RISC-V linker relaxation (https://reviews.llvm.org/D46350 ).
Added:
Modified:
llvm/include/llvm/MC/MCAsmBackend.h
llvm/include/llvm/MC/MCAssembler.h
llvm/lib/MC/MCAsmBackend.cpp
llvm/lib/MC/MCAssembler.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index 8391a465ea264..710e84b05ee5d 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -109,11 +109,10 @@ class MCAsmBackend {
return false;
}
- virtual bool evaluateTargetFixup(const MCAssembler &Asm,
- const MCFixup &Fixup, const MCFragment *DF,
- const MCValue &Target,
- const MCSubtargetInfo *STI, uint64_t &Value,
- bool &WasForced) {
+ virtual bool evaluateTargetFixup(const MCAssembler &Asm, const MCFixup &Fixup,
+ const MCFragment *DF, const MCValue &Target,
+ const MCSubtargetInfo *STI,
+ uint64_t &Value) {
llvm_unreachable("Need to implement hook if target has custom fixups");
}
@@ -156,8 +155,7 @@ class MCAsmBackend {
virtual bool fixupNeedsRelaxationAdvanced(const MCAssembler &,
const MCRelaxableFragment &,
const MCFixup &, const MCValue &,
- uint64_t, bool Resolved,
- bool WasForced) const;
+ uint64_t, bool Resolved) const;
/// Simple predicate for targets where !Resolved implies requiring relaxation
virtual bool fixupNeedsRelaxation(const MCFixup &Fixup,
diff --git a/llvm/include/llvm/MC/MCAssembler.h b/llvm/include/llvm/MC/MCAssembler.h
index a68eb49fda282..57143e3d59b4b 100644
--- a/llvm/include/llvm/MC/MCAssembler.h
+++ b/llvm/include/llvm/MC/MCAssembler.h
@@ -95,14 +95,11 @@ class MCAssembler {
/// evaluates to.
/// \param Value [out] On return, the value of the fixup as currently laid
/// out.
- /// \param WasForced [out] On return, the value in the fixup is set to the
- /// correct value if WasForced is true, even if evaluateFixup returns false.
- /// \return Whether the fixup value was fully resolved. This is true if the
- /// \p Value result is fixed, otherwise the value may change due to
+ /// \param RecordReloc Record relocation if needed.
/// relocation.
bool evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
MCValue &Target, const MCSubtargetInfo *STI,
- uint64_t &Value, bool &WasForced) const;
+ uint64_t &Value, bool RecordReloc) const;
/// Check whether a fixup can be satisfied, or whether it needs to be relaxed
/// (increased in size, in order to hold its value correctly).
@@ -127,9 +124,6 @@ class MCAssembler {
bool relaxCVDefRange(MCCVDefRangeFragment &DF);
bool relaxPseudoProbeAddr(MCPseudoProbeAddrFragment &DF);
- std::tuple<MCValue, uint64_t, bool>
- handleFixup(MCFragment &F, const MCFixup &Fixup, const MCSubtargetInfo *STI);
-
public:
/// Construct a new assembler instance.
//
diff --git a/llvm/lib/MC/MCAsmBackend.cpp b/llvm/lib/MC/MCAsmBackend.cpp
index dedb9c174db3c..f6113e62c0360 100644
--- a/llvm/lib/MC/MCAsmBackend.cpp
+++ b/llvm/lib/MC/MCAsmBackend.cpp
@@ -115,9 +115,11 @@ bool MCAsmBackend::shouldForceRelocation(const MCAssembler &, const MCFixup &,
return Target.getSpecifier();
}
-bool MCAsmBackend::fixupNeedsRelaxationAdvanced(
- const MCAssembler &, const MCRelaxableFragment &, const MCFixup &Fixup,
- const MCValue &, uint64_t Value, bool Resolved, bool WasForced) const {
+bool MCAsmBackend::fixupNeedsRelaxationAdvanced(const MCAssembler &,
+ const MCRelaxableFragment &,
+ const MCFixup &Fixup,
+ const MCValue &, uint64_t Value,
+ bool Resolved) const {
if (!Resolved)
return true;
return fixupNeedsRelaxation(Fixup, Value);
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index db1f95215f906..69803f0a2ed55 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -138,7 +138,7 @@ bool MCAssembler::isThumbFunc(const MCSymbol *Symbol) const {
bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
MCValue &Target, const MCSubtargetInfo *STI,
- uint64_t &Value, bool &WasForced) const {
+ uint64_t &Value, bool RecordReloc) const {
++stats::evaluateFixup;
// FIXME: This code has some duplication with recordRelocation. We should
@@ -150,47 +150,51 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
const MCExpr *Expr = Fixup.getValue();
MCContext &Ctx = getContext();
Value = 0;
- WasForced = false;
if (!Expr->evaluateAsRelocatable(Target, this)) {
Ctx.reportError(Fixup.getLoc(), "expected relocatable expression");
return true;
}
- unsigned FixupFlags = getBackend().getFixupKindInfo(Fixup.getKind()).Flags;
- if (FixupFlags & MCFixupKindInfo::FKF_IsTarget)
- return getBackend().evaluateTargetFixup(*this, Fixup, DF, Target, STI,
- Value, WasForced);
-
- const MCSymbol *Add = Target.getAddSym();
- const MCSymbol *Sub = Target.getSubSym();
- bool IsPCRel = FixupFlags & MCFixupKindInfo::FKF_IsPCRel;
bool IsResolved = false;
- if (!IsPCRel) {
- IsResolved = Target.isAbsolute();
- } else if (Add && !Sub && !Add->isUndefined() && !Add->isAbsolute()) {
- IsResolved = (FixupFlags & MCFixupKindInfo::FKF_Constant) ||
- getWriter().isSymbolRefDifferenceFullyResolvedImpl(
- *this, *Add, *DF, false, true);
+ unsigned FixupFlags = getBackend().getFixupKindInfo(Fixup.getKind()).Flags;
+ if (FixupFlags & MCFixupKindInfo::FKF_IsTarget) {
+ IsResolved =
+ getBackend().evaluateTargetFixup(*this, Fixup, DF, Target, STI, Value);
+ } else {
+ const MCSymbol *Add = Target.getAddSym();
+ const MCSymbol *Sub = Target.getSubSym();
+ Value = Target.getConstant();
+ if (Add && Add->isDefined())
+ Value += getSymbolOffset(*Add);
+ if (Sub && Sub->isDefined())
+ Value -= getSymbolOffset(*Sub);
+
+ bool IsPCRel = FixupFlags & MCFixupKindInfo::FKF_IsPCRel;
+ bool ShouldAlignPC =
+ FixupFlags & MCFixupKindInfo::FKF_IsAlignedDownTo32Bits;
+ if (IsPCRel) {
+ uint64_t Offset = getFragmentOffset(*DF) + Fixup.getOffset();
+
+ // A number of ARM fixups in Thumb mode require that the effective PC
+ // address be determined as the 32-bit aligned version of the actual
+ // offset.
+ if (ShouldAlignPC)
+ Offset &= ~0x3;
+ Value -= Offset;
+
+ if (Add && !Sub && !Add->isUndefined() && !Add->isAbsolute()) {
+ IsResolved = (FixupFlags & MCFixupKindInfo::FKF_Constant) ||
+ getWriter().isSymbolRefDifferenceFullyResolvedImpl(
+ *this, *Add, *DF, false, true);
+ }
+ } else {
+ IsResolved = Target.isAbsolute();
+ assert(!ShouldAlignPC && "FKF_IsAlignedDownTo32Bits must be PC-relative");
+ }
}
- Value = Target.getConstant();
- if (Add && Add->isDefined())
- Value += getSymbolOffset(*Add);
- if (Sub && Sub->isDefined())
- Value -= getSymbolOffset(*Sub);
-
- bool ShouldAlignPC = FixupFlags & MCFixupKindInfo::FKF_IsAlignedDownTo32Bits;
- assert((ShouldAlignPC ? IsPCRel : true) &&
- "FKF_IsAlignedDownTo32Bits is only allowed on PC-relative fixups!");
-
- if (IsPCRel) {
- uint64_t Offset = getFragmentOffset(*DF) + Fixup.getOffset();
-
- // A number of ARM fixups in Thumb mode require that the effective PC
- // address be determined as the 32-bit aligned version of the actual offset.
- if (ShouldAlignPC) Offset &= ~0x3;
- Value -= Offset;
- }
+ if (!RecordReloc)
+ return IsResolved;
// .reloc directive and the backend might force the relocation.
// Backends that customize shouldForceRelocation generally just need the fixup
@@ -200,12 +204,12 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
auto TargetVal = Target;
TargetVal.Cst = Value;
if (Fixup.getKind() >= FirstLiteralRelocationKind ||
- getBackend().shouldForceRelocation(*this, Fixup, TargetVal, STI)) {
+ getBackend().shouldForceRelocation(*this, Fixup, TargetVal, STI))
IsResolved = false;
- WasForced = true;
- }
}
-
+ if (!IsResolved)
+ getWriter().recordRelocation(const_cast<MCAssembler &>(*this), DF, Fixup,
+ Target, Value);
return IsResolved;
}
@@ -844,24 +848,6 @@ void MCAssembler::writeSectionData(raw_ostream &OS,
OS.tell() - Start == getSectionAddressSize(*Sec));
}
-std::tuple<MCValue, uint64_t, bool>
-MCAssembler::handleFixup(MCFragment &F, const MCFixup &Fixup,
- const MCSubtargetInfo *STI) {
- // Evaluate the fixup.
- MCValue Target;
- uint64_t FixedValue;
- bool WasForced;
- bool IsResolved =
- evaluateFixup(Fixup, &F, Target, STI, FixedValue, WasForced);
- if (!IsResolved) {
- // The fixup was unresolved, we need a relocation. Inform the object
- // writer of the relocation, and give it an opportunity to adjust the
- // fixup value if need be.
- getWriter().recordRelocation(*this, &F, Fixup, Target, FixedValue);
- }
- return std::make_tuple(Target, FixedValue, IsResolved);
-}
-
void MCAssembler::layout() {
assert(getBackendPtr() && "Expected assembler backend");
DEBUG_WITH_TYPE("mc-dump", {
@@ -987,10 +973,9 @@ void MCAssembler::layout() {
}
for (const MCFixup &Fixup : Fixups) {
uint64_t FixedValue;
- bool IsResolved;
MCValue Target;
- std::tie(Target, FixedValue, IsResolved) =
- handleFixup(Frag, Fixup, STI);
+ bool IsResolved = evaluateFixup(Fixup, &Frag, Target, STI, FixedValue,
+ /*RecordReloc=*/true);
getBackend().applyFixup(*this, Fixup, Target, Contents, FixedValue,
IsResolved, STI);
}
@@ -1012,11 +997,10 @@ bool MCAssembler::fixupNeedsRelaxation(const MCFixup &Fixup,
assert(getBackendPtr() && "Expected assembler backend");
MCValue Target;
uint64_t Value;
- bool WasForced;
bool Resolved = evaluateFixup(Fixup, DF, Target, DF->getSubtargetInfo(),
- Value, WasForced);
+ Value, /*RecordReloc=*/false);
return getBackend().fixupNeedsRelaxationAdvanced(*this, *DF, Fixup, Target,
- Value, Resolved, WasForced);
+ Value, Resolved);
}
bool MCAssembler::fragmentNeedsRelaxation(const MCRelaxableFragment *F) const {
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index 6e6e5cbfb377b..ff04d88db4fec 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -356,7 +356,7 @@ static bool needsInterworking(const MCAssembler &Asm, const MCSymbol *Sym,
bool ARMAsmBackend::fixupNeedsRelaxationAdvanced(
const MCAssembler &Asm, const MCRelaxableFragment &, const MCFixup &Fixup,
- const MCValue &Target, uint64_t Value, bool Resolved, bool) const {
+ const MCValue &Target, uint64_t Value, bool Resolved) const {
const MCSymbol *Sym = Target.getAddSym();
if (needsInterworking(Asm, Sym, Fixup.getTargetKind()))
return true;
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
index bd5331356b267..df2d375ac32b8 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h
@@ -57,7 +57,7 @@ class ARMAsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxationAdvanced(const MCAssembler &,
const MCRelaxableFragment &,
const MCFixup &, const MCValue &, uint64_t,
- bool, bool) const override;
+ bool) const override;
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override;
diff --git a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
index c6736baa0f32f..4643b6a811ad0 100644
--- a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
+++ b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.cpp
@@ -171,17 +171,11 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
}
}
-bool CSKYAsmBackend::fixupNeedsRelaxationAdvanced(const MCAssembler &,
- const MCRelaxableFragment &,
- const MCFixup &Fixup,
- const MCValue &,
- uint64_t Value, bool Resolved,
- const bool WasForced) const {
- // Return true if the symbol is actually unresolved.
- // Resolved could be always false when shouldForceRelocation return true.
- // We use !WasForced to indicate that the symbol is unresolved and not forced
- // by shouldForceRelocation.
- if (!Resolved && !WasForced)
+bool CSKYAsmBackend::fixupNeedsRelaxationAdvanced(
+ const MCAssembler &, const MCRelaxableFragment &, const MCFixup &Fixup,
+ const MCValue &, uint64_t Value, bool Resolved) const {
+ // Return true if the symbol is unresolved.
+ if (!Resolved)
return true;
int64_t Offset = int64_t(Value);
diff --git a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
index a419ccafe8dee..f8201dc57c2b8 100644
--- a/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
+++ b/llvm/lib/Target/CSKY/MCTargetDesc/CSKYAsmBackend.h
@@ -42,7 +42,7 @@ class CSKYAsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxationAdvanced(const MCAssembler &,
const MCRelaxableFragment &,
const MCFixup &, const MCValue &, uint64_t,
- bool, bool) const override;
+ bool) const override;
bool writeNopData(raw_ostream &OS, uint64_t Count,
const MCSubtargetInfo *STI) const override;
diff --git a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
index 37669c56ea5e6..c37953bc68a31 100644
--- a/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
+++ b/llvm/lib/Target/Hexagon/MCTargetDesc/HexagonAsmBackend.cpp
@@ -568,8 +568,8 @@ class HexagonAsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxationAdvanced(const MCAssembler &Asm,
const MCRelaxableFragment &DF,
const MCFixup &Fixup, const MCValue &,
- uint64_t Value, bool Resolved,
- bool) const override {
+ uint64_t Value,
+ bool Resolved) const override {
MCInst const &MCB = DF.getInst();
assert(HexagonMCInstrInfo::isBundle(MCB));
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index 94d3a006bf47d..410767fc3b468 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -143,19 +143,15 @@ bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(
const MCAssembler &, const MCRelaxableFragment &, const MCFixup &Fixup,
- const MCValue &, uint64_t Value, bool Resolved,
- const bool WasForced) const {
+ const MCValue &, uint64_t Value, bool Resolved) const {
if (!RelaxBranches)
return false;
int64_t Offset = int64_t(Value);
unsigned Kind = Fixup.getTargetKind();
- // Return true if the symbol is actually unresolved.
- // Resolved could be always false when shouldForceRelocation return true.
- // We use !WasForced to indicate that the symbol is unresolved and not forced
- // by shouldForceRelocation.
- if (!Resolved && !WasForced)
+ // Return true if the symbol is unresolved.
+ if (!Resolved)
return true;
switch (Kind) {
@@ -595,12 +591,9 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
}
}
-bool RISCVAsmBackend::evaluateTargetFixup(const MCAssembler &Asm,
- const MCFixup &Fixup,
- const MCFragment *DF,
- const MCValue &Target,
- const MCSubtargetInfo *STI,
- uint64_t &Value, bool &WasForced) {
+bool RISCVAsmBackend::evaluateTargetFixup(
+ const MCAssembler &Asm, const MCFixup &Fixup, const MCFragment *DF,
+ const MCValue &Target, const MCSubtargetInfo *STI, uint64_t &Value) {
const MCFixup *AUIPCFixup;
const MCFragment *AUIPCDF;
MCValue AUIPCTarget;
@@ -647,12 +640,7 @@ bool RISCVAsmBackend::evaluateTargetFixup(const MCAssembler &Asm,
Value = Asm.getSymbolOffset(SA) + AUIPCTarget.getConstant();
Value -= Asm.getFragmentOffset(*AUIPCDF) + AUIPCFixup->getOffset();
- if (shouldForceRelocation(Asm, *AUIPCFixup, AUIPCTarget, STI)) {
- WasForced = true;
- return false;
- }
-
- return true;
+ return !shouldForceRelocation(Asm, *AUIPCFixup, AUIPCTarget, STI);
}
bool RISCVAsmBackend::handleAddSubRelocations(const MCAssembler &Asm,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index 36bf50fcb2b8a..32d6128ba7182 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -49,8 +49,8 @@ class RISCVAsmBackend : public MCAsmBackend {
bool evaluateTargetFixup(const MCAssembler &Asm, const MCFixup &Fixup,
const MCFragment *DF, const MCValue &Target,
- const MCSubtargetInfo *STI, uint64_t &Value,
- bool &WasForced) override;
+ const MCSubtargetInfo *STI,
+ uint64_t &Value) override;
bool handleAddSubRelocations(const MCAssembler &Asm, const MCFragment &F,
const MCFixup &Fixup, const MCValue &Target,
@@ -71,7 +71,7 @@ class RISCVAsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxationAdvanced(const MCAssembler &,
const MCRelaxableFragment &,
const MCFixup &, const MCValue &, uint64_t,
- bool, bool) const override;
+ bool) const override;
std::optional<MCFixupKind> getFixupKind(StringRef Name) const override;
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
index 28f707eb18ae5..09b8dba1b025c 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -180,7 +180,7 @@ class X86AsmBackend : public MCAsmBackend {
bool fixupNeedsRelaxationAdvanced(const MCAssembler &,
const MCRelaxableFragment &,
const MCFixup &, const MCValue &, uint64_t,
- bool, bool) const override;
+ bool) const override;
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override;
@@ -732,7 +732,7 @@ bool X86AsmBackend::mayNeedRelaxation(const MCInst &MI,
bool X86AsmBackend::fixupNeedsRelaxationAdvanced(
const MCAssembler &, const MCRelaxableFragment &, const MCFixup &Fixup,
- const MCValue &Target, uint64_t Value, bool Resolved, bool) const {
+ const MCValue &Target, uint64_t Value, bool Resolved) const {
// If resolved, relax if the value is too big for a (signed) i8.
//
// Currently, `jmp local at plt` relaxes JMP even if the offset is small,
More information about the llvm-commits
mailing list