Skip to content
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

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Mar 22, 2025

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.

MaskRay added 2 commits March 22, 2025 15:03
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Mar 22, 2025

@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-lld-elf
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-risc-v

Author: Fangrui Song (MaskRay)

Changes

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.


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:

  • (modified) lld/test/ELF/riscv-reloc-plt32.s (+3-3)
  • (modified) lld/test/ELF/riscv-undefined-weak.s (+1-1)
  • (modified) lld/test/ELF/riscv64-reloc-got32-pcrel.s (+8-8)
  • (modified) llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h (+6)
  • (modified) llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h (+6)
  • (modified) llvm/include/llvm/Target/TargetLoweringObjectFile.h (+1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+6-5)
  • (modified) llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (+23-7)
  • (modified) llvm/lib/MC/MCAssembler.cpp (+1-1)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+1-1)
  • (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+24-7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp (+11-8)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp (-7)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp (+11-4)
  • (modified) llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp (+8)
  • (modified) llvm/lib/Target/RISCV/RISCVTargetObjectFile.h (+3)
  • (modified) llvm/test/CodeGen/RISCV/dso_local_equivalent.ll (+1-1)
  • (modified) llvm/test/MC/RISCV/data-directive-specifier.s (+11-11)
  • (modified) llvm/test/MC/RISCV/pseudo-jump-invalid.s (+1-1)
  • (modified) llvm/test/MC/RISCV/rv32i-invalid.s (+1)
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]

@MaskRay MaskRay requested review from topperc, lenary and PiJoules March 22, 2025 22:07
@@ -1194,18 +1194,34 @@ const MCExpr *TargetLoweringObjectFileELF::lowerRelativeReference(
MCSymbolRefExpr::create(TM.getSymbol(RHS), getContext()), getContext());
Copy link
Member Author

@MaskRay MaskRay Mar 22, 2025

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).

MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 22, 2025
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
.word foo@PLT - .
.word foo@PLT - . + 1
.word foo@PLT - . - 1
.word %plt(foo - .)
Copy link
Collaborator

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...

Copy link
Member Author

@MaskRay MaskRay Mar 23, 2025

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 - .).

Copy link
Collaborator

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?

Copy link
Member Author

@MaskRay MaskRay Mar 23, 2025

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.)

Copy link
Collaborator

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?

Copy link
Member

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.

Copy link
Member Author

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...

Copy link
Collaborator

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.

Copy link
Member Author

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.)

Copy link
Collaborator

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

MaskRay added 2 commits March 22, 2025 20:36
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 23, 2025
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
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 24, 2025
…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
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 24, 2025
…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
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 24, 2025
…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
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 24, 2025
…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
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 24, 2025
…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
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 24, 2025
…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
MaskRay added 2 commits March 23, 2025 22:44
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@MaskRay MaskRay changed the title [RISCV] Replace @plt/@gotpcrel in data directives with %plt %gotpcrel [RISCV] Replace @plt/@gotpcrel in data directives with %pltpcrel %gotpcrel Mar 24, 2025
@@ -2289,25 +2314,6 @@ bool TargetLoweringObjectFileWasm::shouldPutJumpTableInFunctionSection(
return false;
}

const MCExpr *TargetLoweringObjectFileWasm::lowerRelativeReference(
Copy link
Member Author

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.

Copy link
Member

@lenary lenary left a 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) {
Copy link
Member

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.

Copy link
Member Author

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.

ChuanqiXu9 and others added 4 commits March 27, 2025 20:42
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
@MaskRay MaskRay changed the base branch from users/MaskRay/spr/main.riscv-replace-pltgotpcrel-in-data-directives-with-plt-gotpcrel to main March 29, 2025 18:08
@MaskRay MaskRay merged commit fe6fb91 into main Mar 29, 2025
6 of 12 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/riscv-replace-pltgotpcrel-in-data-directives-with-plt-gotpcrel branch March 29, 2025 18:08
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Mar 29, 2025
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants