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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ELF_RELOC(R_RISCV_TLS_DTPREL32, 8)
ELF_RELOC(R_RISCV_TLS_DTPREL64, 9)
ELF_RELOC(R_RISCV_TLS_TPREL32, 10)
ELF_RELOC(R_RISCV_TLS_TPREL64, 11)
ELF_RELOC(R_RISCV_TLSDESC, 12)
ELF_RELOC(R_RISCV_BRANCH, 16)
ELF_RELOC(R_RISCV_JAL, 17)
ELF_RELOC(R_RISCV_CALL, 18)
Expand Down Expand Up @@ -56,3 +57,7 @@ ELF_RELOC(R_RISCV_IRELATIVE, 58)
ELF_RELOC(R_RISCV_PLT32, 59)
ELF_RELOC(R_RISCV_SET_ULEB128, 60)
Copy link
Member

Choose a reason for hiding this comment

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

Define the dynamic relocation type R_RISCV_TLSDESC 12 as well.

(The lld patch incorrectly uses R_RISCV_TLSDESC_CALL for the dynamic reloc. I am going to take over that part and TLS optimization.)

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 removed them per your request. Just so we'd have a version where they were in place, I added the code to make them relaxable. But I'm fine either way, so if you'd prefer we start w/o the relaxation relocations it should be good now.

Related: do you think its worth testing that they're not there?

Copy link
Member

Choose a reason for hiding this comment

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

Related: do you think its worth testing that they're not there?

I've made a comment on the test "I think we should use llvm-objdump -d -r to check relocations."
If you do that, -NEXT is sufficient to ensure that there is no R_RISCV_RELAX :)

With my experience on #79099 , I think two R_RISCV_RELAX are sufficient. I probably need to follow up on the spec.

ELF_RELOC(R_RISCV_SUB_ULEB128, 61)
ELF_RELOC(R_RISCV_TLSDESC_HI20, 62)
ilovepi marked this conversation as resolved.
Show resolved Hide resolved
ELF_RELOC(R_RISCV_TLSDESC_LOAD_LO12, 63)
ELF_RELOC(R_RISCV_TLSDESC_ADD_LO12, 64)
ELF_RELOC(R_RISCV_TLSDESC_CALL, 65)
3 changes: 3 additions & 0 deletions llvm/include/llvm/CodeGen/CommandFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ unsigned getTLSSize();
bool getEmulatedTLS();
std::optional<bool> getExplicitEmulatedTLS();

bool getEnableTLSDESC();
std::optional<bool> getExplicitEnableTLSDESC();

bool getUniqueSectionNames();

bool getUniqueBasicBlockSectionNames();
Expand Down
3 changes: 3 additions & 0 deletions llvm/include/llvm/Target/TargetMachine.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ class TargetMachine {
/// Returns true if this target uses emulated TLS.
bool useEmulatedTLS() const;

/// Returns true if this target uses TLS Descriptors.
bool useTLSDESC() const;

/// Returns the TLS model which should be used for the given global variable.
TLSModel::Model getTLSModel(const GlobalValue *GV) const;

Expand Down
17 changes: 10 additions & 7 deletions llvm/include/llvm/Target/TargetOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,13 @@ namespace llvm {
IgnoreXCOFFVisibility(false), XCOFFTracebackTable(true),
UniqueSectionNames(true), UniqueBasicBlockSectionNames(false),
TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
EmulatedTLS(false), EnableIPRA(false), EmitStackSizeSection(false),
EnableMachineOutliner(false), EnableMachineFunctionSplitter(false),
SupportsDefaultOutlining(false), EmitAddrsig(false),
EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
EnableDebugEntryValues(false), ValueTrackingVariableLocations(false),
ForceDwarfFrameSection(false), XRayFunctionIndex(true),
DebugStrictDwarf(false), Hotpatch(false),
EmulatedTLS(false), EnableTLSDESC(false), EnableIPRA(false),
EmitStackSizeSection(false), EnableMachineOutliner(false),
EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
EmitAddrsig(false), EmitCallSiteInfo(false),
SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
XRayFunctionIndex(true), DebugStrictDwarf(false), Hotpatch(false),
PPCGenScalarMASSEntries(false), JMCInstrument(false),
EnableCFIFixup(false), MisExpect(false), XCOFFReadOnlyPointers(false),
FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
Expand Down Expand Up @@ -295,6 +295,9 @@ namespace llvm {
/// function in the runtime library..
unsigned EmulatedTLS : 1;

/// EnableTLSDESC - This flag enables TLS Descriptors.
unsigned EnableTLSDESC : 1;

/// This flag enables InterProcedural Register Allocation (IPRA).
unsigned EnableIPRA : 1;

Expand Down
7 changes: 7 additions & 0 deletions llvm/include/llvm/TargetParser/Triple.h
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,13 @@ class Triple {
isWindowsCygwinEnvironment() || isOHOSFamily();
}

/// Tests whether the target uses TLS Descriptor by default.
bool hasDefaultTLSDESC() const {
// TODO: Improve check for other platforms, like Android, and RISC-V
// Note: This is currently only used on RISC-V.
return isOSBinFormatELF() && isAArch64();
}

/// Tests whether the target uses -data-sections as default.
bool hasDefaultDataSections() const {
return isOSBinFormatXCOFF() || isWasm();
Expand Down
8 changes: 8 additions & 0 deletions llvm/lib/CodeGen/CommandFlags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ CGOPT(bool, XCOFFTracebackTable)
CGOPT(std::string, BBSections)
CGOPT(unsigned, TLSSize)
CGOPT_EXP(bool, EmulatedTLS)
CGOPT_EXP(bool, EnableTLSDESC)
CGOPT(bool, UniqueSectionNames)
CGOPT(bool, UniqueBasicBlockSectionNames)
CGOPT(EABI, EABIVersion)
Expand Down Expand Up @@ -404,6 +405,11 @@ codegen::RegisterCodeGenFlags::RegisterCodeGenFlags() {
"emulated-tls", cl::desc("Use emulated TLS model"), cl::init(false));
CGBINDOPT(EmulatedTLS);

static cl::opt<bool> EnableTLSDESC(
"enable-tlsdesc", cl::desc("Enable the use of TLS Descriptors"),
cl::init(false));
CGBINDOPT(EnableTLSDESC);

static cl::opt<bool> UniqueSectionNames(
"unique-section-names", cl::desc("Give unique names to every section"),
cl::init(true));
Expand Down Expand Up @@ -568,6 +574,8 @@ codegen::InitTargetOptionsFromCodeGenFlags(const Triple &TheTriple) {
Options.TLSSize = getTLSSize();
Options.EmulatedTLS =
getExplicitEmulatedTLS().value_or(TheTriple.hasDefaultEmulatedTLS());
Options.EnableTLSDESC =
getExplicitEnableTLSDESC().value_or(TheTriple.hasDefaultTLSDESC());
Options.ExceptionModel = getExceptionModel();
Options.EmitStackSizeSection = getEnableStackSizeSection();
Options.EnableMachineFunctionSplitter = getEnableMachineFunctionSplitter();
Expand Down
63 changes: 54 additions & 9 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,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);

Expand Down Expand Up @@ -549,6 +555,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 {
Expand Down Expand Up @@ -601,7 +617,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.

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.
Expand Down Expand Up @@ -854,7 +873,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>(); }
Expand Down Expand Up @@ -911,14 +932,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>(); }
Expand Down Expand Up @@ -1556,6 +1579,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");
Expand Down Expand Up @@ -3324,6 +3352,19 @@ bool RISCVAsmParser::checkPseudoAddTPRel(MCInst &Inst,
return false;
}

bool RISCVAsmParser::checkPseudoTLSDESCCall(MCInst &Inst,
topperc marked this conversation as resolved.
Show resolved Hide resolved
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(),
llvm::SMLoc());
Expand Down Expand Up @@ -3559,6 +3600,10 @@ bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
if (checkPseudoAddTPRel(Inst, Operands))
return true;
break;
case RISCV::PseudoTLSDESCCall:
if (checkPseudoTLSDESCCall(Inst, Operands))
return true;
break;
case RISCV::PseudoSEXT_B:
emitPseudoExtend(Inst, /*SignExtend=*/true, /*Width=*/8, IDLoc, Out);
return false;
Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ 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");
Expand Down Expand Up @@ -126,6 +132,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;
}

Expand Down Expand Up @@ -411,6 +418,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:
Expand All @@ -421,6 +429,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)) {
Expand Down Expand Up @@ -524,6 +533,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;
Expand Down
6 changes: 5 additions & 1 deletion llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,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

Expand Down
15 changes: 15 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ enum Fixups {
// Used to generate an R_RISCV_ALIGN relocation, which indicates the linker
// should fixup the alignment after linker relaxation.
fixup_riscv_align,
// Fixups indicating a TLS descriptor code sequence, corresponding to auipc,
// lw/ld, addi, and jalr, respectively.
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)?

fixup_riscv_tlsdesc_load_lo12,
fixup_riscv_tlsdesc_add_lo12,
fixup_riscv_tlsdesc_call,

// Used as a sentinel, must be the last
fixup_riscv_invalid,
Expand Down
Loading