MCValue: Store SymA specifier at Specifier
The relocation specifier should be accessed via MCValue::Specifier.
However, some targets encode the relocation specifier within SymA using
MCSymbolRefExpr::SubclassData and access it via getAccessVariant(), though
this method is now deprecated.
This change stores the SymA specifier at Specifier as well, unifying the
two code paths.
* CSKY: GOT- and PLT- relocations now suppress the STT_SECTION
conversion.
* AArch64: https://reviews.llvm.org/D156505 added `getRefkind` check to
prevent folding. This is a hack and is now removed.
MCValue: Unify relocation specifier storage by storing SymA specifier at Specifier
The relocation specifier is accessed via MCValue::Specifier, but some
targets encoded it within SymA using MCSymbolRefExpr::SubclassData and
retrieved it through the now-deprecated getAccessVariant() method. This
commit unifies the two approaches by storing the SymA specifier at
`Specifier` as well.
Additional changes:
- CSKY: GOT- and PLT- relocations now suppress STT_SECTION conversion.
- AArch64: Removed the `getRefkind` check hack (introduced in https://reviews.llvm.org/D156505) that prevented folding.
Removed the assertion from `getRelocType`.
- RISCV: Removed the assertion from `getRelocType`.
Future plans:
- Replace MCSymbolRefExpr members with MCSymbol within MCValue.
- Remove `getSymSpecifier` (added for migration).
diff --git a/llvm/include/llvm/MC/MCExpr.h b/llvm/include/llvm/MC/MCExpr.h
index b0e347d..0d7a961 100644
--- a/llvm/include/llvm/MC/MCExpr.h
+++ b/llvm/include/llvm/MC/MCExpr.h
@@ -135,7 +135,8 @@
static bool evaluateSymbolicAdd(const MCAssembler *, const SectionAddrMap *,
bool, const MCValue &,
const MCSymbolRefExpr *,
- const MCSymbolRefExpr *, int64_t, MCValue &);
+ const MCSymbolRefExpr *, int64_t, uint32_t,
+ MCValue &);
};
inline raw_ostream &operator<<(raw_ostream &OS, const MCExpr &E) {
diff --git a/llvm/include/llvm/MC/MCValue.h b/llvm/include/llvm/MC/MCValue.h
index 9ef7770..fbe32ae 100644
--- a/llvm/include/llvm/MC/MCValue.h
+++ b/llvm/include/llvm/MC/MCValue.h
@@ -36,6 +36,10 @@
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; }
@@ -67,18 +71,23 @@
// Get the relocation specifier from SymA. This is a workaround for targets
// that do not use MCValue::Specifier.
- uint16_t getSymSpecifier() const { return SymA->getSpecifier(); }
- // Get the relocation specifier from SymA, or 0 when SymA is null.
- uint16_t getAccessVariant() const;
+ uint16_t getSymSpecifier() const { return Specifier; }
+ uint16_t getAccessVariant() const { return Specifier; }
static MCValue get(const MCSymbolRefExpr *SymA,
- const MCSymbolRefExpr *SymB = nullptr,
- int64_t Val = 0, uint32_t RefKind = 0) {
+ const MCSymbolRefExpr *SymB = nullptr, int64_t Val = 0,
+ uint32_t Specifier = 0) {
MCValue R;
R.Cst = Val;
R.SymA = SymA;
R.SymB = SymB;
- R.Specifier = RefKind;
+ 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 5e30812..5739097 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -168,7 +168,7 @@
IsResolved = false;
} else {
auto &SA = *Target.getAddSym();
- if (Target.getSymSpecifier() || SA.isUndefined()) {
+ if (Target.SymSpecifier || SA.isUndefined()) {
IsResolved = false;
} else {
IsResolved = (FixupFlags & MCFixupKindInfo::FKF_Constant) ||
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index b921b55..e3d4c5c 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -439,7 +439,7 @@
const MCValue &LHS,
const MCSymbolRefExpr *RhsAdd,
const MCSymbolRefExpr *RhsSub, int64_t RHS_Cst,
- MCValue &Res) {
+ uint32_t RhsSpec, MCValue &Res) {
const MCSymbol *LHS_A = LHS.getAddSym();
const MCSymbol *LHS_B = LHS.getSubSym();
int64_t LHS_Cst = LHS.getConstant();
@@ -451,16 +451,16 @@
int64_t Result_Cst = LHS_Cst + RHS_Cst;
// If we have a layout, we can fold resolved differences.
- if (Asm) {
+ if (Asm && !LHS.getSpecifier() && !RhsSpec) {
// 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).
// might bring more opportunities.
- if (LHS_A && RHS_B && !LHS.getSymA()->getSpecifier()) {
+ if (LHS_A && RHS_B) {
attemptToFoldSymbolOffsetDifference(Asm, Addrs, InSet, LHS_A, RHS_B,
Result_Cst);
}
- if (RHS_A && LHS_B && !RhsAdd->getSpecifier()) {
+ if (RHS_A && LHS_B) {
attemptToFoldSymbolOffsetDifference(Asm, Addrs, InSet, RHS_A, LHS_B,
Result_Cst);
}
@@ -476,7 +476,12 @@
auto *B = LHS_B ? LHS.getSymB() : RHS_B ? RhsSub : nullptr;
if (B && B->getKind() != MCSymbolRefExpr::VK_None)
return false;
+ auto Spec = LHS.getSpecifier();
+ if (!Spec)
+ Spec = RhsSpec;
Res = MCValue::get(A, B, Result_Cst);
+ Res.Specifier = Spec;
+ Res.SymSpecifier = 0;
return true;
}
@@ -634,9 +639,6 @@
return false;
case MCBinaryExpr::Add:
case MCBinaryExpr::Sub:
- // TODO: Prevent folding for AArch64 @AUTH operands.
- if (LHSValue.getSpecifier() || RHSValue.getSpecifier())
- return false;
if (Op == MCBinaryExpr::Sub) {
std::swap(RHSValue.SymA, RHSValue.SymB);
RHSValue.Cst = -(uint64_t)RHSValue.Cst;
@@ -652,7 +654,8 @@
return true;
}
return evaluateSymbolicAdd(Asm, Addrs, InSet, LHSValue, RHSValue.SymA,
- RHSValue.SymB, RHSValue.Cst, Res);
+ RHSValue.SymB, RHSValue.Cst,
+ RHSValue.SymSpecifier, Res);
}
}
diff --git a/llvm/lib/MC/MCValue.cpp b/llvm/lib/MC/MCValue.cpp
index 913fe83..77deb0b 100644
--- a/llvm/lib/MC/MCValue.cpp
+++ b/llvm/lib/MC/MCValue.cpp
@@ -43,9 +43,3 @@
print(dbgs());
}
#endif
-
-uint16_t MCValue::getAccessVariant() const {
- if (!SymA)
- return 0;
- return SymA->getSpecifier();
-}
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
index c5accb5..3335d9d 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
@@ -426,8 +426,13 @@
AArch64MCExpr::Specifier SymLoc = AArch64MCExpr::getSymbolLoc(RefKind);
if (SymLoc == AArch64AuthMCExpr::VK_AUTH ||
SymLoc == AArch64AuthMCExpr::VK_AUTHADDR) {
+ const auto *Expr = dyn_cast<AArch64AuthMCExpr>(Fixup.getValue());
+ if (!Expr) {
+ Asm.getContext().reportError(Fixup.getValue()->getLoc(),
+ "expected relocatable expression");
+ return;
+ }
assert(Value == 0);
- const auto *Expr = cast<AArch64AuthMCExpr>(Fixup.getValue());
Value = (uint64_t(Expr->getDiscriminator()) << 32) |
(uint64_t(Expr->getKey()) << 60) |
(uint64_t(Expr->hasAddressDiversity()) << 63);
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
index b03c55c..c3c4c64 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
@@ -116,12 +116,6 @@
AArch64MCExpr::Specifier SymLoc = AArch64MCExpr::getSymbolLoc(RefKind);
bool IsNC = AArch64MCExpr::isNotChecked(RefKind);
- assert((!Target.getAddSym() ||
- Target.getSymSpecifier() == AArch64MCExpr::None ||
- Target.getSymSpecifier() == AArch64MCExpr::VK_PLT ||
- Target.getSymSpecifier() == AArch64MCExpr::VK_GOTPCREL) &&
- "Should only be expression-level modifiers here");
-
switch (SymLoc) {
case AArch64MCExpr::VK_DTPREL:
case AArch64MCExpr::VK_GOTTPREL:
diff --git a/llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp b/llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
index 845c590..e33961a 100644
--- a/llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
+++ b/llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
@@ -196,9 +196,9 @@
Type = ELF::R_PPC_ADDR14; // XXX: or BRNTAKEN?_
break;
case PPC::fixup_ppc_half16:
- switch (RefKind) {
+ switch (Modifier) {
default:
- break;
+ llvm_unreachable("Unsupported specifier");
case PPCMCExpr::VK_LO:
return ELF::R_PPC_ADDR16_LO;
case PPCMCExpr::VK_HI:
@@ -217,9 +217,7 @@
return ELF::R_PPC64_ADDR16_HIGHEST;
case PPCMCExpr::VK_HIGHESTA:
return ELF::R_PPC64_ADDR16_HIGHESTA;
- }
- switch (Modifier) {
- default: llvm_unreachable("Unsupported Modifier");
+
case PPCMCExpr::VK_None:
Type = ELF::R_PPC_ADDR16;
break;
@@ -373,17 +371,12 @@
break;
case PPC::fixup_ppc_half16ds:
case PPC::fixup_ppc_half16dq:
- switch (RefKind) {
+ switch (Modifier) {
default:
Ctx.reportError(Fixup.getLoc(), "invalid VariantKind");
return ELF::R_PPC64_NONE;
- case PPCMCExpr::VK_None:
- break;
case PPCMCExpr::VK_LO:
return ELF::R_PPC64_ADDR16_LO_DS;
- }
- switch (Modifier) {
- default: llvm_unreachable("Unsupported Modifier");
case PPCMCExpr::VK_None:
Type = ELF::R_PPC64_ADDR16_DS;
break;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
index 586440f..1662b80 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
@@ -50,9 +50,6 @@
const MCValue &Target,
const MCFixup &Fixup,
bool IsPCRel) const {
- assert((!Target.getAddSym() ||
- Target.getSymSpecifier() == MCSymbolRefExpr::VK_None) &&
- "sym@specifier should have been rejected");
const MCExpr *Expr = Fixup.getValue();
// Determine the type of the relocation
unsigned Kind = Fixup.getTargetKind();
diff --git a/llvm/test/MC/AArch64/elf-reloc-ptrauth.s b/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
index 9fe78a4..6f47e2d 100644
--- a/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
+++ b/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
@@ -165,15 +165,6 @@
// RUN: FileCheck %s --check-prefix=ERROBJ
// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
-.quad sym@AUTH(ia,42) + 1
-
-// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
-.quad 1 + sym@AUTH(ia,42)
-
-// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
-.quad 1 + sym@AUTH(ia,42) + 1
-
-// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
.quad sym@AUTH(ia,42) + sym@AUTH(ia,42)
// TODO: do we really want to emit an error here? It might not be important
@@ -181,11 +172,17 @@
// distance remains the same. Leave it in such state as for now since it
// makes code simpler: subtraction of a non-AUTH symbol and of a constant
// are handled identically.
-// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
+// ERROBJ: :[[#@LINE+1]]:7: error: Cannot represent a difference across sections
.quad _g9@AUTH(ia,42) - _g8
-// ERROBJ: :[[#@LINE+1]]:7: error: expected relocatable expression
+// ERROBJ: :[[#@LINE+1]]:7: error: Cannot represent a difference across sections
.quad _g9@AUTH(ia,42) - _g8@AUTH(ia,42)
.quad 0
+// ERROBJ: :[[#@LINE+1]]:23: error: expected relocatable expression
+.quad sym@AUTH(ia,42) + 1
+
+// ERROBJ: :[[#@LINE+1]]:9: error: expected relocatable expression
+.quad 1 + sym@AUTH(ia,42)
+
.endif // ERROBJ
diff --git a/llvm/test/MC/CSKY/relocation-specifier.s b/llvm/test/MC/CSKY/relocation-specifier.s
index 759d6df..e305743 100644
--- a/llvm/test/MC/CSKY/relocation-specifier.s
+++ b/llvm/test/MC/CSKY/relocation-specifier.s
@@ -2,8 +2,8 @@
# RUN: llvm-readelf -rs %t | FileCheck %s --check-prefix=READELF
# READELF: '.rela.data'
-# READELF: R_CKCORE_GOT32 00000000 .data + 0
-# READELF: R_CKCORE_PLT32 00000000 .data + 0
+# READELF: R_CKCORE_GOT32 00000000 local + 0
+# READELF: R_CKCORE_PLT32 00000000 local + 0
# READELF: TLS GLOBAL DEFAULT UND gd
# READELF: TLS GLOBAL DEFAULT UND ld
