Skip to content

Commit d5893fc

Browse files
MaskRayMaskRay
committed
MCValue: Replace MCSymbolRefExpr members with MCSymbol
Commit 0999cbd (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 38c3ad3 and 4182d2d Revert the temporary RISCV/LoongArch workaround (7e62715) during migration. MCAssembler::evaluateFixup needs an extra `!Add->isAbsolute()` case to support `movq abs@GOTPCREL(%rip), %rax; abs = 42` in llvm/test/MC/ELF/relocation-alias.s (ELFObjectWriter::isSymbolRefDifferenceFullyResolvedImpl asserts if called on an absolute symbol).
1 parent 4182d2d commit d5893fc

File tree

8 files changed

+52
-97
lines changed

8 files changed

+52
-97
lines changed

‎llvm/include/llvm/MC/MCExpr.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,7 @@ class MCExpr {
133133
/// @}
134134

135135
static bool evaluateSymbolicAdd(const MCAssembler *, const SectionAddrMap *,
136-
bool, const MCValue &,
137-
const MCSymbolRefExpr *,
138-
const MCSymbolRefExpr *, int64_t, uint32_t,
136+
bool, const MCValue &, const MCValue &,
139137
MCValue &);
140138
};
141139

@@ -263,6 +261,9 @@ class MCSymbolRefExpr : public MCExpr {
263261

264262
const MCSymbol &getSymbol() const { return *Symbol; }
265263

264+
// Some targets encode the relocation specifier within SymA using
265+
// MCSymbolRefExpr::SubclassData and access it via getAccessVariant(), though
266+
// this method is now deprecated.
266267
VariantKind getKind() const {
267268
return (VariantKind)(getSubclassData() & VariantKindMask);
268269
}

‎llvm/include/llvm/MC/MCValue.h

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,40 +25,24 @@ class raw_ostream;
2525
// Not all targets support SymB. For PC-relative relocations, a specifier is
2626
// typically used instead of setting SymB to DOT.
2727
//
28-
// Some targets encode the relocation specifier within SymA using
29-
// MCSymbolRefExpr::SubclassData and access it via getAccessVariant(), though
30-
// this method is now deprecated.
31-
//
3228
// This class must remain a simple POD value class, as it needs to reside in
3329
// unions and similar structures.
3430
class MCValue {
35-
const MCSymbolRefExpr *SymA = nullptr, *SymB = nullptr;
31+
const MCSymbol *SymA = nullptr, *SymB = nullptr;
3632
int64_t Cst = 0;
3733
uint32_t Specifier = 0;
3834

39-
// SymA has a relocation specifier. This is a workaround for targets
40-
// that encode specifiers within MCSymbolRefExpr::SubclassData.
41-
bool SymSpecifier = false;
42-
43-
// SymB cannot have a specifier. Use getSubSym instead.
44-
const MCSymbolRefExpr *getSymB() const { return SymB; }
45-
4635
public:
4736
friend class MCAssembler;
4837
friend class MCExpr;
4938
MCValue() = default;
5039
int64_t getConstant() const { return Cst; }
51-
const MCSymbolRefExpr *getSymA() const { return SymA; }
5240
uint32_t getRefKind() const { return Specifier; }
5341
uint32_t getSpecifier() const { return Specifier; }
5442
void setSpecifier(uint32_t S) { Specifier = S; }
5543

56-
const MCSymbol *getAddSym() const {
57-
return SymA ? &SymA->getSymbol() : nullptr;
58-
}
59-
const MCSymbol *getSubSym() const {
60-
return SymB ? &SymB->getSymbol() : nullptr;
61-
}
44+
const MCSymbol *getAddSym() const { return SymA; }
45+
const MCSymbol *getSubSym() const { return SymB; }
6246

6347
/// Is this an absolute (as opposed to relocatable) value.
6448
bool isAbsolute() const { return !SymA && !SymB; }
@@ -72,22 +56,16 @@ class MCValue {
7256
// Get the relocation specifier from SymA. This is a workaround for targets
7357
// that do not use MCValue::Specifier.
7458
uint16_t getSymSpecifier() const { return Specifier; }
59+
// Get the relocation specifier from SymA, or 0 when SymA is null.
7560
uint16_t getAccessVariant() const { return Specifier; }
7661

77-
static MCValue get(const MCSymbolRefExpr *SymA,
78-
const MCSymbolRefExpr *SymB = nullptr, int64_t Val = 0,
79-
uint32_t Specifier = 0) {
62+
static MCValue get(const MCSymbol *SymA, const MCSymbol *SymB = nullptr,
63+
int64_t Val = 0, uint32_t Specifier = 0) {
8064
MCValue R;
8165
R.Cst = Val;
8266
R.SymA = SymA;
8367
R.SymB = SymB;
8468
R.Specifier = Specifier;
85-
assert(!(Specifier && SymA && SymA->getSpecifier()) &&
86-
"Specifier cannot be used with legacy SymSpecifier");
87-
if (!Specifier && SymA && SymA->getSpecifier()) {
88-
R.Specifier = SymA->getSpecifier();
89-
R.SymSpecifier = true;
90-
}
9169
return R;
9270
}
9371

‎llvm/lib/MC/MCAssembler.cpp

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -161,34 +161,23 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
161161
return getBackend().evaluateTargetFixup(*this, Fixup, DF, Target, STI,
162162
Value, WasForced);
163163

164+
const MCSymbol *Add = Target.getAddSym();
165+
const MCSymbol *Sub = Target.getSubSym();
164166
bool IsPCRel = FixupFlags & MCFixupKindInfo::FKF_IsPCRel;
165167
bool IsResolved = false;
166-
if (IsPCRel) {
167-
if (Target.getSubSym() || !Target.getAddSym()) {
168-
IsResolved = false;
169-
} else {
170-
auto &SA = *Target.getAddSym();
171-
if (Target.SymSpecifier || SA.isUndefined()) {
172-
IsResolved = false;
173-
} else {
174-
IsResolved = (FixupFlags & MCFixupKindInfo::FKF_Constant) ||
175-
getWriter().isSymbolRefDifferenceFullyResolvedImpl(
176-
*this, SA, *DF, false, true);
177-
}
178-
}
179-
} else {
168+
if (!IsPCRel) {
180169
IsResolved = Target.isAbsolute();
170+
} else if (Add && !Sub && !Add->isUndefined() && !Add->isAbsolute()) {
171+
IsResolved = (FixupFlags & MCFixupKindInfo::FKF_Constant) ||
172+
getWriter().isSymbolRefDifferenceFullyResolvedImpl(
173+
*this, *Add, *DF, false, true);
181174
}
182175

183176
Value = Target.getConstant();
184-
185-
if (const MCSymbol *Add = Target.getAddSym()) {
186-
if (Add->isDefined())
187-
Value += getSymbolOffset(*Add);
188-
}
189-
if (const MCSymbol *Sub = Target.getSubSym())
190-
if (Sub->isDefined())
191-
Value -= getSymbolOffset(*Sub);
177+
if (Add && Add->isDefined())
178+
Value += getSymbolOffset(*Add);
179+
if (Sub && Sub->isDefined())
180+
Value -= getSymbolOffset(*Sub);
192181

193182
bool ShouldAlignPC = FixupFlags & MCFixupKindInfo::FKF_IsAlignedDownTo32Bits;
194183
assert((ShouldAlignPC ? IsPCRel : true) &&
@@ -208,8 +197,8 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
208197
// kind. AVR needs the fixup value to bypass the assembly time overflow with a
209198
// relocation.
210199
if (IsResolved) {
211-
auto TargetVal = MCValue::get(Target.getSymA(), Target.getSymB(), Value,
212-
Target.getRefKind());
200+
auto TargetVal = Target;
201+
TargetVal.Cst = Value;
213202
if (Fixup.getKind() >= FirstLiteralRelocationKind ||
214203
getBackend().shouldForceRelocation(*this, Fixup, TargetVal, STI)) {
215204
IsResolved = false;

‎llvm/lib/MC/MCExpr.cpp

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -436,22 +436,21 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
436436
// early.
437437
bool MCExpr::evaluateSymbolicAdd(const MCAssembler *Asm,
438438
const SectionAddrMap *Addrs, bool InSet,
439-
const MCValue &LHS,
440-
const MCSymbolRefExpr *RhsAdd,
441-
const MCSymbolRefExpr *RhsSub, int64_t RHS_Cst,
442-
uint32_t RhsSpec, MCValue &Res) {
439+
const MCValue &LHS, const MCValue &RHS,
440+
MCValue &Res) {
443441
const MCSymbol *LHS_A = LHS.getAddSym();
444442
const MCSymbol *LHS_B = LHS.getSubSym();
445443
int64_t LHS_Cst = LHS.getConstant();
446444

447-
const MCSymbol *RHS_A = RhsAdd ? &RhsAdd->getSymbol() : nullptr;
448-
const MCSymbol *RHS_B = RhsSub ? &RhsSub->getSymbol() : nullptr;
445+
const MCSymbol *RHS_A = RHS.getAddSym();
446+
const MCSymbol *RHS_B = RHS.getSubSym();
447+
int64_t RHS_Cst = RHS.getConstant();
449448

450449
// Fold the result constant immediately.
451450
int64_t Result_Cst = LHS_Cst + RHS_Cst;
452451

453452
// If we have a layout, we can fold resolved differences.
454-
if (Asm && !LHS.getSpecifier() && !RhsSpec) {
453+
if (Asm && !LHS.getSpecifier() && !RHS.getSpecifier()) {
455454
// While LHS_A-LHS_B and RHS_A-RHS_B from recursive calls have already been
456455
// folded, reassociating terms in
457456
// Result = (LHS_A - LHS_B + LHS_Cst) + (RHS_A - RHS_B + RHS_Cst).
@@ -472,16 +471,12 @@ bool MCExpr::evaluateSymbolicAdd(const MCAssembler *Asm,
472471

473472
// At this point, we have at most one additive symbol and one subtractive
474473
// symbol -- find them.
475-
auto *A = LHS_A ? LHS.getSymA() : RHS_A ? RhsAdd : nullptr;
476-
auto *B = LHS_B ? LHS.getSymB() : RHS_B ? RhsSub : nullptr;
477-
if (B && B->getKind() != MCSymbolRefExpr::VK_None)
478-
return false;
474+
auto *A = LHS_A ? LHS_A : RHS_A;
475+
auto *B = LHS_B ? LHS_B : RHS_B;
479476
auto Spec = LHS.getSpecifier();
480477
if (!Spec)
481-
Spec = RhsSpec;
482-
Res = MCValue::get(A, B, Result_Cst);
483-
Res.Specifier = Spec;
484-
Res.SymSpecifier = 0;
478+
Spec = RHS.getSpecifier();
479+
Res = MCValue::get(A, B, Result_Cst, Spec);
485480
return true;
486481
}
487482

@@ -532,18 +527,16 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
532527
InSet || IsMachO)) {
533528
if (Kind != MCSymbolRefExpr::VK_None) {
534529
if (Res.isAbsolute()) {
535-
Res = MCValue::get(SRE, nullptr, 0);
530+
Res = MCValue::get(&Sym, nullptr, 0, Kind);
536531
return true;
537532
}
538533
// If the reference has a variant kind, we can only handle expressions
539534
// which evaluate exactly to a single unadorned symbol. Attach the
540535
// original VariantKind to SymA of the result.
541-
if (Res.getRefKind() != MCSymbolRefExpr::VK_None || !Res.getSymA() ||
542-
Res.getSubSym() || Res.getConstant())
536+
if (Res.getRefKind() != MCSymbolRefExpr::VK_None ||
537+
!Res.getAddSym() || Res.getSubSym() || Res.getConstant())
543538
return false;
544-
Res = MCValue::get(
545-
MCSymbolRefExpr::create(Res.getAddSym(), Kind, Asm->getContext()),
546-
Res.getSymB(), Res.getConstant(), Res.getRefKind());
539+
Res.Specifier = Kind;
547540
}
548541
if (!IsMachO)
549542
return true;
@@ -565,7 +558,7 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
565558
}
566559
}
567560

568-
Res = MCValue::get(SRE, nullptr, 0);
561+
Res = MCValue::get(&Sym, nullptr, 0, Kind);
569562
return true;
570563
}
571564

@@ -587,7 +580,7 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
587580
return false;
588581

589582
// The cast avoids undefined behavior if the constant is INT64_MIN.
590-
Res = MCValue::get(Value.getSymB(), Value.getSymA(),
583+
Res = MCValue::get(Value.getSubSym(), Value.getAddSym(),
591584
-(uint64_t)Value.getConstant());
592585
break;
593586
case MCUnaryExpr::Not:
@@ -653,9 +646,11 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
653646
Res = RHSValue;
654647
return true;
655648
}
656-
return evaluateSymbolicAdd(Asm, Addrs, InSet, LHSValue, RHSValue.SymA,
657-
RHSValue.SymB, RHSValue.Cst,
658-
RHSValue.SymSpecifier, Res);
649+
if (LHSValue.SymB && LHSValue.Specifier)
650+
return false;
651+
if (RHSValue.SymB && RHSValue.Specifier)
652+
return false;
653+
return evaluateSymbolicAdd(Asm, Addrs, InSet, LHSValue, RHSValue, Res);
659654
}
660655
}
661656

‎llvm/lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,6 @@ bool AVRMCExpr::evaluateAsRelocatableImpl(MCValue &Result,
8080
if (!Asm || !Asm->hasLayout())
8181
return false;
8282

83-
MCContext &Context = Asm->getContext();
84-
const MCSymbolRefExpr *Sym = nullptr;
8583
auto Spec = AVRMCExpr::VK_None;
8684
if (Value.getSymSpecifier() != MCSymbolRefExpr::VK_None)
8785
return false;
@@ -90,8 +88,8 @@ bool AVRMCExpr::evaluateAsRelocatableImpl(MCValue &Result,
9088
Spec = AVRMCExpr::VK_PM;
9189

9290
// TODO: don't attach specifier to MCSymbolRefExpr.
93-
Sym = MCSymbolRefExpr::create(Value.getAddSym(), Spec, Context);
94-
Result = MCValue::get(Sym, nullptr, Value.getConstant());
91+
Result =
92+
MCValue::get(Value.getAddSym(), nullptr, Value.getConstant(), Spec);
9593
}
9694

9795
return true;

‎llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ bool LoongArchAsmBackend::shouldInsertFixupForCodeAlign(MCAssembler &Asm,
236236
MCSym = MCSymbolRefExpr::create(Sym, Ctx);
237237
getSecToAlignSym()[Sec] = MCSym;
238238
}
239-
return MCValue::get(MCSym, nullptr,
239+
return MCValue::get(&MCSym->getSymbol(), nullptr,
240240
MaxBytesToEmit << 8 | Log2(AF.getAlignment()));
241241
};
242242

@@ -497,11 +497,8 @@ bool LoongArchAsmBackend::handleAddSubRelocations(const MCAssembler &Asm,
497497
default:
498498
llvm_unreachable("unsupported fixup size");
499499
}
500-
MCValue A = MCValue::get(
501-
MCSymbolRefExpr::create(Target.getAddSym(), Asm.getContext()), nullptr,
502-
Target.getConstant());
503-
MCValue B = MCValue::get(
504-
MCSymbolRefExpr::create(Target.getSubSym(), Asm.getContext()));
500+
MCValue A = MCValue::get(Target.getAddSym(), nullptr, Target.getConstant());
501+
MCValue B = MCValue::get(Target.getSubSym());
505502
auto FA = MCFixup::create(Fixup.getOffset(), nullptr, std::get<0>(FK));
506503
auto FB = MCFixup::create(Fixup.getOffset(), nullptr, std::get<1>(FK));
507504
auto &Assembler = const_cast<MCAssembler &>(Asm);

‎llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -656,11 +656,8 @@ bool RISCVAsmBackend::handleAddSubRelocations(const MCAssembler &Asm,
656656
default:
657657
llvm_unreachable("unsupported fixup size");
658658
}
659-
MCValue A = MCValue::get(
660-
MCSymbolRefExpr::create(Target.getAddSym(), Asm.getContext()), nullptr,
661-
Target.getConstant());
662-
MCValue B = MCValue::get(
663-
MCSymbolRefExpr::create(Target.getSubSym(), Asm.getContext()));
659+
MCValue A = MCValue::get(Target.getAddSym(), nullptr, Target.getConstant());
660+
MCValue B = MCValue::get(Target.getSubSym());
664661
auto FA = MCFixup::create(
665662
Fixup.getOffset(), nullptr,
666663
static_cast<MCFixupKind>(FirstLiteralRelocationKind + TA));

‎llvm/test/MC/AArch64/elf-reloc-ptrauth.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ _g9:
175175
// ERROBJ: :[[#@LINE+1]]:7: error: Cannot represent a difference across sections
176176
.quad _g9@AUTH(ia,42) - _g8
177177

178-
// ERROBJ: :[[#@LINE+1]]:7: error: Cannot represent a difference across sections
178+
// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
179179
.quad _g9@AUTH(ia,42) - _g8@AUTH(ia,42)
180180
.quad 0
181181

0 commit comments

Comments
 (0)