-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
|
||
|
@@ -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 { | ||
|
@@ -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 || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of these change to accept more values for There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
@@ -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>(); } | ||
|
@@ -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>(); } | ||
|
@@ -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"); | ||
|
@@ -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()); | ||
|
@@ -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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder whether we can just use There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've 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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've 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 noR_RISCV_RELAX
:)With my experience on #79099 , I think two
R_RISCV_RELAX
are sufficient. I probably need to follow up on the spec.