[llvm] 75f6fe2 - [MC] Remove unneeded MCSymbolRefExpr::create overload and add comments
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 5 22:10:13 PST 2025
Author: Fangrui Song
Date: 2025-03-05T22:10:08-08:00
New Revision: 75f6fe2ee5eb3e966cd4c335abd018921d6337d3
URL: https://github.com/llvm/llvm-project/commit/75f6fe2ee5eb3e966cd4c335abd018921d6337d3
DIFF: https://github.com/llvm/llvm-project/commit/75f6fe2ee5eb3e966cd4c335abd018921d6337d3.diff
LOG: [MC] Remove unneeded MCSymbolRefExpr::create overload and add comments
The StringRef overload is often error-prone as users might forget to
register the MCSymbol.
Add comments to MCTargetExpr and MCSymbolRefExpr::VariantKind.
In the distant future the VariantKind parameter might be removed.
Added:
Modified:
llvm/include/llvm/MC/MCExpr.h
llvm/lib/MC/MCExpr.cpp
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCExpr.h b/llvm/include/llvm/MC/MCExpr.h
index a7802ba9a7b4d..ab96b39d9f296 100644
--- a/llvm/include/llvm/MC/MCExpr.h
+++ b/llvm/include/llvm/MC/MCExpr.h
@@ -191,6 +191,11 @@ class MCConstantExpr : public MCExpr {
/// of the symbol as external.
class MCSymbolRefExpr : public MCExpr {
public:
+ // VariantKind isn't ideal for encoding relocation operators because:
+ // (a) other expressions, like MCConstantExpr (e.g., 4 at l) and MCBinaryExpr
+ // (e.g., (a+1)@l), also need it; (b) semantics become unclear (e.g., folding
+ // expressions with @). MCTargetExpr, as used by AArch64 and RISC-V, offers a
+ // cleaner approach.
enum VariantKind : uint16_t {
VK_None,
VK_Invalid,
@@ -384,14 +389,13 @@ class MCSymbolRefExpr : public MCExpr {
/// \name Construction
/// @{
- static const MCSymbolRefExpr *create(const MCSymbol *Symbol, MCContext &Ctx) {
- return MCSymbolRefExpr::create(Symbol, VK_None, Ctx);
+ static const MCSymbolRefExpr *create(const MCSymbol *Symbol, MCContext &Ctx,
+ SMLoc Loc = SMLoc()) {
+ return MCSymbolRefExpr::create(Symbol, VK_None, Ctx, Loc);
}
static const MCSymbolRefExpr *create(const MCSymbol *Symbol, VariantKind Kind,
MCContext &Ctx, SMLoc Loc = SMLoc());
- static const MCSymbolRefExpr *create(StringRef Name, VariantKind Kind,
- MCContext &Ctx);
/// @}
/// \name Accessors
@@ -630,8 +634,10 @@ class MCBinaryExpr : public MCExpr {
}
};
-/// This is an extension point for target-specific MCExpr subclasses to
-/// implement.
+/// Extension point for target-specific MCExpr subclasses to implement.
+/// This can encode a relocation operator, serving as a replacement for
+/// MCSymbolRefExpr::VariantKind. Ideally, limit this to
+/// top-level use, avoiding its inclusion as a subexpression.
///
/// NOTE: All subclasses are required to have trivial destructors because
/// MCExprs are bump pointer allocated and not destructed.
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index e1e2d8f815043..db3b441f4cced 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -246,11 +246,6 @@ const MCSymbolRefExpr *MCSymbolRefExpr::create(const MCSymbol *Sym,
return new (Ctx) MCSymbolRefExpr(Sym, Kind, Ctx.getAsmInfo(), Loc);
}
-const MCSymbolRefExpr *MCSymbolRefExpr::create(StringRef Name, VariantKind Kind,
- MCContext &Ctx) {
- return create(Ctx.getOrCreateSymbol(Name), Kind, Ctx);
-}
-
/* *** */
void MCTargetExpr::anchor() {}
diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index 3a0d5ea03dd53..ba58c10c3515a 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -12438,9 +12438,9 @@ bool ARMAsmParser::parseDirectiveTLSDescSeq(SMLoc L) {
if (getLexer().isNot(AsmToken::Identifier))
return TokError("expected variable after '.tlsdescseq' directive");
- const MCSymbolRefExpr *SRE =
- MCSymbolRefExpr::create(Parser.getTok().getIdentifier(),
- MCSymbolRefExpr::VK_ARM_TLSDESCSEQ, getContext());
+ auto *Sym = getContext().getOrCreateSymbol(Parser.getTok().getIdentifier());
+ const auto *SRE = MCSymbolRefExpr::create(
+ Sym, MCSymbolRefExpr::VK_ARM_TLSDESCSEQ, getContext());
Lex();
if (parseEOL())
diff --git a/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp b/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
index a2ca90c35bad3..6ed77d45b8a74 100644
--- a/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
+++ b/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
@@ -1269,9 +1269,7 @@ void MipsTargetELFStreamer::emitDirectiveCpLoad(unsigned RegNo) {
TmpInst.setOpcode(Mips::LUi);
TmpInst.addOperand(MCOperand::createReg(GPReg));
const MCExpr *HiSym = MipsMCExpr::create(
- MipsMCExpr::MEK_HI,
- MCSymbolRefExpr::create("_gp_disp", MCSymbolRefExpr::VK_None,
- MCA.getContext()),
+ MipsMCExpr::MEK_HI, MCSymbolRefExpr::create(GP_Disp, MCA.getContext()),
MCA.getContext());
TmpInst.addOperand(MCOperand::createExpr(HiSym));
getStreamer().emitInstruction(TmpInst, STI);
@@ -1282,9 +1280,7 @@ void MipsTargetELFStreamer::emitDirectiveCpLoad(unsigned RegNo) {
TmpInst.addOperand(MCOperand::createReg(GPReg));
TmpInst.addOperand(MCOperand::createReg(GPReg));
const MCExpr *LoSym = MipsMCExpr::create(
- MipsMCExpr::MEK_LO,
- MCSymbolRefExpr::create("_gp_disp", MCSymbolRefExpr::VK_None,
- MCA.getContext()),
+ MipsMCExpr::MEK_LO, MCSymbolRefExpr::create(GP_Disp, MCA.getContext()),
MCA.getContext());
TmpInst.addOperand(MCOperand::createExpr(LoSym));
getStreamer().emitInstruction(TmpInst, STI);
diff --git a/llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp b/llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
index aeae1619a187f..d20037605d7a4 100644
--- a/llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
+++ b/llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
@@ -1539,8 +1539,8 @@ bool PPCAsmParser::parseOperand(OperandVector &Operands) {
if (!(parseOptionalToken(AsmToken::Identifier) &&
Tok.getString().compare_insensitive("plt") == 0))
return Error(Tok.getLoc(), "expected 'plt'");
- EVal = MCSymbolRefExpr::create(TlsGetAddr, MCSymbolRefExpr::VK_PLT,
- getContext());
+ EVal = MCSymbolRefExpr::create(getContext().getOrCreateSymbol(TlsGetAddr),
+ MCSymbolRefExpr::VK_PLT, getContext());
if (parseOptionalToken(AsmToken::Plus)) {
const MCExpr *Addend = nullptr;
SMLoc EndLoc;
More information about the llvm-commits
mailing list