[llvm] d5893fc - MCValue: Replace MCSymbolRefExpr members with MCSymbol
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sat Apr 5 21:02:13 PDT 2025
Author: Fangrui Song
Date: 2025-04-05T21:02:08-07:00
New Revision: d5893fc2a7e1191afdb4940469ec9371a319b114
URL: https://github.com/llvm/llvm-project/commit/d5893fc2a7e1191afdb4940469ec9371a319b114
DIFF: https://github.com/llvm/llvm-project/commit/d5893fc2a7e1191afdb4940469ec9371a319b114.diff
LOG: MCValue: Replace MCSymbolRefExpr members with MCSymbol
Commit 0999cbd0b9ed8aa893cce10d681dec6d54b200ad (2014) introduced
`MCValue::RefKind` for AArch64 ELF as a clean approach to encode the
relocation specifier.
Following numerous migration commits, direct references to getSymA and
getSymB have been eliminated. This allows us to seamlessly update SymA
and SymB, replacing MCSymbolRefExpr with MCSymbol.
Removeing reliance on a MCAssembler::evaluateFixup hack
(`if (Target.SymSpecifier || SA.isUndefined()) {` (previosuly
`if (A->getKind() != MCSymbolRefExpr::VK_None || SA.isUndefined()) {`))
requires 38c3ad36be1facbe6db2dede7e93c0f12fb4e1dc and 4182d2dcb5ecbfc34d41a6cd11810cd36844eddb
Revert the temporary RISCV/LoongArch workaround
(7e62715e0cd433ed97749549c6582c4e1aa689a3) during migration.
MCAssembler::evaluateFixup needs an extra `!Add->isAbsolute()` case
to support `movq abs at GOTPCREL(%rip), %rax; abs = 42` in llvm/test/MC/ELF/relocation-alias.s
(ELFObjectWriter::isSymbolRefDifferenceFullyResolvedImpl asserts if
called on an absolute symbol).
Added:
Modified:
llvm/include/llvm/MC/MCExpr.h
llvm/include/llvm/MC/MCValue.h
llvm/lib/MC/MCAssembler.cpp
llvm/lib/MC/MCExpr.cpp
llvm/lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp
llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
llvm/test/MC/AArch64/elf-reloc-ptrauth.s
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCExpr.h b/llvm/include/llvm/MC/MCExpr.h
index 0d7a961c364db..6311a134cf5a7 100644
--- a/llvm/include/llvm/MC/MCExpr.h
+++ b/llvm/include/llvm/MC/MCExpr.h
@@ -133,9 +133,7 @@ class MCExpr {
/// @}
static bool evaluateSymbolicAdd(const MCAssembler *, const SectionAddrMap *,
- bool, const MCValue &,
- const MCSymbolRefExpr *,
- const MCSymbolRefExpr *, int64_t, uint32_t,
+ bool, const MCValue &, const MCValue &,
MCValue &);
};
@@ -263,6 +261,9 @@ class MCSymbolRefExpr : public MCExpr {
const MCSymbol &getSymbol() const { return *Symbol; }
+ // Some targets encode the relocation specifier within SymA using
+ // MCSymbolRefExpr::SubclassData and access it via getAccessVariant(), though
+ // this method is now deprecated.
VariantKind getKind() const {
return (VariantKind)(getSubclassData() & VariantKindMask);
}
diff --git a/llvm/include/llvm/MC/MCValue.h b/llvm/include/llvm/MC/MCValue.h
index fbe32ae5d59fe..74f68e33e8763 100644
--- a/llvm/include/llvm/MC/MCValue.h
+++ b/llvm/include/llvm/MC/MCValue.h
@@ -25,40 +25,24 @@ class raw_ostream;
// Not all targets support SymB. For PC-relative relocations, a specifier is
// typically used instead of setting SymB to DOT.
//
-// Some targets encode the relocation specifier within SymA using
-// MCSymbolRefExpr::SubclassData and access it via getAccessVariant(), though
-// this method is now deprecated.
-//
// This class must remain a simple POD value class, as it needs to reside in
// unions and similar structures.
class MCValue {
- const MCSymbolRefExpr *SymA = nullptr, *SymB = nullptr;
+ const MCSymbol *SymA = nullptr, *SymB = nullptr;
int64_t Cst = 0;
uint32_t Specifier = 0;
- // SymA has a relocation specifier. This is a workaround for targets
- // that encode specifiers within MCSymbolRefExpr::SubclassData.
- bool SymSpecifier = false;
-
- // SymB cannot have a specifier. Use getSubSym instead.
- const MCSymbolRefExpr *getSymB() const { return SymB; }
-
public:
friend class MCAssembler;
friend class MCExpr;
MCValue() = default;
int64_t getConstant() const { return Cst; }
- const MCSymbolRefExpr *getSymA() const { return SymA; }
uint32_t getRefKind() const { return Specifier; }
uint32_t getSpecifier() const { return Specifier; }
void setSpecifier(uint32_t S) { Specifier = S; }
- const MCSymbol *getAddSym() const {
- return SymA ? &SymA->getSymbol() : nullptr;
- }
- const MCSymbol *getSubSym() const {
- return SymB ? &SymB->getSymbol() : nullptr;
- }
+ const MCSymbol *getAddSym() const { return SymA; }
+ const MCSymbol *getSubSym() const { return SymB; }
/// Is this an absolute (as opposed to relocatable) value.
bool isAbsolute() const { return !SymA && !SymB; }
@@ -72,22 +56,16 @@ class MCValue {
// Get the relocation specifier from SymA. This is a workaround for targets
// that do not use MCValue::Specifier.
uint16_t getSymSpecifier() const { return Specifier; }
+ // Get the relocation specifier from SymA, or 0 when SymA is null.
uint16_t getAccessVariant() const { return Specifier; }
- static MCValue get(const MCSymbolRefExpr *SymA,
- const MCSymbolRefExpr *SymB = nullptr, int64_t Val = 0,
- uint32_t Specifier = 0) {
+ static MCValue get(const MCSymbol *SymA, const MCSymbol *SymB = nullptr,
+ int64_t Val = 0, uint32_t Specifier = 0) {
MCValue R;
R.Cst = Val;
R.SymA = SymA;
R.SymB = SymB;
R.Specifier = Specifier;
- assert(!(Specifier && SymA && SymA->getSpecifier()) &&
- "Specifier cannot be used with legacy SymSpecifier");
- if (!Specifier && SymA && SymA->getSpecifier()) {
- R.Specifier = SymA->getSpecifier();
- R.SymSpecifier = true;
- }
return R;
}
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 5739097cc2620..0bfb32115fe66 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -161,34 +161,23 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
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) {
- if (Target.getSubSym() || !Target.getAddSym()) {
- IsResolved = false;
- } else {
- auto &SA = *Target.getAddSym();
- if (Target.SymSpecifier || SA.isUndefined()) {
- IsResolved = false;
- } else {
- IsResolved = (FixupFlags & MCFixupKindInfo::FKF_Constant) ||
- getWriter().isSymbolRefDifferenceFullyResolvedImpl(
- *this, SA, *DF, false, true);
- }
- }
- } else {
+ 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);
}
Value = Target.getConstant();
-
- if (const MCSymbol *Add = Target.getAddSym()) {
- if (Add->isDefined())
- Value += getSymbolOffset(*Add);
- }
- if (const MCSymbol *Sub = Target.getSubSym())
- if (Sub->isDefined())
- Value -= getSymbolOffset(*Sub);
+ if (Add && Add->isDefined())
+ Value += getSymbolOffset(*Add);
+ if (Sub && Sub->isDefined())
+ Value -= getSymbolOffset(*Sub);
bool ShouldAlignPC = FixupFlags & MCFixupKindInfo::FKF_IsAlignedDownTo32Bits;
assert((ShouldAlignPC ? IsPCRel : true) &&
@@ -208,8 +197,8 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
// kind. AVR needs the fixup value to bypass the assembly time overflow with a
// relocation.
if (IsResolved) {
- auto TargetVal = MCValue::get(Target.getSymA(), Target.getSymB(), Value,
- Target.getRefKind());
+ auto TargetVal = Target;
+ TargetVal.Cst = Value;
if (Fixup.getKind() >= FirstLiteralRelocationKind ||
getBackend().shouldForceRelocation(*this, Fixup, TargetVal, STI)) {
IsResolved = false;
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index e3d4c5c27fe69..c856ef5f97203 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -436,22 +436,21 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
// early.
bool MCExpr::evaluateSymbolicAdd(const MCAssembler *Asm,
const SectionAddrMap *Addrs, bool InSet,
- const MCValue &LHS,
- const MCSymbolRefExpr *RhsAdd,
- const MCSymbolRefExpr *RhsSub, int64_t RHS_Cst,
- uint32_t RhsSpec, MCValue &Res) {
+ const MCValue &LHS, const MCValue &RHS,
+ MCValue &Res) {
const MCSymbol *LHS_A = LHS.getAddSym();
const MCSymbol *LHS_B = LHS.getSubSym();
int64_t LHS_Cst = LHS.getConstant();
- const MCSymbol *RHS_A = RhsAdd ? &RhsAdd->getSymbol() : nullptr;
- const MCSymbol *RHS_B = RhsSub ? &RhsSub->getSymbol() : nullptr;
+ const MCSymbol *RHS_A = RHS.getAddSym();
+ const MCSymbol *RHS_B = RHS.getSubSym();
+ int64_t RHS_Cst = RHS.getConstant();
// Fold the result constant immediately.
int64_t Result_Cst = LHS_Cst + RHS_Cst;
// If we have a layout, we can fold resolved
diff erences.
- if (Asm && !LHS.getSpecifier() && !RhsSpec) {
+ if (Asm && !LHS.getSpecifier() && !RHS.getSpecifier()) {
// While LHS_A-LHS_B and RHS_A-RHS_B from recursive calls have already been
// folded, reassociating terms in
// Result = (LHS_A - LHS_B + LHS_Cst) + (RHS_A - RHS_B + RHS_Cst).
@@ -472,16 +471,12 @@ bool MCExpr::evaluateSymbolicAdd(const MCAssembler *Asm,
// At this point, we have at most one additive symbol and one subtractive
// symbol -- find them.
- auto *A = LHS_A ? LHS.getSymA() : RHS_A ? RhsAdd : nullptr;
- auto *B = LHS_B ? LHS.getSymB() : RHS_B ? RhsSub : nullptr;
- if (B && B->getKind() != MCSymbolRefExpr::VK_None)
- return false;
+ auto *A = LHS_A ? LHS_A : RHS_A;
+ auto *B = LHS_B ? LHS_B : RHS_B;
auto Spec = LHS.getSpecifier();
if (!Spec)
- Spec = RhsSpec;
- Res = MCValue::get(A, B, Result_Cst);
- Res.Specifier = Spec;
- Res.SymSpecifier = 0;
+ Spec = RHS.getSpecifier();
+ Res = MCValue::get(A, B, Result_Cst, Spec);
return true;
}
@@ -532,18 +527,16 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
InSet || IsMachO)) {
if (Kind != MCSymbolRefExpr::VK_None) {
if (Res.isAbsolute()) {
- Res = MCValue::get(SRE, nullptr, 0);
+ Res = MCValue::get(&Sym, nullptr, 0, Kind);
return true;
}
// If the reference has a variant kind, we can only handle expressions
// which evaluate exactly to a single unadorned symbol. Attach the
// original VariantKind to SymA of the result.
- if (Res.getRefKind() != MCSymbolRefExpr::VK_None || !Res.getSymA() ||
- Res.getSubSym() || Res.getConstant())
+ if (Res.getRefKind() != MCSymbolRefExpr::VK_None ||
+ !Res.getAddSym() || Res.getSubSym() || Res.getConstant())
return false;
- Res = MCValue::get(
- MCSymbolRefExpr::create(Res.getAddSym(), Kind, Asm->getContext()),
- Res.getSymB(), Res.getConstant(), Res.getRefKind());
+ Res.Specifier = Kind;
}
if (!IsMachO)
return true;
@@ -565,7 +558,7 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
}
}
- Res = MCValue::get(SRE, nullptr, 0);
+ Res = MCValue::get(&Sym, nullptr, 0, Kind);
return true;
}
@@ -587,7 +580,7 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
return false;
// The cast avoids undefined behavior if the constant is INT64_MIN.
- Res = MCValue::get(Value.getSymB(), Value.getSymA(),
+ Res = MCValue::get(Value.getSubSym(), Value.getAddSym(),
-(uint64_t)Value.getConstant());
break;
case MCUnaryExpr::Not:
@@ -653,9 +646,11 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
Res = RHSValue;
return true;
}
- return evaluateSymbolicAdd(Asm, Addrs, InSet, LHSValue, RHSValue.SymA,
- RHSValue.SymB, RHSValue.Cst,
- RHSValue.SymSpecifier, Res);
+ if (LHSValue.SymB && LHSValue.Specifier)
+ return false;
+ if (RHSValue.SymB && RHSValue.Specifier)
+ return false;
+ return evaluateSymbolicAdd(Asm, Addrs, InSet, LHSValue, RHSValue, Res);
}
}
diff --git a/llvm/lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp b/llvm/lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp
index c9957c2881111..6e0415fa91264 100644
--- a/llvm/lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp
+++ b/llvm/lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp
@@ -80,8 +80,6 @@ bool AVRMCExpr::evaluateAsRelocatableImpl(MCValue &Result,
if (!Asm || !Asm->hasLayout())
return false;
- MCContext &Context = Asm->getContext();
- const MCSymbolRefExpr *Sym = nullptr;
auto Spec = AVRMCExpr::VK_None;
if (Value.getSymSpecifier() != MCSymbolRefExpr::VK_None)
return false;
@@ -90,8 +88,8 @@ bool AVRMCExpr::evaluateAsRelocatableImpl(MCValue &Result,
Spec = AVRMCExpr::VK_PM;
// TODO: don't attach specifier to MCSymbolRefExpr.
- Sym = MCSymbolRefExpr::create(Value.getAddSym(), Spec, Context);
- Result = MCValue::get(Sym, nullptr, Value.getConstant());
+ Result =
+ MCValue::get(Value.getAddSym(), nullptr, Value.getConstant(), Spec);
}
return true;
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
index c39482094db12..e74c8af2a850c 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
@@ -236,7 +236,7 @@ bool LoongArchAsmBackend::shouldInsertFixupForCodeAlign(MCAssembler &Asm,
MCSym = MCSymbolRefExpr::create(Sym, Ctx);
getSecToAlignSym()[Sec] = MCSym;
}
- return MCValue::get(MCSym, nullptr,
+ return MCValue::get(&MCSym->getSymbol(), nullptr,
MaxBytesToEmit << 8 | Log2(AF.getAlignment()));
};
@@ -497,11 +497,8 @@ bool LoongArchAsmBackend::handleAddSubRelocations(const MCAssembler &Asm,
default:
llvm_unreachable("unsupported fixup size");
}
- MCValue A = MCValue::get(
- MCSymbolRefExpr::create(Target.getAddSym(), Asm.getContext()), nullptr,
- Target.getConstant());
- MCValue B = MCValue::get(
- MCSymbolRefExpr::create(Target.getSubSym(), Asm.getContext()));
+ MCValue A = MCValue::get(Target.getAddSym(), nullptr, Target.getConstant());
+ MCValue B = MCValue::get(Target.getSubSym());
auto FA = MCFixup::create(Fixup.getOffset(), nullptr, std::get<0>(FK));
auto FB = MCFixup::create(Fixup.getOffset(), nullptr, std::get<1>(FK));
auto &Assembler = const_cast<MCAssembler &>(Asm);
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index d245b15a39210..652dd6586492d 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -656,11 +656,8 @@ bool RISCVAsmBackend::handleAddSubRelocations(const MCAssembler &Asm,
default:
llvm_unreachable("unsupported fixup size");
}
- MCValue A = MCValue::get(
- MCSymbolRefExpr::create(Target.getAddSym(), Asm.getContext()), nullptr,
- Target.getConstant());
- MCValue B = MCValue::get(
- MCSymbolRefExpr::create(Target.getSubSym(), Asm.getContext()));
+ MCValue A = MCValue::get(Target.getAddSym(), nullptr, Target.getConstant());
+ MCValue B = MCValue::get(Target.getSubSym());
auto FA = MCFixup::create(
Fixup.getOffset(), nullptr,
static_cast<MCFixupKind>(FirstLiteralRelocationKind + TA));
diff --git a/llvm/test/MC/AArch64/elf-reloc-ptrauth.s b/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
index 6f47e2d614d19..0b66811458da5 100644
--- a/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
+++ b/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
@@ -175,7 +175,7 @@ _g9:
// ERROBJ: :[[#@LINE+1]]:7: error: Cannot represent a
diff erence across sections
.quad _g9 at AUTH(ia,42) - _g8
-// ERROBJ: :[[#@LINE+1]]:7: error: Cannot represent a
diff erence across sections
+// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
.quad _g9 at AUTH(ia,42) - _g8 at AUTH(ia,42)
.quad 0
More information about the llvm-commits
mailing list