-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RISCV] Replace @plt/@gotpcrel in data directives with %pltpcrel %gotpcrel #132569
[RISCV] Replace @plt/@gotpcrel in data directives with %pltpcrel %gotpcrel #132569
Conversation
Created using spr 1.3.5-bogner [skip ci]
Created using spr 1.3.5-bogner
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-backend-risc-v Author: Fangrui Song (MaskRay) Changesclang -fexperimental-relative-c++-abi-vtables might generate @plt and The @plt syntax was selected was simply due to a quirk of AsmParser: RISC-V favors the This PR adds support for
Patch is 21.89 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132569.diff 20 Files Affected:
diff --git a/lld/test/ELF/riscv-reloc-plt32.s b/lld/test/ELF/riscv-reloc-plt32.s
index 8cbc2d3f442c0..a50b9fe3f137a 100644
--- a/lld/test/ELF/riscv-reloc-plt32.s
+++ b/lld/test/ELF/riscv-reloc-plt32.s
@@ -18,6 +18,6 @@
.globl _start
_start:
.data
- .word foo@PLT - .
- .word foo@PLT - . + 1
- .word foo@PLT - . - 1
+ .word %plt(foo - .)
+ .word %plt(foo - . + 1)
+ .word %plt(foo - . - 1)
diff --git a/lld/test/ELF/riscv-undefined-weak.s b/lld/test/ELF/riscv-undefined-weak.s
index 8a78e1f838338..d78f55394b619 100644
--- a/lld/test/ELF/riscv-undefined-weak.s
+++ b/lld/test/ELF/riscv-undefined-weak.s
@@ -97,4 +97,4 @@ branch:
# PC-NOT: .plt:
# PLT: .plt:
-.word target@plt - .
+.word %plt(target - .)
diff --git a/lld/test/ELF/riscv64-reloc-got32-pcrel.s b/lld/test/ELF/riscv64-reloc-got32-pcrel.s
index 24bd828235b25..a8f42ae6df2b9 100644
--- a/lld/test/ELF/riscv64-reloc-got32-pcrel.s
+++ b/lld/test/ELF/riscv64-reloc-got32-pcrel.s
@@ -12,16 +12,16 @@ bar:
.globl _start
_start: // PC = 0x33a8
-// bar@GOTPCREL = 0x2398 (got entry for `bar`) - 0x33a8 (.) = 0xf0efffff
-// bar@GOTPCREL+4 = 0x2398 (got entry for `bar`) - 0x33ac (.) + 4 = 0xf0efffff
-// bar@GOTPCREL-4 = 0x2398 (got entry for `bar`) - 0x33b0 (.) - 4 = 0xe4efffff
+// %gotpcrel(bar) = 0x2398 (got entry for `bar`) - 0x33a8 (.) = 0xf0efffff
+// %gotpcrel(bar+4) = 0x2398 (got entry for `bar`) - 0x33ac (.) + 4 = 0xf0efffff
+// %gotpcrel(bar-4) = 0x2398 (got entry for `bar`) - 0x33b0 (.) - 4 = 0xe4efffff
// CHECK: Contents of section .data:
// CHECK-NEXT: {{.*}} f0efffff f0efffff e4efffff
- .word bar@GOTPCREL
- .word bar@GOTPCREL+4
- .word bar@GOTPCREL-4
+ .word %gotpcrel(bar)
+ .word %gotpcrel(bar+4)
+ .word %gotpcrel(bar-4)
// WARN: relocation R_RISCV_GOT32_PCREL out of range: {{.*}} is not in [-2147483648, 2147483647]; references 'baz'
// WARN: relocation R_RISCV_GOT32_PCREL out of range: {{.*}} is not in [-2147483648, 2147483647]; references 'baz'
- .word baz@GOTPCREL+0xffffffff
- .word baz@GOTPCREL-0xffffffff
+ .word %gotpcrel(baz+0xffffffff)
+ .word %gotpcrel(baz-0xffffffff)
diff --git a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
index 7392963bd341f..f6e875e7ad373 100644
--- a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
+++ b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
@@ -38,6 +38,7 @@ class TargetLoweringObjectFileELF : public TargetLoweringObjectFile {
protected:
uint8_t PLTRelativeSpecifier = 0;
+ bool UseTargetMCExprForPLTRelative = true;
public:
TargetLoweringObjectFileELF();
@@ -112,11 +113,16 @@ class TargetLoweringObjectFileELF : public TargetLoweringObjectFile {
MCSection *getStaticDtorSection(unsigned Priority,
const MCSymbol *KeySym) const override;
+ virtual const MCExpr *createTargetMCExpr(const MCExpr *Expr,
+ uint8_t Specifier) const {
+ return nullptr;
+ }
const MCExpr *lowerRelativeReference(const GlobalValue *LHS,
const GlobalValue *RHS,
const TargetMachine &TM) const override;
const MCExpr *lowerDSOLocalEquivalent(const DSOLocalEquivalent *Equiv,
+ const MCSymbolRefExpr *RHS,
const TargetMachine &TM) const override;
MCSection *getSectionForCommandLines() const override;
diff --git a/llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h b/llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
index 59cee18bfdf42..c7f098be70945 100644
--- a/llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
+++ b/llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
@@ -396,6 +396,12 @@ class MCTargetAsmParser : public MCAsmParserExtension {
virtual bool parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
return getParser().parsePrimaryExpr(Res, EndLoc, nullptr);
}
+ // Parse an expression in a data directive, possibly with a relocation
+ // specifier.
+ virtual bool parseDataExpr(const MCExpr *&Res) {
+ SMLoc EndLoc;
+ return getParser().parseExpression(Res, EndLoc);
+ }
virtual bool parseRegister(MCRegister &Reg, SMLoc &StartLoc,
SMLoc &EndLoc) = 0;
diff --git a/llvm/include/llvm/Target/TargetLoweringObjectFile.h b/llvm/include/llvm/Target/TargetLoweringObjectFile.h
index a5ed1b29dc1bc..77148320ccfd5 100644
--- a/llvm/include/llvm/Target/TargetLoweringObjectFile.h
+++ b/llvm/include/llvm/Target/TargetLoweringObjectFile.h
@@ -209,6 +209,7 @@ class TargetLoweringObjectFile : public MCObjectFileInfo {
}
virtual const MCExpr *lowerDSOLocalEquivalent(const DSOLocalEquivalent *Equiv,
+ const MCSymbolRefExpr *RHS,
const TargetMachine &TM) const {
return nullptr;
}
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 743551822a8fc..c5e8aa39a9115 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3382,7 +3382,7 @@ const MCExpr *AsmPrinter::lowerConstant(const Constant *CV) {
return lowerBlockAddressConstant(*BA);
if (const auto *Equiv = dyn_cast<DSOLocalEquivalent>(CV))
- return getObjFileLowering().lowerDSOLocalEquivalent(Equiv, TM);
+ return getObjFileLowering().lowerDSOLocalEquivalent(Equiv, nullptr, TM);
if (const NoCFIValue *NC = dyn_cast<NoCFIValue>(CV))
return MCSymbolRefExpr::create(getSymbol(NC->getGlobalValue()), Ctx);
@@ -3481,12 +3481,13 @@ const MCExpr *AsmPrinter::lowerConstant(const Constant *CV) {
if (!RelocExpr) {
const MCExpr *LHSExpr =
MCSymbolRefExpr::create(getSymbol(LHSGV), Ctx);
+ auto *RHSExpr = MCSymbolRefExpr::create(getSymbol(RHSGV), Ctx);
if (DSOEquiv &&
getObjFileLowering().supportDSOLocalEquivalentLowering())
- LHSExpr =
- getObjFileLowering().lowerDSOLocalEquivalent(DSOEquiv, TM);
- RelocExpr = MCBinaryExpr::createSub(
- LHSExpr, MCSymbolRefExpr::create(getSymbol(RHSGV), Ctx), Ctx);
+ RelocExpr = getObjFileLowering().lowerDSOLocalEquivalent(
+ DSOEquiv, RHSExpr, TM);
+ else
+ RelocExpr = MCBinaryExpr::createSub(LHSExpr, RHSExpr, Ctx);
}
int64_t Addend = (LHSOffset - RHSOffset).getSExtValue();
if (Addend != 0)
diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 27a70e6f775b3..ee0db578d55b8 100644
--- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1194,18 +1194,34 @@ const MCExpr *TargetLoweringObjectFileELF::lowerRelativeReference(
MCSymbolRefExpr::create(TM.getSymbol(RHS), getContext()), getContext());
}
+// Reference the function or its PLT entry (`Equiv`), optionally with a
+// subtrahend (`RHS`).
const MCExpr *TargetLoweringObjectFileELF::lowerDSOLocalEquivalent(
- const DSOLocalEquivalent *Equiv, const TargetMachine &TM) const {
+ const DSOLocalEquivalent *Equiv, const MCSymbolRefExpr *RHS,
+ const TargetMachine &TM) const {
assert(supportDSOLocalEquivalentLowering());
+ // If GV is dso_local, reference the function itself. Return GV or GV-RHS.
const auto *GV = Equiv->getGlobalValue();
-
- // A PLT entry is not needed for dso_local globals.
+ const MCExpr *Res = MCSymbolRefExpr::create(TM.getSymbol(GV), getContext());
+ if (RHS)
+ Res = MCBinaryExpr::createSub(Res, RHS, getContext());
if (GV->isDSOLocal() || GV->isImplicitDSOLocal())
- return MCSymbolRefExpr::create(TM.getSymbol(GV), getContext());
-
- return MCSymbolRefExpr::create(TM.getSymbol(GV), PLTRelativeSpecifier,
- getContext());
+ return Res;
+
+ // Otherwise, reference the PLT. Return a relocatable expression with the PLT
+ // specifier, %plt(GV) or %plt(GV-RHS).
+ Res = createTargetMCExpr(Res, PLTRelativeSpecifier);
+ if (Res)
+ return Res;
+
+ // If the target only supports the legacy syntax @plt, return GV@plt or
+ // GV@plt - RHS.
+ Res = MCSymbolRefExpr::create(TM.getSymbol(GV), PLTRelativeSpecifier,
+ getContext());
+ if (RHS)
+ Res = MCBinaryExpr::createSub(Res, RHS, getContext());
+ return Res;
}
MCSection *TargetLoweringObjectFileELF::getSectionForCommandLines() const {
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index fb85accf267b4..06f94d5e89198 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -237,7 +237,7 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF,
// A linker relaxation target may emit ADD/SUB relocations for A-B+C. Let
// recordRelocation handle non-VK_None cases like A@plt-B+C.
if (!IsResolved && Target.getSymA() && Target.getSymB() &&
- Target.getSymA()->getKind() == MCSymbolRefExpr::VK_None &&
+ Target.getRefKind() == 0 &&
getBackend().handleAddSubRelocations(*this, *DF, Fixup, Target, Value))
return true;
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index c11c01e52fc2e..87d9a7b8727df 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -3144,7 +3144,7 @@ bool AsmParser::parseDirectiveValue(StringRef IDVal, unsigned Size) {
auto parseOp = [&]() -> bool {
const MCExpr *Value;
SMLoc ExprLoc = getLexer().getLoc();
- if (checkForValidSection() || parseExpression(Value))
+ if (checkForValidSection() || getTargetParser().parseDataExpr(Value))
return true;
// Special case constant expressions to match code generator.
if (const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(Value)) {
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 05997cf78c6b1..5f673574c200d 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -225,6 +225,8 @@ class RISCVAsmParser : public MCTargetAsmParser {
}
bool parseOperand(OperandVector &Operands, StringRef Mnemonic);
+ bool parseExprWithSpecifier(const MCExpr *&Res, SMLoc &E);
+ bool parseDataExpr(const MCExpr *&Res) override;
bool parseDirectiveOption();
bool parseDirectiveAttribute();
@@ -2233,8 +2235,17 @@ ParseStatus RISCVAsmParser::parseOperandWithSpecifier(OperandVector &Operands) {
SMLoc S = getLoc();
SMLoc E;
- if (!parseOptionalToken(AsmToken::Percent) ||
- getLexer().getKind() != AsmToken::Identifier)
+ if (!parseOptionalToken(AsmToken::Percent))
+ return Error(getLoc(), "expected '%' relocation specifier");
+ const MCExpr *Expr = nullptr;
+ bool Failed = parseExprWithSpecifier(Expr, E);
+ if (!Failed)
+ Operands.push_back(RISCVOperand::createImm(Expr, S, E, isRV64()));
+ return Failed;
+}
+
+bool RISCVAsmParser::parseExprWithSpecifier(const MCExpr *&Res, SMLoc &E) {
+ if (getLexer().getKind() != AsmToken::Identifier)
return Error(getLoc(), "expected '%' relocation specifier");
StringRef Identifier = getParser().getTok().getIdentifier();
auto Spec = RISCVMCExpr::getSpecifierForName(Identifier);
@@ -2243,15 +2254,21 @@ ParseStatus RISCVAsmParser::parseOperandWithSpecifier(OperandVector &Operands) {
getParser().Lex(); // Eat the identifier
if (parseToken(AsmToken::LParen, "expected '('"))
- return ParseStatus::Failure;
+ return true;
const MCExpr *SubExpr;
if (getParser().parseParenExpression(SubExpr, E))
- return ParseStatus::Failure;
+ return true;
- const MCExpr *ModExpr = RISCVMCExpr::create(SubExpr, *Spec, getContext());
- Operands.push_back(RISCVOperand::createImm(ModExpr, S, E, isRV64()));
- return ParseStatus::Success;
+ Res = RISCVMCExpr::create(SubExpr, *Spec, getContext());
+ return false;
+}
+
+bool RISCVAsmParser::parseDataExpr(const MCExpr *&Res) {
+ SMLoc E;
+ if (parseOptionalToken(AsmToken::Percent))
+ return parseExprWithSpecifier(Res, E);
+ return getParser().parseExpression(Res);
}
ParseStatus RISCVAsmParser::parseBareSymbol(OperandVector &Operands) {
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
index f8841dd657568..afd5d3e4893f1 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
@@ -50,6 +50,9 @@ unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx,
const MCValue &Target,
const MCFixup &Fixup,
bool IsPCRel) const {
+ assert((!Target.getSymA() ||
+ Target.getSymA()->getKind() == MCSymbolRefExpr::VK_None) &&
+ "sym@specifier should have been rejected");
const MCExpr *Expr = Fixup.getValue();
// Determine the type of the relocation
unsigned Kind = Fixup.getTargetKind();
@@ -73,9 +76,8 @@ unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx,
return ELF::R_RISCV_NONE;
case FK_Data_4:
case FK_PCRel_4:
- return uint8_t(Target.getAccessVariant()) == RISCVMCExpr::VK_PLT
- ? ELF::R_RISCV_PLT32
- : ELF::R_RISCV_32_PCREL;
+ return Target.getRefKind() == RISCVMCExpr::VK_PLT ? ELF::R_RISCV_PLT32
+ : ELF::R_RISCV_32_PCREL;
case RISCV::fixup_riscv_pcrel_hi20:
return ELF::R_RISCV_PCREL_HI20;
case RISCV::fixup_riscv_pcrel_lo12_i:
@@ -129,11 +131,12 @@ unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx,
Ctx.reportError(Fixup.getLoc(), "2-byte data relocations not supported");
return ELF::R_RISCV_NONE;
case FK_Data_4:
- if (Expr->getKind() == MCExpr::Target &&
- cast<RISCVMCExpr>(Expr)->getSpecifier() == RISCVMCExpr::VK_32_PCREL)
- return ELF::R_RISCV_32_PCREL;
- if (getSpecifier(Target.getSymA()) == RISCVMCExpr::VK_GOTPCREL)
- return ELF::R_RISCV_GOT32_PCREL;
+ if (Expr->getKind() == MCExpr::Target) {
+ if (cast<RISCVMCExpr>(Expr)->getSpecifier() == RISCVMCExpr::VK_32_PCREL)
+ return ELF::R_RISCV_32_PCREL;
+ if (cast<RISCVMCExpr>(Expr)->getSpecifier() == RISCVMCExpr::VK_GOTPCREL)
+ return ELF::R_RISCV_GOT32_PCREL;
+ }
return ELF::R_RISCV_32;
case FK_Data_8:
return ELF::R_RISCV_64;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
index c327f9d5f0f1e..7e9b312d3c25e 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
@@ -18,11 +18,6 @@
#include "llvm/TargetParser/Triple.h"
using namespace llvm;
-const MCAsmInfo::VariantKindDesc variantKindDescs[] = {
- {RISCVMCExpr::VK_GOTPCREL, "GOTPCREL"},
- {RISCVMCExpr::VK_PLT, "PLT"},
-};
-
void RISCVMCAsmInfo::anchor() {}
RISCVMCAsmInfo::RISCVMCAsmInfo(const Triple &TT) {
@@ -33,8 +28,6 @@ RISCVMCAsmInfo::RISCVMCAsmInfo(const Triple &TT) {
ExceptionsType = ExceptionHandling::DwarfCFI;
Data16bitsDirective = "\t.half\t";
Data32bitsDirective = "\t.word\t";
-
- initializeVariantKinds(variantKindDescs);
}
const MCExpr *RISCVMCAsmInfo::getExprForFDESymbol(const MCSymbol *Sym,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
index 7de53e6d3f479..ce365d0036ce3 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
@@ -94,8 +94,10 @@ bool RISCVMCExpr::evaluateAsRelocatableImpl(MCValue &Res,
Res = MCValue::get(Res.getSymA(), Res.getSymB(), Res.getConstant(),
getSpecifier());
- // Custom fixup types are not valid with symbol difference expressions.
- return Res.getSymB() ? getSpecifier() == VK_None : true;
+ // When the subtrahend (SymB) is present, the relocatable expression
+ // only allows None and PLT as the specifier.
+ return !Res.getSymB() ||
+ llvm::is_contained({VK_None, VK_PLT}, getSpecifier());
}
void RISCVMCExpr::visitUsedExpr(MCStreamer &Streamer) const {
@@ -119,14 +121,15 @@ RISCVMCExpr::getSpecifierForName(StringRef name) {
.Case("tlsdesc_load_lo", VK_TLSDESC_LOAD_LO)
.Case("tlsdesc_add_lo", VK_TLSDESC_ADD_LO)
.Case("tlsdesc_call", VK_TLSDESC_CALL)
+ // Used in data directives
+ .Case("plt", VK_PLT)
+ .Case("gotpcrel", VK_GOTPCREL)
.Default(std::nullopt);
}
StringRef RISCVMCExpr::getSpecifierName(Specifier S) {
switch (S) {
case VK_None:
- case VK_PLT:
- case VK_GOTPCREL:
llvm_unreachable("not used as %specifier()");
case VK_LO:
return "lo";
@@ -162,6 +165,10 @@ StringRef RISCVMCExpr::getSpecifierName(Specifier S) {
return "call_plt";
case VK_32_PCREL:
return "32_pcrel";
+ case VK_PLT:
+ return "plt";
+ case VK_GOTPCREL:
+ return "gotpcrel";
}
llvm_unreachable("Invalid ELF symbol kind");
}
diff --git a/llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp b/llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp
index e546815b70935..7e98f1124d2e5 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp
@@ -28,6 +28,7 @@ void RISCVELFTargetObjectFile::Initialize(MCContext &Ctx,
TargetLoweringObjectFileELF::Initialize(Ctx, TM);
PLTRelativeSpecifier = RISCVMCExpr::VK_PLT;
+ UseTargetMCExprForPLTRelative = true;
SupportIndirectSymViaGOTPCRel = true;
SmallDataSection = getContext().getELFSection(
@@ -180,3 +181,10 @@ MCSection *RISCVELFTargetObjectFile::getSectionForConstant(
return TargetLoweringObjectFileELF::getSectionForConstant(DL, Kind, C,
Alignment);
}
+
+const MCExpr *
+RISCVELFTargetObjectFile::createTargetMCExpr(const MCExpr *Expr,
+ uint8_t Specifier) const {
+ return RISCVMCExpr::create(Expr, RISCVMCExpr::Specifier(Specifier),
+ getContext());
+}
diff --git a/llvm/lib/Target/RISCV/RISCVTargetObjectFile.h b/llvm/lib/Target/RISCV/RISCVTargetObjectFile.h
index ff7e3e4c752c3..b6da3f4721f4b 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetObjectFile.h
+++ b/llvm/lib/Target/RISCV/RISCVTargetObjectFile.h
@@ -48,6 +48,9 @@ class RISCVELFTargetObjectFile : public TargetLoweringObjectFileELF {
bool isInSmallSection(uint64_t Size) const;
+ const MCExpr *createTargetMCExpr(const MCExpr *Expr,
+ uint8_t Specifier) const override;
+
const MCExpr *getIndirectSymViaGOTPCRel(const GlobalValue *GV,
const MCSymbol *Sym,
const MCValue &MV, int64_t Offset,
diff --git a/llvm/test/CodeGen/RISCV/dso_local_equivalent.ll b/llvm/test/CodeGen/RISCV/dso_local_equivalent.ll
index 0979967614b8a..9823e8b33fd9d 100644
--- a/llvm/test/CodeGen/RISCV/dso_local_equivalent.ll
+++ b/llvm/test/CodeGen/RISCV/dso_local_equivalent.ll
@@ -4,7 +4,7 @@
declare void @extern_func()
; CHECK-LABEL: const:
-; CHECK-NEXT: .word extern_func@PLT-const
+; CHECK-NEXT: .word %plt(extern_func-const)
;; Note that for riscv32, the ptrtoint will actually upcast the ptr it to an
;; oversized 64-bit pointer that eventually gets truncated. This isn't needed
diff --git a/llvm/test/MC/RISCV/data-directive-specifier.s b/llvm/test/MC/RISCV/data-directive-specifier.s
index a578f9720eccd..9d0f66ee384d9 100644
--- a/llvm/test/MC/RISCV/data-directive-specifier.s
+++ b/llvm/test/MC/RISCV/data-directive-specifier.s
@@ -15,12 +15,12 @@ l:
# CHECK-NEXT: 0x10 R_RISCV_PLT32 g 0x18
# CHECK-NEXT: }
.data
-.word l@plt - .
-.word l@plt - .data
+.word %plt(l - .)
+.word %plt(l - .data)
-.word extern@plt - . + 4
-.word g@plt - . + 8
-.word g@plt - .data + 8
+.word %plt(extern - . + 4)
+.word %plt(g - . + 8)
+.word %plt(g - .data + 8)
# CHECK: Section ({{.*}}) .rela.data1 {
# CHECK-NEXT: 0x0 R_RISCV_GOT32_PCREL data1 0x0
@@ -30,14 +30,14 @@ l:
# CHE...
[truncated]
|
@@ -1194,18 +1194,34 @@ const MCExpr *TargetLoweringObjectFileELF::lowerRelativeReference( | |||
MCSymbolRefExpr::create(TM.getSymbol(RHS), getContext()), getContext()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lowerRelativeReference
uses MCSymbolRefExpr but I do not intend to touch it. It seems legacy (probably should be removed).
clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %plt(foo-.)` and `.word %gotpcreel(foo)` and drops `@plt` `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm#132569
lld/test/ELF/riscv-reloc-plt32.s
Outdated
.word foo@PLT - . | ||
.word foo@PLT - . + 1 | ||
.word foo@PLT - . - 1 | ||
.word %plt(foo - .) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would %plt(foo) - .
not be the saner syntax? PLT of an offset is a bit nonsensical...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The core principle is that relocation specifiers must operate on the full expression, not just a part of it. (semantically closer to gas https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers#gnu-assembler-internals)
This would avoid a lot of complexity/ambiguity in parsing and expression folding.
.word %plt(foo - .)
might look unusual. It describes a PC-relative relocatable expression.
We evaluate it to a relocatable expression (relocation_specifier(sym_a - sym_b + offset)
), and allows the referenced "addend" (sym_a) to be a PLT.
.word %plt(foo)
describes an absolute reference to foo's PLT, which we don't support.
Branch instructions have an implicit PC-relative operand, so call foo@plt
(legacy syntax) could be viewed as call %plt(foo)
(conceptually, we shouldn't add the syntax), which is internally
converted to PC-relative as %plt(foo - .)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I know, but it's pretty weird and confusing syntax. It's not really written that way because it makes sense, it's just written that way because it aligns with how implementations think about it.
Perhaps %pltpcrel, to mirror %gotpcrel, would be the right thing to do here that sidesteps the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's challenging to use an inherent PC-relative specifier (e.g. %pltpcrel
; which I actually thought about).... This is because
in the code block around llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3477
(AsmPrinter::lowerConstant
), the function doesn't actually know its current address. The function call needs a subtrahend. (llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll
contains a better test that demonstrates this: the subtrahend vtable
might be a few bytes before the current address (that feature might be obsoleted by the dsolocal equivalent feature))
(If we adopt %pltpcrel
and if we ever decide to support .word %plt(foo)
, we would introduce another specifier. While with %plt(foo - .)
, we won't need a new specifier.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have %(got_)pcrel_hi and now %gotpcrel, what's so different about this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I don't like %plt(foo - .)
, because the thing being put into the instruction is really the difference between foo's PLT entry's address, and the current address (rather than a PLT entry for the difference between foo and the current address). I would much prefer %pltpcrel(foo)
if %plt(foo)-.
is not a direction you are happy with, given the existing %gotpcrel(foo)
means the difference between the current address and foo's GOT entry's address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IR doesn't model the current location (DOT). Instead, It computes SymA-SymB+offset
.
SymB
might not be the current location, but it and the current location must be in the same section.
Let's use llvm/test/CodeGen/X86/x86-64-plt-relative-reloc.ll
(legacy; but slightly better than dso_local_equivalent.ll
):
@vtable = constant [5 x i32] [i32 0,
i32 trunc (i64 sub (i64 ptrtoint (ptr @fn1 to i64), i64 ptrtoint (ptr getelementptr ([5 x i32], ptr @vtable, i32 0, i32 1) to i64)) to i32),
i32 trunc (i64 sub (i64 ptrtoint (ptr @fn2 to i64), i64 ptrtoint (ptr getelementptr ([5 x i32], ptr @vtable, i32 0, i32 1) to i64)) to i32),
i32 trunc (i64 sub (i64 ptrtoint (ptr @fn3 to i64), i64 ptrtoint (ptr getelementptr ([5 x i32], ptr @vtable, i32 0, i32 1) to i64)) to i32),
i32 trunc (i64 sub (i64 ptrtoint (ptr @global4 to i64), i64 ptrtoint (ptr getelementptr ([5 x i32], ptr @vtable, i32 0, i32 1) to i64)) to i32)
]
If we need to emit %pltpcrel
, we will have to change AsmPrinter::lowerConstant
to know the base GlobalVariable, which might be possible...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well my overarching point would be that user-facing syntax should not be beholden to arbitrary historic implementation choices. If it's truly impossible to make it work then that's one thing, but I doubt that to be the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented %pltpcrel
, while a bit complex, I am happy with the result. (As in RISCVMCExpr::evaluateAsRelocatableImpl
, we don't need an exception for VK_PLT
, if we go with %plt
. This might be difficult to explain without playing with it.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not looked at the implementation in detail, but thank you for taking the time to do so, I know from experience it can be quite painful getting the MC code to do things that weren't previously expected of it
Created using spr 1.3.5-bogner [skip ci]
Created using spr 1.3.5-bogner
clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %plt(foo-.)` and `.word %gotpcreel(foo)` and drops `@plt` `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm#132569
…pcrel clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %pltpcrel(foo+offset)` and `.word %gotpcrel(foo)`, and drops `@plt` and `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm#132569
…pcrel clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %pltpcrel(foo+offset)` and `.word %gotpcrel(foo)`, and drops `@plt` and `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm#132569
…pcrel clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %pltpcrel(foo+offset)` and `.word %gotpcrel(foo)`, and drops `@plt` and `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm#132569
…pcrel clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %pltpcrel(foo+offset)` and `.word %gotpcrel(foo)`, and drops `@plt` and `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm#132569
…pcrel clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %pltpcrel(foo+offset)` and `.word %gotpcrel(foo)`, and drops `@plt` and `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm#132569
…pcrel clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %pltpcrel(foo+offset)` and `.word %gotpcrel(foo)`, and drops `@plt` and `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm#132569
Created using spr 1.3.5-bogner [skip ci]
Created using spr 1.3.5-bogner
@@ -2289,25 +2314,6 @@ bool TargetLoweringObjectFileWasm::shouldPutJumpTableInFunctionSection( | |||
return false; | |||
} | |||
|
|||
const MCExpr *TargetLoweringObjectFileWasm::lowerRelativeReference( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll pre-commit this. wasm and xcoff did cargo culting and added the unneeded functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy with this, but I don't know if Jessica has more feedback. Comments below are nits or for follow-ups
return Failed; | ||
} | ||
|
||
bool RISCVAsmParser::parseExprWithSpecifier(const MCExpr *&Res, SMLoc &E) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great to move these all to ParseStatus, but this PR isn't the right moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand it, ParseStatus
is preferred for parse*
functions generated by TableGen.
However, ParseStatus
is used very lightly in AsmParser.cpp, where parseDataExpr
is needed. So the bool return value here should be ok.
Created using spr 1.3.5-bogner [skip ci]
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner [skip ci]
Created using spr 1.3.5-bogner
…tpcrel %gotpcrel clang -fexperimental-relative-c++-abi-vtables might generate `@plt` and `@gotpcrel` specifiers in data directives. The syntax is not used in humand-written assembly code, and is not supported by GNU assembler. Note: the `@plt` in `.word foo@plt` is different from the legacy `call func@plt` (where `@plt` is simply ignored). The `@plt` syntax was selected was simply due to a quirk of AsmParser: the syntax was supported by all targets until I updated it to be an opt-in feature in a067175 RISC-V favors the `%specifier(expr)` syntax following MIPS and Sparc, and we should follow this convention. This PR adds support for `.word %pltpcrel(foo+offset)` and `.word %gotpcrel(foo)`, and drops `@plt` and `@gotpcrel`. * MCValue::SymA can no longer have a SymbolVariant. Add an assert similar to that of AArch64ELFObjectWriter.cpp before https://reviews.llvm.org/D81446 (see my analysis at https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers if intrigued) * `jump foo@plt, x31` now has a different diagnostic. Pull Request: llvm/llvm-project#132569
clang -fexperimental-relative-c++-abi-vtables might generate
@plt
and@gotpcrel
specifiers in data directives. The syntax is not used inhumand-written assembly code, and is not supported by GNU assembler.
Note: the
@plt
in.word foo@plt
is different fromthe legacy
call func@plt
(where@plt
is simply ignored).The
@plt
syntax was selected was simply due to a quirk of AsmParser:the syntax was supported by all targets until I updated it
to be an opt-in feature in a067175
RISC-V favors the
%specifier(expr)
syntax following MIPS and Sparc,and we should follow this convention.
This PR adds support for
.word %pltpcrel(foo+offset)
and.word %gotpcrel(foo)
, and drops@plt
and@gotpcrel
.similar to that of AArch64ELFObjectWriter.cpp before
https://reviews.llvm.org/D81446 (see my analysis at
https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers
if intrigued)
jump foo@plt, x31
now has a different diagnostic.