Skip to content

Commit 68472a3

Browse files
committed
[MC] Restore MCAsmBackend::shouldForceRelocation to false
For IsPCRel fixups, we had the `A->getKind() != MCSymbolRefExpr::VK_None` condition to force relocations. The condition has then been changed to `Target.getSpecifier()` (086af83). 38c3ad3 updated shouldForceRelocation to test `Target.getSpecifier`. It is not a good fit as many targets with %lo/%hi style specifiers (SPARC, MIPS, PowerPC, RISC-V) do not force relocations for these specifiers. Revert the Target.getSpecifier implementation (38c3ad3) and update targets that need it (SystemZAsmBackend/X86AsmBackend) instead. Targets need customization might define addReloc instead. Note: The X86AsmBackend implementation is too conservative. GNU Assembler doesn't emit a relocation for `call local@plt; local` a3d3931 added missing coverage for GOT/PLT related relocations.
1 parent 586e1df commit 68472a3

File tree

5 files changed

+25
-15
lines changed

5 files changed

+25
-15
lines changed

llvm/include/llvm/MC/MCAsmBackend.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,11 @@ class MCAsmBackend {
8888
/// Get information on a fixup kind.
8989
virtual MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const;
9090

91-
// Hook to check if a relocation is needed. The default implementation tests
92-
// whether the MCValue has a relocation specifier.
91+
// Hook used by the default `addReloc` to check if a relocation is needed.
9392
virtual bool shouldForceRelocation(const MCAssembler &, const MCFixup &,
94-
const MCValue &, const MCSubtargetInfo *);
93+
const MCValue &, const MCSubtargetInfo *) {
94+
return false;
95+
}
9596

9697
/// Hook to check if extra nop bytes must be inserted for alignment directive.
9798
/// For some targets this may be necessary in order to support linker

llvm/lib/MC/MCAsmBackend.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,6 @@ MCFixupKindInfo MCAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
109109
return Builtins[Kind - FK_NONE];
110110
}
111111

112-
bool MCAsmBackend::shouldForceRelocation(const MCAssembler &, const MCFixup &,
113-
const MCValue &Target,
114-
const MCSubtargetInfo *) {
115-
return Target.getSpecifier();
116-
}
117-
118112
bool MCAsmBackend::fixupNeedsRelaxationAdvanced(const MCAssembler &,
119113
const MCFixup &Fixup,
120114
const MCValue &, uint64_t Value,

llvm/lib/Target/Sparc/MCTargetDesc/SparcAsmBackend.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,6 @@ namespace {
200200
return Info;
201201
}
202202

203-
bool shouldForceRelocation(const MCAssembler &, const MCFixup &,
204-
const MCValue &,
205-
const MCSubtargetInfo *) override {
206-
return false;
207-
}
208-
209203
void relaxInstruction(MCInst &Inst,
210204
const MCSubtargetInfo &STI) const override {
211205
// FIXME.

llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCAsmBackend.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "llvm/MC/MCInst.h"
1818
#include "llvm/MC/MCObjectWriter.h"
1919
#include "llvm/MC/MCSubtargetInfo.h"
20+
#include "llvm/MC/MCValue.h"
2021

2122
using namespace llvm;
2223

@@ -112,6 +113,8 @@ class SystemZMCAsmBackend : public MCAsmBackend {
112113
// Override MCAsmBackend
113114
std::optional<MCFixupKind> getFixupKind(StringRef Name) const override;
114115
MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
116+
bool shouldForceRelocation(const MCAssembler &, const MCFixup &,
117+
const MCValue &, const MCSubtargetInfo *) override;
115118
void applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
116119
const MCValue &Target, MutableArrayRef<char> Data,
117120
uint64_t Value, bool IsResolved,
@@ -152,6 +155,13 @@ MCFixupKindInfo SystemZMCAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
152155
return SystemZ::MCFixupKindInfos[Kind - FirstTargetFixupKind];
153156
}
154157

158+
bool SystemZMCAsmBackend::shouldForceRelocation(const MCAssembler &,
159+
const MCFixup &,
160+
const MCValue &Target,
161+
const MCSubtargetInfo *) {
162+
return Target.getSpecifier();
163+
}
164+
155165
void SystemZMCAsmBackend::applyFixup(const MCAssembler &Asm,
156166
const MCFixup &Fixup,
157167
const MCValue &Target,

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@ class X86AsmBackend : public MCAsmBackend {
169169

170170
MCFixupKindInfo getFixupKindInfo(MCFixupKind Kind) const override;
171171

172+
bool shouldForceRelocation(const MCAssembler &, const MCFixup &,
173+
const MCValue &, const MCSubtargetInfo *) override;
174+
172175
void applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
173176
const MCValue &Target, MutableArrayRef<char> Data,
174177
uint64_t Value, bool IsResolved,
@@ -687,6 +690,14 @@ static unsigned getFixupKindSize(unsigned Kind) {
687690
}
688691
}
689692

693+
// Force relocation when there is a specifier. This might be too conservative -
694+
// GAS doesn't emit a relocation for call local@plt; local:.
695+
bool X86AsmBackend::shouldForceRelocation(const MCAssembler &, const MCFixup &,
696+
const MCValue &Target,
697+
const MCSubtargetInfo *) {
698+
return Target.getSpecifier();
699+
}
700+
690701
void X86AsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
691702
const MCValue &, MutableArrayRef<char> Data,
692703
uint64_t Value, bool IsResolved,

0 commit comments

Comments
 (0)