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] Support Global Dynamic TLSDESC in the RISC-V backend #66915

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Sep 20, 2023

This patch adds basic TLSDESC support for the global dynamic case in the RISC-V backend.

Specifically, we add new relocation types for TLSDESC, as prescribed in
riscv-non-isa/riscv-elf-psabi-doc#373, and add a new pseudo instruction
to simplify code generation.

This patch does not try to optimize the local dynamic case, which can be improved in separate
patches. Linker side changes will also be handled separately.

The current implementation is only enabled when passing the new -enable-tlsdesc codegen flag.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 20, 2023

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

@llvm/pr-subscribers-llvm-binary-utilities

Changes

This patch adds basic TLSDESC support for the global dynamic case in the RISC-V backend.

Specifically, we add new relocation types for TLSDESC, as prescribed in
riscv-non-isa/riscv-elf-psabi-doc#373, and add a new pseudo instruction
to simplify code generation.

This patch does not try to optimize the local dynamic case, which can be improved in separate
patches. Linker side changes will also be handled separately.

The current implementation is only enabled when passing the new -riscv-enable-tlsdesc flag.


Patch is 36.27 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66915.diff

17 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def (+4)
  • (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+88-9)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp (+9)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h (+5-1)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp (+15)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h (+12)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp (+46)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp (+18)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h (+4)
  • (modified) llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp (+12)
  • (modified) llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp (+53)
  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+22-1)
  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.h (+3)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+5-1)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+36)
  • (modified) llvm/lib/Target/RISCV/RISCVTargetMachine.cpp (+4)
  • (modified) llvm/test/CodeGen/RISCV/tls-models.ll (+96)
diff --git a/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def b/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
index 9a126df01531195..94420395fa0fadd 100644
--- a/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
+++ b/llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
@@ -55,3 +55,7 @@ ELF_RELOC(R_RISCV_SET32,             56)
 ELF_RELOC(R_RISCV_32_PCREL,          57)
 ELF_RELOC(R_RISCV_IRELATIVE,         58)
 ELF_RELOC(R_RISCV_PLT32,             59)
+ELF_RELOC(R_RISCV_TLSDESC_HI20,      62)
+ELF_RELOC(R_RISCV_TLSDESC_LOAD_LO12, 63)
+ELF_RELOC(R_RISCV_TLSDESC_ADD_LO12,  64)
+ELF_RELOC(R_RISCV_TLSDESC_CALL,      65)
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 7d8d82e381313bf..1303f5e85aeeb65 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -152,6 +152,7 @@ class RISCVAsmParser : public MCTargetAsmParser {
   // Helper to emit pseudo instruction "la.tls.gd" used in global-dynamic TLS
   // addressing.
   void emitLoadTLSGDAddress(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out);
+  void emitLoadTLSDescAddress(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out);
 
   // Helper to emit pseudo load/store instruction with a symbol.
   void emitLoadStoreSymbol(MCInst &Inst, unsigned Opcode, SMLoc IDLoc,
@@ -170,6 +171,12 @@ class RISCVAsmParser : public MCTargetAsmParser {
   // 'add' is an overloaded mnemonic.
   bool checkPseudoAddTPRel(MCInst &Inst, OperandVector &Operands);
 
+  // Checks that a PseudoTLSDESCCall is using x5/t0 in its output operand.
+  // Enforcing this using a restricted register class for the output
+  // operand of PseudoTLSDESCCall results in a poor diagnostic due to the fact
+  // 'jalr' is an overloaded mnemonic.
+  bool checkPseudoTLSDESCCall(MCInst &Inst, OperandVector &Operands);
+
   // Check instruction constraints.
   bool validateInstruction(MCInst &Inst, OperandVector &Operands);
 
@@ -533,6 +540,16 @@ struct RISCVOperand final : public MCParsedAsmOperand {
            VK == RISCVMCExpr::VK_RISCV_TPREL_ADD;
   }
 
+  bool isTLSDESCCallSymbol() const {
+    int64_t Imm;
+    RISCVMCExpr::VariantKind VK = RISCVMCExpr::VK_RISCV_None;
+    // Must be of 'immediate' type but not a constant.
+    if (!isImm() || evaluateConstantImm(getImm(), Imm, VK))
+      return false;
+    return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
+           VK == RISCVMCExpr::VK_RISCV_TLSDESC_CALL;
+  }
+
   bool isCSRSystemRegister() const { return isSystemRegister(); }
 
   bool isVTypeImm(unsigned N) const {
@@ -584,7 +601,10 @@ struct RISCVOperand final : public MCParsedAsmOperand {
     if (!isImm())
       return false;
     bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
-    if (VK == RISCVMCExpr::VK_RISCV_LO || VK == RISCVMCExpr::VK_RISCV_PCREL_LO)
+    if (VK == RISCVMCExpr::VK_RISCV_LO ||
+        VK == RISCVMCExpr::VK_RISCV_PCREL_LO ||
+        VK == RISCVMCExpr::VK_RISCV_TLSDESC_LOAD_LO ||
+        VK == RISCVMCExpr::VK_RISCV_TLSDESC_ADD_LO)
       return true;
     // Given only Imm, ensuring that the actually specified constant is either
     // a signed or unsigned 64-bit number is unfortunately impossible.
@@ -837,7 +857,9 @@ struct RISCVOperand final : public MCParsedAsmOperand {
     return IsValid && ((IsConstantImm && VK == RISCVMCExpr::VK_RISCV_None) ||
                        VK == RISCVMCExpr::VK_RISCV_LO ||
                        VK == RISCVMCExpr::VK_RISCV_PCREL_LO ||
-                       VK == RISCVMCExpr::VK_RISCV_TPREL_LO);
+                       VK == RISCVMCExpr::VK_RISCV_TPREL_LO ||
+                       VK == RISCVMCExpr::VK_RISCV_TLSDESC_LOAD_LO ||
+                       VK == RISCVMCExpr::VK_RISCV_TLSDESC_ADD_LO);
   }
 
   bool isSImm12Lsb0() const { return isBareSimmNLsb0<12>(); }
@@ -894,14 +916,16 @@ struct RISCVOperand final : public MCParsedAsmOperand {
       return IsValid && (VK == RISCVMCExpr::VK_RISCV_PCREL_HI ||
                          VK == RISCVMCExpr::VK_RISCV_GOT_HI ||
                          VK == RISCVMCExpr::VK_RISCV_TLS_GOT_HI ||
-                         VK == RISCVMCExpr::VK_RISCV_TLS_GD_HI);
-    } else {
-      return isUInt<20>(Imm) && (VK == RISCVMCExpr::VK_RISCV_None ||
-                                 VK == RISCVMCExpr::VK_RISCV_PCREL_HI ||
-                                 VK == RISCVMCExpr::VK_RISCV_GOT_HI ||
-                                 VK == RISCVMCExpr::VK_RISCV_TLS_GOT_HI ||
-                                 VK == RISCVMCExpr::VK_RISCV_TLS_GD_HI);
+                         VK == RISCVMCExpr::VK_RISCV_TLS_GD_HI ||
+                         VK == RISCVMCExpr::VK_RISCV_TLSDESC_HI);
     }
+
+    return isUInt<20>(Imm) && (VK == RISCVMCExpr::VK_RISCV_None ||
+                               VK == RISCVMCExpr::VK_RISCV_PCREL_HI ||
+                               VK == RISCVMCExpr::VK_RISCV_GOT_HI ||
+                               VK == RISCVMCExpr::VK_RISCV_TLS_GOT_HI ||
+                               VK == RISCVMCExpr::VK_RISCV_TLS_GD_HI ||
+                               VK == RISCVMCExpr::VK_RISCV_TLSDESC_HI);
   }
 
   bool isSImm21Lsb0JAL() const { return isBareSimmNLsb0<21>(); }
@@ -1515,6 +1539,11 @@ bool RISCVAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
     SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
     return Error(ErrorLoc, "operand must be a symbol with %tprel_add modifier");
   }
+  case Match_InvalidTLSDESCCallSymbol: {
+    SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
+    return Error(ErrorLoc,
+                 "operand must be a symbol with %tlsdesc_call modifier");
+  }
   case Match_InvalidRTZArg: {
     SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
     return Error(ErrorLoc, "operand must be 'rtz' floating-point rounding mode");
@@ -3107,6 +3136,41 @@ void RISCVAsmParser::emitLoadTLSGDAddress(MCInst &Inst, SMLoc IDLoc,
                     RISCV::ADDI, IDLoc, Out);
 }
 
+void RISCVAsmParser::emitLoadTLSDescAddress(MCInst &Inst, SMLoc IDLoc,
+                                            MCStreamer &Out) {
+  // The load TLS GD address pseudo-instruction "la.tlsdesc" is used in
+  // global-dynamic TLS model addressing of global symbols:
+  //   la.tlsdesc rdest, symbol
+  // expands to
+  //   TmpLabel: AUIPC rdest, %tlsdesc_hi(symbol)
+  //             ADDI rdest, rdest, %pcrel_lo(TmpLabel)
+  MCOperand DestReg = Inst.getOperand(0);
+  const MCExpr *Symbol = Inst.getOperand(1).getExpr();
+
+  MCContext &Ctx = getContext();
+
+  MCSymbol *TmpLabel = Ctx.createNamedTempSymbol("pcrel_hi");
+  Out.emitLabel(TmpLabel);
+
+  const RISCVMCExpr *SymbolHi =
+      RISCVMCExpr::create(Symbol, RISCVMCExpr::VK_RISCV_TLSDESC_HI, Ctx);
+  emitToStreamer(
+      Out, MCInstBuilder(RISCV::AUIPC).addOperand(DestReg).addExpr(SymbolHi));
+
+  const MCExpr *RefToLinkTmpLabel =
+      RISCVMCExpr::create(MCSymbolRefExpr::create(TmpLabel, Ctx),
+                          RISCVMCExpr::VK_RISCV_TLSDESC_LOAD_LO, Ctx);
+
+  emitToStreamer(Out, MCInstBuilder(RISCV::ADDI)
+                          .addOperand(DestReg)
+                          .addOperand(DestReg)
+                          .addExpr(RefToLinkTmpLabel));
+
+  emitToStreamer(
+      Out,
+      MCInstBuilder(RISCV::JALR).addReg(RISCV::X5).addReg(RISCV::X5).addImm(0));
+}
+
 void RISCVAsmParser::emitLoadStoreSymbol(MCInst &Inst, unsigned Opcode,
                                          SMLoc IDLoc, MCStreamer &Out,
                                          bool HasTmpReg) {
@@ -3246,6 +3310,18 @@ bool RISCVAsmParser::checkPseudoAddTPRel(MCInst &Inst,
 
   return false;
 }
+bool RISCVAsmParser::checkPseudoTLSDESCCall(MCInst &Inst,
+                                            OperandVector &Operands) {
+  assert(Inst.getOpcode() == RISCV::PseudoTLSDESCCall && "Invalid instruction");
+  assert(Inst.getOperand(0).isReg() && "Unexpected operand kind");
+  if (Inst.getOperand(0).getReg() != RISCV::X5) {
+    SMLoc ErrorLoc = ((RISCVOperand &)*Operands[3]).getStartLoc();
+    return Error(ErrorLoc, "the output operand must be t0/x5 when using "
+                           "%tlsdesc_call modifier");
+  }
+
+  return false;
+}
 
 std::unique_ptr<RISCVOperand> RISCVAsmParser::defaultMaskRegOp() const {
   return RISCVOperand::createReg(RISCV::NoRegister, llvm::SMLoc(),
@@ -3443,6 +3519,9 @@ bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
   case RISCV::PseudoLA_TLS_GD:
     emitLoadTLSGDAddress(Inst, IDLoc, Out);
     return false;
+  case RISCV::PseudoLA_TLSDESC:
+    emitLoadTLSDescAddress(Inst, IDLoc, Out);
+    return false;
   case RISCV::PseudoLB:
     emitLoadStoreSymbol(Inst, RISCV::LB, IDLoc, Out, /*HasTmpReg=*/false);
     return false;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index ca5aeb943c3be75..0b070e72c3ecb3d 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -80,6 +80,11 @@ RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
       {"fixup_riscv_call_plt", 0, 64, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_riscv_relax", 0, 0, 0},
       {"fixup_riscv_align", 0, 0, 0},
+
+      {"fixup_riscv_tlsdesc_hi20", 12, 20, MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget},
+      {"fixup_riscv_tlsdesc_load_lo12", 20, 12, 0},
+      {"fixup_riscv_tlsdesc_add_lo12", 20, 12, 0},
+      {"fixup_riscv_tlsdesc_call", 0, 0, 0},
   };
   static_assert((std::size(Infos)) == RISCV::NumTargetFixupKinds,
                 "Not all fixup kinds added to Infos array");
@@ -118,6 +123,7 @@ bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
   case RISCV::fixup_riscv_got_hi20:
   case RISCV::fixup_riscv_tls_got_hi20:
   case RISCV::fixup_riscv_tls_gd_hi20:
+  case RISCV::fixup_riscv_tlsdesc_hi20:
     return true;
   }
 
@@ -390,6 +396,7 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
   case RISCV::fixup_riscv_got_hi20:
   case RISCV::fixup_riscv_tls_got_hi20:
   case RISCV::fixup_riscv_tls_gd_hi20:
+  case RISCV::fixup_riscv_tlsdesc_hi20:
     llvm_unreachable("Relocation should be unconditionally forced\n");
   case FK_Data_1:
   case FK_Data_2:
@@ -399,6 +406,7 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
   case RISCV::fixup_riscv_lo12_i:
   case RISCV::fixup_riscv_pcrel_lo12_i:
   case RISCV::fixup_riscv_tprel_lo12_i:
+  case RISCV::fixup_riscv_tlsdesc_load_lo12:
     return Value & 0xfff;
   case RISCV::fixup_riscv_12_i:
     if (!isInt<12>(Value)) {
@@ -502,6 +510,7 @@ bool RISCVAsmBackend::evaluateTargetFixup(
   switch (Fixup.getTargetKind()) {
   default:
     llvm_unreachable("Unexpected fixup kind!");
+  case RISCV::fixup_riscv_tlsdesc_hi20:
   case RISCV::fixup_riscv_pcrel_hi20:
     AUIPCFixup = &Fixup;
     AUIPCDF = DF;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 20ff26a39dc3b30..244d2e56c9fa2e7 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -256,11 +256,15 @@ enum {
   MO_TPREL_ADD = 10,
   MO_TLS_GOT_HI = 11,
   MO_TLS_GD_HI = 12,
+  MO_TLSDESC_HI = 13,
+  MO_TLSDESC_LOAD_LO = 14,
+  MO_TLSDESC_ADD_LO = 15,
+  MO_TLSDESC_CALL = 16,
 
   // Used to differentiate between target-specific "direct" flags and "bitmask"
   // flags. A machine operand can only have one "direct" flag, but can have
   // multiple "bitmask" flags.
-  MO_DIRECT_FLAG_MASK = 15
+  MO_DIRECT_FLAG_MASK = 31
 };
 } // namespace RISCVII
 
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
index 0799267eaf7c769..bf73b82eaea880c 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
@@ -77,6 +77,14 @@ unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx,
       return ELF::R_RISCV_TLS_GOT_HI20;
     case RISCV::fixup_riscv_tls_gd_hi20:
       return ELF::R_RISCV_TLS_GD_HI20;
+    case RISCV::fixup_riscv_tlsdesc_hi20:
+      return ELF::R_RISCV_TLSDESC_HI20;
+    case RISCV::fixup_riscv_tlsdesc_load_lo12:
+      return ELF::R_RISCV_TLSDESC_LOAD_LO12;
+    case RISCV::fixup_riscv_tlsdesc_add_lo12:
+      return ELF::R_RISCV_TLSDESC_ADD_LO12;
+    case RISCV::fixup_riscv_tlsdesc_call:
+      return ELF::R_RISCV_TLSDESC_CALL;
     case RISCV::fixup_riscv_jal:
       return ELF::R_RISCV_JAL;
     case RISCV::fixup_riscv_branch:
@@ -96,6 +104,13 @@ unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx,
   default:
     Ctx.reportError(Fixup.getLoc(), "unsupported relocation type");
     return ELF::R_RISCV_NONE;
+  case RISCV::fixup_riscv_tlsdesc_load_lo12:
+    return ELF::R_RISCV_TLSDESC_LOAD_LO12;
+  case RISCV::fixup_riscv_tlsdesc_add_lo12:
+    return ELF::R_RISCV_TLSDESC_ADD_LO12;
+  case RISCV::fixup_riscv_tlsdesc_call:
+    return ELF::R_RISCV_TLSDESC_CALL;
+
   case FK_Data_1:
     Ctx.reportError(Fixup.getLoc(), "1-byte data relocations not supported");
     return ELF::R_RISCV_NONE;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h
index f3d0841eb6bca64..3e8239ac08458b8 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h
@@ -71,6 +71,18 @@ enum Fixups {
   // Used to generate an R_RISCV_ALIGN relocation, which indicates the linker
   // should fixup the alignment after linker relaxation.
   fixup_riscv_align,
+  // 20-bit fixup corresponding to %tlsdesc_hi(foo) for instructions like
+  // auipc
+  fixup_riscv_tlsdesc_hi20,
+  // 12-bit fixup corresponding to %tlsdesc_load_lo(foo) for instructions like
+  // addi
+  fixup_riscv_tlsdesc_load_lo12,
+  // 12-bit fixup corresponding to %tlsdesc_add_lo(foo) for instructions like
+  // addi
+  fixup_riscv_tlsdesc_add_lo12,
+  // Fixup representing a function call to TLS descriptor resolve function,
+  // %tlsdesc_call
+  fixup_riscv_tlsdesc_call,
 
   // Used as a sentinel, must be the last
   fixup_riscv_invalid,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index 716c3ac14d116ba..a0f6c74ba7d9f8f 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -57,6 +57,10 @@ class RISCVMCCodeEmitter : public MCCodeEmitter {
                           SmallVectorImpl<MCFixup> &Fixups,
                           const MCSubtargetInfo &STI) const;
 
+  void expandTLSDESCCall(const MCInst &MI, SmallVectorImpl<char> &CB,
+                         SmallVectorImpl<MCFixup> &Fixups,
+                         const MCSubtargetInfo &STI) const;
+
   void expandAddTPRel(const MCInst &MI, SmallVectorImpl<char> &CB,
                       SmallVectorImpl<MCFixup> &Fixups,
                       const MCSubtargetInfo &STI) const;
@@ -150,6 +154,31 @@ void RISCVMCCodeEmitter::expandFunctionCall(const MCInst &MI,
   support::endian::write(CB, Binary, support::little);
 }
 
+void RISCVMCCodeEmitter::expandTLSDESCCall(const MCInst &MI, SmallVectorImpl<char> &CB,
+                                           SmallVectorImpl<MCFixup> &Fixups,
+                                           const MCSubtargetInfo &STI) const {
+  MCOperand SrcSymbol = MI.getOperand(3);
+  assert(SrcSymbol.isExpr() &&
+         "Expected expression as first input to TLSDESCCALL");
+  const RISCVMCExpr *Expr = dyn_cast<RISCVMCExpr>(SrcSymbol.getExpr());
+  MCRegister Link = MI.getOperand(0).getReg();
+  MCRegister Dest = MI.getOperand(1).getReg();
+  MCRegister Imm = MI.getOperand(2).getImm();
+  Fixups.push_back(MCFixup::create(
+      0, Expr, MCFixupKind(RISCV::fixup_riscv_tlsdesc_call), MI.getLoc()));
+  // Emit fixup_riscv_relax for jalr where the relax feature is enabled.
+  if (STI.hasFeature(RISCV::FeatureRelax)) {
+    const MCConstantExpr *Dummy = MCConstantExpr::create(0, Ctx);
+    Fixups.push_back(MCFixup::create(
+        0, Dummy, MCFixupKind(RISCV::fixup_riscv_relax), MI.getLoc()));
+  }
+  MCInst Call =
+      MCInstBuilder(RISCV::JALR).addReg(Link).addReg(Dest).addImm(Imm);
+
+  uint32_t Binary = getBinaryCodeForInstr(Call, Fixups, STI);
+  support::endian::write(CB, Binary, support::little);
+}
+
 // Expand PseudoAddTPRel to a simple ADD with the correct relocation.
 void RISCVMCCodeEmitter::expandAddTPRel(const MCInst &MI,
                                         SmallVectorImpl<char> &CB,
@@ -299,6 +328,10 @@ void RISCVMCCodeEmitter::encodeInstruction(const MCInst &MI,
     expandLongCondBr(MI, CB, Fixups, STI);
     MCNumEmitted += 2;
     return;
+  case RISCV::PseudoTLSDESCCall:
+    expandTLSDESCCall(MI, CB, Fixups, STI);
+    MCNumEmitted += 1;
+    return;
   }
 
   switch (Size) {
@@ -441,6 +474,19 @@ unsigned RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
       FixupKind = RISCV::fixup_riscv_call_plt;
       RelaxCandidate = true;
       break;
+    case RISCVMCExpr::VK_RISCV_TLSDESC_HI:
+      FixupKind = RISCV::fixup_riscv_tlsdesc_hi20;
+      break;
+    case RISCVMCExpr::VK_RISCV_TLSDESC_LOAD_LO:
+      FixupKind = RISCV::fixup_riscv_tlsdesc_load_lo12;
+      break;
+    case RISCVMCExpr::VK_RISCV_TLSDESC_ADD_LO:
+      FixupKind = RISCV::fixup_riscv_tlsdesc_add_lo12;
+      break;
+    case RISCVMCExpr::VK_RISCV_TLSDESC_CALL:
+      FixupKind = RISCV::fixup_riscv_tlsdesc_call;
+      RelaxCandidate = true;
+      break;
     }
   } else if ((Kind == MCExpr::SymbolRef &&
                  cast<MCSymbolRefExpr>(Expr)->getKind() ==
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
index d67351102bc1cde..35d0b7dfff3c39e 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
@@ -81,6 +81,7 @@ const MCFixup *RISCVMCExpr::getPCRelHiFixup(const MCFragment **DFOut) const {
     case RISCV::fixup_riscv_tls_got_hi20:
     case RISCV::fixup_riscv_tls_gd_hi20:
     case RISCV::fixup_riscv_pcrel_hi20:
+    case RISCV::fixup_riscv_tlsdesc_hi20:
       if (DFOut)
         *DFOut = DF;
       return &F;
@@ -121,6 +122,10 @@ RISCVMCExpr::VariantKind RISCVMCExpr::getVariantKindForName(StringRef name) {
       .Case("tprel_add", VK_RISCV_TPREL_ADD)
       .Case("tls_ie_pcrel_hi", VK_RISCV_TLS_GOT_HI)
       .Case("tls_gd_pcrel_hi", VK_RISCV_TLS_GD_HI)
+      .Case("tlsdesc_hi", VK_RISCV_TLSDESC_HI)
+      .Case("tlsdesc_load_lo", VK_RISCV_TLSDESC_LOAD_LO)
+      .Case("tlsdesc_add_lo", VK_RISCV_TLSDESC_ADD_LO)
+      .Case("tlsdesc_call", VK_RISCV_TLSDESC_CALL)
       .Default(VK_RISCV_Invalid);
 }
 
@@ -147,6 +152,14 @@ StringRef RISCVMCExpr::getVariantKindName(VariantKind Kind) {
     return "tprel_add";
   case VK_RISCV_TLS_GOT_HI:
     return "tls_ie_pcrel_hi";
+  case VK_RISCV_TLSDESC_HI:
+    return "tlsdesc_hi";
+  case VK_RISCV_TLSDESC_LOAD_LO:
+    return "tlsdesc_load_lo";
+  case VK_RISCV_TLSDESC_ADD_LO:
+    return "tlsdesc_add_lo";
+  case VK_RISCV_TLSDESC_CALL:
+    return "tlsdesc_call";
   case VK_RISCV_TLS_GD_HI:
     return "tls_gd_pcrel_hi";
   case VK_RISCV_CALL:
@@ -195,6 +208,9 @@ void RISCVMCExpr::fixELFSymbolsInTLSFixups(MCAssembler &Asm) const {
   case VK_RISCV_TPREL_HI:
   case VK_RISCV_TLS_GOT_HI:
   case VK_RISCV_TLS_GD_HI:
+  case VK_RISCV_TLSDESC_HI:
+  case VK_RISCV_TLSDESC_ADD_LO:
+  case VK_RISCV_TLSDESC_LOAD_LO:
     break;
   }
 
@@ -208,6 +224,8 @@ bool RISCVMCExpr::evaluateAsConstant(int64_t &Res) const {
       Kind == VK_RISCV_GOT_HI || Kind == VK_RISCV_TPREL_HI ||
       Kind == VK_RISCV_TPREL_LO || Kind == VK_RISCV_TPREL_ADD ||
       Kind == VK_RISCV_TLS_GOT_HI || Kind == VK_RISCV_TLS_GD_HI ||
+      Kind == VK_RISCV_TLSDESC_HI || Kind == VK_RISCV_TLSDESC_LOAD_LO ||
+      Kind == VK_RISCV_TLSDESC_ADD_LO || Kind == VK_RISCV_TLSDESC_CALL ||
       Kind == VK_RISCV_CALL || Kind == VK_RISCV_CALL_PLT)
     return false;
 
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h
index ee83bf0208ef46e..fcc4c5c439645a9 100644
--- a/llvm/lib/Target/RISCV/MCT...
[truncated]

@ilovepi
Copy link
Contributor Author

ilovepi commented Oct 17, 2023

ping

@ilovepi
Copy link
Contributor Author

ilovepi commented Nov 28, 2023

ping. @topperc @jrtc27 @asb can we take a look at this? psABI has approved TLSDESC, and there are patches out for GCC and binutils.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a test in test/MC/RISCV for the AsmParser?


emitToStreamer(
Out,
MCInstBuilder(RISCV::JALR).addReg(RISCV::X5).addReg(RISCV::X5).addImm(0));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this use DestReg instead of X5?

// la.tlsdesc rdest, symbol
// expands to
// TmpLabel: AUIPC rdest, %tlsdesc_hi(symbol)
// ADDI rdest, rdest, %pcrel_lo(TmpLabel)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the JALR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually now that you’ve mentioned that bug in the documentation I’m questioning whether using the pseudo instruction is the correct approach. I followed the example for the existing TLS implementations, but those are also mentioned in the psABI and I think this may have been my own invention when I was first learning my way around the backend (and still am). To work correctly I think the pseudo would either need another register or I guess assemblers would need to scavenge one. Either way I don’t think that is quite right. Well … unless there is a way to make a pseudo (or something like) it completely internal, which I’m not aware of.

In that case, I guess I would need to translate the pseudo expansion into SelectionDag code instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, AArch64 has the pseudo TLSDESC_CALLSEQ, and nothing in the AsmParser. They do have a translation in the AsmPrinter though, which I don't think is functionally different than the pseudo expansion.

Do yo think its appropriate to omit the AsmParser bits here, since I don't expect for us to ever emit la.tlsdesc instructions? Otherwise, I'm not super sure how to adapt this, since its unclear to me how to write the code sequence, labels, and relocations in RISCVISelLowering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've gone ahead and dropped the AsmParser bits for la.tlsdesc, since we won't output that into assembly. Le tme now if you're happy with that change, or you'd prefer a different solution.

@llvmbot llvmbot added the mc Machine (object) code label Nov 29, 2023
Copy link

github-actions bot commented Nov 29, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

.addReg(RISCV::X4);

if (MI.hasOneMemOperand())
TPAdd->addMemOperand(*MF, *MI.memoperands_begin());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think an ADD instruction should have a memory operand.

I'm not sure where a MemOperand on the MI would come from, there wasn't one created in getGeneralDynamicTLSDescAddr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. I'll update the patch to reflect that.

@topperc
Copy link
Collaborator

topperc commented Nov 29, 2023

Do these instructions need to appear back to back in the final binary. I notice AArch mentions a linker optimization. Expanding in pre RA expansion would allow them to become separated.

@ilovepi
Copy link
Contributor Author

ilovepi commented Nov 29, 2023

I don't think they are required to be contiguous, based on https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#tls-descriptors .

specifically it mentions

- Instructions outside the sequence that do not clobber the registers used within the sequence may be inserted in-between the instructions of the sequence (known as instruction scheduling).

- Instructions in the sequence with no data dependency may be reordered. In the preceding example, the only instructions that can be reordered are lw and addi.

The relaxations listed here https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#tls-descriptors--initial-exec-relaxation and here https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#tls-descriptors--local-exec-relaxation don't seem to require them either.

My understanding of the discussion on the psABI thread is that the relocations should allow the linker to identify the instructions within the sequence, and it should have enough freedom given the number of instructions to relax it in all cases. I do think it would make relaxation far simpler if it was required to be a contiguous sequence.

@@ -6843,6 +6845,24 @@ SDValue RISCVTargetLowering::getDynamicTLSAddr(GlobalAddressSDNode *N,
return LowerCallTo(CLI).first;
}

SDValue
RISCVTargetLowering::getGeneralDynamicTLSDescAddr(GlobalAddressSDNode *N,
Copy link
Member

@MaskRay MaskRay Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GeneralDynamicTLSDesc in the name getGeneralDynamicTLSDescAddr is misleading. general-dynamic refers to a traditional TLS mode while TLSDESC is entirely different.

(People might wonder whether _TLS_MODULE_BASE_ from other arches is local-dynamic TLSDESC. No, it isn't. They just have some similarities.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, AArch64 handles lowering for llvm's general-dynamic and local-dynamic cases differently. I was trying to make it clear here that we're not trying to do anything fancy like elide loads of the __MODULE_BASE__ for multiple accesses. I'm not convinced we want to do that in RISC-V, so it's probably fine to rename this.

Maybe getTLSDescAddr is good enough? at least until we decide that we want to optimize them differently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getTLSDescAddr sounds good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.


.Ltlsdesc_hi0:
jalr x5, 0(a1), %tlsdesc_hi(.Ltlsdesc_hi0) # CHECK: :[[@LINE]]:17: error: operand must be a symbol with %tlsdesc_call modifier
jalr x1, 0(a1), %tlsdesc_call(.Ltlsdesc_hi0) # CHECK: :[[@LINE]]:12: error: the output operand must be t0/x5 when using %tlsdesc_call modifier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we are missing MC layer tests that don't generate an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I've added some that should address that i rv32-tlsdesc-valid.s and rv64-tlsdesc-valid.s

Copy link
Member

@MaskRay MaskRay Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's clearer to place these negative tests in tlsdesc.s using the .ifdef ERR pattern.

@@ -1722,6 +1722,35 @@ let hasSideEffects = 0, mayLoad = 0, mayStore = 0, Size = 8, isCodeGenOnly = 0,
isAsmParserOnly = 1 in
def PseudoLA_TLS_GD : Pseudo<(outs GPR:$dst), (ins bare_symbol:$src), [],
"la.tls.gd", "$dst, $src">;
let hasSideEffects = 0, mayLoad = 1, mayStore = 0, Size = 32, isCodeGenOnly = 0,
isAsmParserOnly = 1 in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This says AsmParserOnly, but I don't see any assembler tests for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I think I missed this when I dropped the AsmParser bits. So, I think that means I want to set isAsmParserOnly=0, isCodeGenOnly=0, isPseudo=1, which I think is the case now. Is that the correct way to model a pseudo that is only for some internal state that shouldn't be emitted to assembly?

That's my understanding from https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/include/llvm/Target/Target.td#L554 , at least, but I'm not sure if that is a correct interpretation or not.

@@ -597,7 +613,10 @@ struct RISCVOperand final : public MCParsedAsmOperand {
if (!isImm())
return false;
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
if (VK == RISCVMCExpr::VK_RISCV_LO || VK == RISCVMCExpr::VK_RISCV_PCREL_LO)
if (VK == RISCVMCExpr::VK_RISCV_LO ||
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these change to accept more values for VK are tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tests added in RISCV/MC should cover this now. Please let me know if I've missed one of the cases.

// auipc
fixup_riscv_tlsdesc_hi20,
// 12-bit fixup corresponding to %tlsdesc_load_lo(foo) for instructions like
// addi
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this say load instead of addi?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've fixed it now to use lw, but maybe it should just read ... for load instructions instead? WDYT?

@@ -3,8 +3,8 @@

# Out of range immediates
## simm12
flh ft1, -2049(a0) # CHECK: :[[@LINE]]:10: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo modifier or an integer in the range [-2048, 2047]
fsh ft2, 2048(a1) # CHECK: :[[@LINE]]:10: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo modifier or an integer in the range [-2048, 2047]
flh ft1, -2049(a0) # CHECK: :[[@LINE]]:10: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo/%tlsdesc_load_lo modifier or an integer in the range [-2048, 2047]
Copy link
Member

@MaskRay MaskRay Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

%tlsdesc_load_lo only makes sense for ld. It seems that changing it requires updates to lots of files. In addition, %tlsdesc_add_lo is not mentioned.

Perhaps just drop the diagnostic message change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.


## Check invalid usage
.ifdef ERR
auipc x1, %tlsdesc_call(foo) # ERR: :[[@LINE]]:12: error: operand must be a symbol with a %pcrel_hi/%got_pcrel_hi/%tls_ie_pcrel_hi/%tls_gd_pcrel_hi/%tlsdesc_hi modifier or an integer in the range
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: [[@LINE]] is deprecated. Use [[#@LINE]]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. done.

@@ -71,6 +71,18 @@ enum Fixups {
// Used to generate an R_RISCV_ALIGN relocation, which indicates the linker
// should fixup the alignment after linker relaxation.
fixup_riscv_align,
// 20-bit fixup corresponding to %tlsdesc_hi(foo) for instructions like
// auipc
fixup_riscv_tlsdesc_hi20,
Copy link
Member

@MaskRay MaskRay Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instructions like auipc

It seems that tlsdesc is auipc-style only, instead of lui-style. You can say that this is for auipc.

Instead of having 4 comments, perhaps use one single comment to cover the following four as they are not supposed to be used independently.

// Fixups indicating a TLS descriptor code sequence, corresponding to auipc, lw/ld, addi, and jalr, respectively.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we can just use MCFixupKind(FirstLiteralRelocationKind + ELF::R_RISCV_TLSDESC_CALL)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've adjusted the comments per your suggestion, but could you elaborate more on what you're thinking w.r.t. MCFixupKind(FirstLiteralRelocationKind + ELF::R_RISCV_TLSDESC_CALL)?

@ilovepi
Copy link
Contributor Author

ilovepi commented Jan 9, 2024

@topperc @MaskRay , do either of you have strong feelings about abandoning this in favor of #77515? That PR was made w/ spr and should be a bit easier to understand/review (since it won't have merge or fixup commits) and will make the LLD side changes easier to stack.

@ilovepi
Copy link
Contributor Author

ilovepi commented Jan 9, 2024

As @MaskRay pointed out in #66916, there's a bit too much context in this review, so please just ignore my earlier question about moving to a stacked PR.

@@ -80,6 +80,11 @@ static cl::opt<bool> EnableRISCVDeadRegisterElimination(
" them with stores to x0"),
cl::init(true));

// TODO: This should be controlled by -mtls-dialect=<option>
cl::opt<bool> EnableRISCVTLSDESC("riscv-enable-tlsdesc",
Copy link
Member

@MaskRay MaskRay Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add an option to codegen::InitTargetOptionsFromCodeGenFlags, then reference it using something like DAG.getTarget().useTLSDESC(). aarch64 ELF only implements TLSDESC for dynamic TLS models, it is fine for it to ignore useTLSDESC.

When Clang -mtls-dialect= is added, its initTargetOptions will set this TargetOptions member.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good suggestion. I was thinking of doing something like that as a follow up for the Clang driver patches, but I agree, its probably better to do it in this patch.

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the TLSDESC option is moved to use llvm/lib/CodeGen/CommandFlags.cpp

It's worth giving others some time to respond.

@ilovepi
Copy link
Contributor Author

ilovepi commented Jan 22, 2024

@topperc Is there anything else you think needs to be done for the code generation?

MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jan 23, 2024
Support
R_RISCV_TLSDESC_HI20/R_RISCV_TLSDESC_LOAD_LO12_I/R_RISCV_TLSDESC_ADD_LO12_I/R_RISCV_TLSDESC_CALL.
LOAD_LO12_I/ADD_LO12_I/CALL relocations reference a label at the HI20
location, which requires special handling. We save the value of HI20 to
be reused.

For -no-pie/-pie links, TLSDESC to initial-exec or local-exec
optimizations are eligible. Implement the relevant hooks
(R_RELAX_TLS_GD_TO_LE, R_RELAX_TLS_GD_TO_IE). AUIPC/L[DW] for
initial-exec uses GOT offsets, which may be adjusted in the presence of
linker relaxation.

```
// TLSDESC to LE/IE optimization
.Ltlsdesc_hi2:
  auipc a4, %tlsdesc_hi(c)                      # if R_RISCV_RELAX: remove; otherwise, NOP
  load  a5, %tlsdesc_load_lo(.Ltlsdesc_hi2)(a4) # if R_RISCV_RELAX: remove; otherwise, NOP
  addi  a0, a4, %tlsdesc_add_lo(.Ltlsdesc_hi2)
  jalr  t0, 0(a5), %tlsdesc_call(.Ltlsdesc_hi2)
  add   a0, a0, tp
```

* `riscv64-tlsdesc.s` is inspired by `i386-tlsdesc-gd.s` (https://reviews.llvm.org/D112582).
* `riscv64-tlsdesc-relax.s` tests linker relaxation.

DO NOT SUBMIT: rebase after llvm#66915 lands
@ilovepi
Copy link
Contributor Author

ilovepi commented Jan 23, 2024

@MaskRay I've rebased it for you.

@MaskRay
Copy link
Member

MaskRay commented Jan 23, 2024

The current implementation is only enabled when passing the new -riscv-enable-tlsdesc flag.

This needs an update. The cl::opt has been removed.

MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jan 23, 2024
Support
R_RISCV_TLSDESC_HI20/R_RISCV_TLSDESC_LOAD_LO12_I/R_RISCV_TLSDESC_ADD_LO12_I/R_RISCV_TLSDESC_CALL.
LOAD_LO12_I/ADD_LO12_I/CALL relocations reference a label at the HI20
location, which requires special handling. We save the value of HI20 to
be reused.

For -no-pie/-pie links, TLSDESC to initial-exec or local-exec
optimizations are eligible. Implement the relevant hooks
(R_RELAX_TLS_GD_TO_LE, R_RELAX_TLS_GD_TO_IE). AUIPC/L[DW] for
initial-exec uses GOT offsets, which may be adjusted in the presence of
linker relaxation.

```
// TLSDESC to LE/IE optimization
.Ltlsdesc_hi2:
  auipc a4, %tlsdesc_hi(c)                      # if R_RISCV_RELAX: remove; otherwise, NOP
  load  a5, %tlsdesc_load_lo(.Ltlsdesc_hi2)(a4) # if R_RISCV_RELAX: remove; otherwise, NOP
  addi  a0, a4, %tlsdesc_add_lo(.Ltlsdesc_hi2)
  jalr  t0, 0(a5), %tlsdesc_call(.Ltlsdesc_hi2)
  add   a0, a0, tp
```

* `riscv64-tlsdesc.s` is inspired by `i386-tlsdesc-gd.s` (https://reviews.llvm.org/D112582).
* `riscv64-tlsdesc-relax.s` tests linker relaxation.

DO NOT SUBMIT: rebase after llvm#66915 lands
@@ -0,0 +1,44 @@
# RUN: llvm-mc -filetype=obj -triple riscv32 < %s --defsym RV32=1 | llvm-objdump -d -M no-aliases - | FileCheck %s --check-prefixes=INST,RV32
# RUN: llvm-mc -filetype=obj -triple riscv64 < %s | llvm-objdump -d -M no-aliases - | FileCheck %s --check-prefixes=INST,RV64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use llvm-objdump -d -r to check relocations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

This patch adds basic TLSDESC support for the global dynamic case in the
RISC-V backend by adding new relocation types for TLSDESC, as prescribed
in riscv-non-isa/riscv-elf-psabi-doc#373.

We also add a new pseudo instruction to simplify code generation.

Possible improvements for the local dynamic case will be addressed in separate
patches.

The current implementation is only enabled when passing the -enable-tlsdesc
codegen flag.
@ilovepi
Copy link
Contributor Author

ilovepi commented Jan 24, 2024

@topperc I spoke to @MaskRay and he's satified w/ the patch now. Is there anything else you'd like to see addressed? or are we good to land this?

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ilovepi ilovepi merged commit 03a61d3 into llvm:main Jan 24, 2024
3 of 4 checks passed
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jan 24, 2024
Support
R_RISCV_TLSDESC_HI20/R_RISCV_TLSDESC_LOAD_LO12_I/R_RISCV_TLSDESC_ADD_LO12_I/R_RISCV_TLSDESC_CALL.
LOAD_LO12_I/ADD_LO12_I/CALL relocations reference a label at the HI20
location, which requires special handling. We save the value of HI20 to
be reused.

For -no-pie/-pie links, TLSDESC to initial-exec or local-exec
optimizations are eligible. Implement the relevant hooks
(R_RELAX_TLS_GD_TO_LE, R_RELAX_TLS_GD_TO_IE). AUIPC/L[DW] for
initial-exec uses GOT offsets, which may be adjusted in the presence of
linker relaxation.

```
// TLSDESC to LE/IE optimization
.Ltlsdesc_hi2:
  auipc a4, %tlsdesc_hi(c)                      # if R_RISCV_RELAX: remove; otherwise, NOP
  load  a5, %tlsdesc_load_lo(.Ltlsdesc_hi2)(a4) # if R_RISCV_RELAX: remove; otherwise, NOP
  addi  a0, a4, %tlsdesc_add_lo(.Ltlsdesc_hi2)
  jalr  t0, 0(a5), %tlsdesc_call(.Ltlsdesc_hi2)
  add   a0, a0, tp
```

* `riscv64-tlsdesc.s` is inspired by `i386-tlsdesc-gd.s` (https://reviews.llvm.org/D112582).
* `riscv64-tlsdesc-relax.s` tests linker relaxation.
* `riscv-tlsdesc-gd-mixed.s` is inspired by `x86-64-tlsdesc-gd-mixed.s` (https://reviews.llvm.org/D116900).

DO NOT SUBMIT: rebase after llvm#66915 lands
MaskRay added a commit that referenced this pull request Jan 25, 2024
Fix comment typos in #66915, and relocation type names related to the
example in the psABI
(riscv-non-isa/riscv-elf-psabi-doc#420).
MaskRay added a commit that referenced this pull request Jan 26, 2024
GCC supports -mtls-dialect= for several architectures to select TLSDESC.
This patch supports the following values

* x86: "gnu". "gnu2" (TLSDESC) is not supported yet.
* RISC-V: "trad" (general dynamic), "desc" (TLSDESC, see #66915)

AArch64 toolchains seem to support TLSDESC from the beginning, and the
general dynamic model has poor support. Nobody seems to use the option
-mtls-dialect= at all, so we don't bother with it.
There also seems very little interest in AArch32's TLSDESC support.

TLSDESC does not change IR, but affects object file generation. Without
a backend option the option is a no-op for in-process ThinLTO.

There seems no motivation to have fine-grained control mixing trad/desc
for TLS, so we just pass -mllvm, and don't bother with a modules flag
metadata or function attribute.

Co-authored-by: Paul Kirth <paulkirth@google.com>
@ilovepi ilovepi deleted the feature-gd-tlsdesc branch January 26, 2024 18:03
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jan 26, 2024
GCC supports -mtls-dialect= for several architectures to select TLSDESC.
This patch supports the following values

* x86: "gnu". "gnu2" (TLSDESC) is not supported yet.
* RISC-V: "trad" (general dynamic), "desc" (TLSDESC, see llvm#66915)

AArch64 toolchains seem to support TLSDESC from the beginning, and the
general dynamic model has poor support. Nobody seems to use the option
-mtls-dialect= at all, so we don't bother with it.
There also seems very little interest in AArch32's TLSDESC support.

TLSDESC does not change IR, but affects object file generation. Without
a backend option the option is a no-op for in-process ThinLTO.

There seems no motivation to have fine-grained control mixing trad/desc
for TLS, so we just pass -mllvm, and don't bother with a modules flag
metadata or function attribute.

Co-authored-by: Paul Kirth <paulkirth@google.com>
(cherry picked from commit 36b4a9c)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jan 26, 2024
GCC supports -mtls-dialect= for several architectures to select TLSDESC.
This patch supports the following values

* x86: "gnu". "gnu2" (TLSDESC) is not supported yet.
* RISC-V: "trad" (general dynamic), "desc" (TLSDESC, see llvm#66915)

AArch64 toolchains seem to support TLSDESC from the beginning, and the
general dynamic model has poor support. Nobody seems to use the option
-mtls-dialect= at all, so we don't bother with it.
There also seems very little interest in AArch32's TLSDESC support.

TLSDESC does not change IR, but affects object file generation. Without
a backend option the option is a no-op for in-process ThinLTO.

There seems no motivation to have fine-grained control mixing trad/desc
for TLS, so we just pass -mllvm, and don't bother with a modules flag
metadata or function attribute.

Co-authored-by: Paul Kirth <paulkirth@google.com>
(cherry picked from commit 36b4a9c)
tstellar pushed a commit that referenced this pull request Jan 27, 2024
GCC supports -mtls-dialect= for several architectures to select TLSDESC.
This patch supports the following values

* x86: "gnu". "gnu2" (TLSDESC) is not supported yet.
* RISC-V: "trad" (general dynamic), "desc" (TLSDESC, see #66915)

AArch64 toolchains seem to support TLSDESC from the beginning, and the
general dynamic model has poor support. Nobody seems to use the option
-mtls-dialect= at all, so we don't bother with it.
There also seems very little interest in AArch32's TLSDESC support.

TLSDESC does not change IR, but affects object file generation. Without
a backend option the option is a no-op for in-process ThinLTO.

There seems no motivation to have fine-grained control mixing trad/desc
for TLS, so we just pass -mllvm, and don't bother with a modules flag
metadata or function attribute.

Co-authored-by: Paul Kirth <paulkirth@google.com>
(cherry picked from commit 36b4a9c)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
GCC supports -mtls-dialect= for several architectures to select TLSDESC.
This patch supports the following values

* x86: "gnu". "gnu2" (TLSDESC) is not supported yet.
* RISC-V: "trad" (general dynamic), "desc" (TLSDESC, see llvm#66915)

AArch64 toolchains seem to support TLSDESC from the beginning, and the
general dynamic model has poor support. Nobody seems to use the option
-mtls-dialect= at all, so we don't bother with it.
There also seems very little interest in AArch32's TLSDESC support.

TLSDESC does not change IR, but affects object file generation. Without
a backend option the option is a no-op for in-process ThinLTO.

There seems no motivation to have fine-grained control mixing trad/desc
for TLS, so we just pass -mllvm, and don't bother with a modules flag
metadata or function attribute.

Co-authored-by: Paul Kirth <paulkirth@google.com>
(cherry picked from commit 36b4a9c)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
GCC supports -mtls-dialect= for several architectures to select TLSDESC.
This patch supports the following values

* x86: "gnu". "gnu2" (TLSDESC) is not supported yet.
* RISC-V: "trad" (general dynamic), "desc" (TLSDESC, see llvm#66915)

AArch64 toolchains seem to support TLSDESC from the beginning, and the
general dynamic model has poor support. Nobody seems to use the option
-mtls-dialect= at all, so we don't bother with it.
There also seems very little interest in AArch32's TLSDESC support.

TLSDESC does not change IR, but affects object file generation. Without
a backend option the option is a no-op for in-process ThinLTO.

There seems no motivation to have fine-grained control mixing trad/desc
for TLS, so we just pass -mllvm, and don't bother with a modules flag
metadata or function attribute.

Co-authored-by: Paul Kirth <paulkirth@google.com>
(cherry picked from commit 36b4a9c)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
GCC supports -mtls-dialect= for several architectures to select TLSDESC.
This patch supports the following values

* x86: "gnu". "gnu2" (TLSDESC) is not supported yet.
* RISC-V: "trad" (general dynamic), "desc" (TLSDESC, see llvm#66915)

AArch64 toolchains seem to support TLSDESC from the beginning, and the
general dynamic model has poor support. Nobody seems to use the option
-mtls-dialect= at all, so we don't bother with it.
There also seems very little interest in AArch32's TLSDESC support.

TLSDESC does not change IR, but affects object file generation. Without
a backend option the option is a no-op for in-process ThinLTO.

There seems no motivation to have fine-grained control mixing trad/desc
for TLS, so we just pass -mllvm, and don't bother with a modules flag
metadata or function attribute.

Co-authored-by: Paul Kirth <paulkirth@google.com>
(cherry picked from commit 36b4a9c)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
GCC supports -mtls-dialect= for several architectures to select TLSDESC.
This patch supports the following values

* x86: "gnu". "gnu2" (TLSDESC) is not supported yet.
* RISC-V: "trad" (general dynamic), "desc" (TLSDESC, see llvm#66915)

AArch64 toolchains seem to support TLSDESC from the beginning, and the
general dynamic model has poor support. Nobody seems to use the option
-mtls-dialect= at all, so we don't bother with it.
There also seems very little interest in AArch32's TLSDESC support.

TLSDESC does not change IR, but affects object file generation. Without
a backend option the option is a no-op for in-process ThinLTO.

There seems no motivation to have fine-grained control mixing trad/desc
for TLS, so we just pass -mllvm, and don't bother with a modules flag
metadata or function attribute.

Co-authored-by: Paul Kirth <paulkirth@google.com>
(cherry picked from commit 36b4a9c)
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.

None yet

4 participants