Conversation
|
@llvm/pr-subscribers-backend-aarch64 Author: Shivam Gupta (xgupta) ChangesThis patch adds support for the %dtprel relocation specifier in the AArch64 assembler. This specifier is used to generate the R_AARCH64_TLS_DTPREL64 relocation, which is used in .debug_info sections to describe the location of thread-local variables. Prerequisite for #146572 (cherry picked from commit 60c1020) Full diff: https://github.com/llvm/llvm-project/pull/191021.diff 5 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 5ef3e2e50ec86..2221d5790a249 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -182,6 +182,7 @@ class AArch64AsmParser : public MCTargetAsmParser {
bool showMatchError(SMLoc Loc, unsigned ErrCode, uint64_t ErrorInfo,
OperandVector &Operands);
+ bool parseExprWithSpecifier(const MCExpr *&Res, SMLoc &E);
bool parseDataExpr(const MCExpr *&Res) override;
bool parseAuthExpr(const MCExpr *&Res, SMLoc &EndLoc);
@@ -4669,6 +4670,7 @@ bool AArch64AsmParser::parseSymbolicImmVal(const MCExpr *&ImmVal) {
.Case("prel_g1_nc", AArch64::S_PREL_G1_NC)
.Case("prel_g0", AArch64::S_PREL_G0)
.Case("prel_g0_nc", AArch64::S_PREL_G0_NC)
+ .Case("dtprel", AArch64::S_DTPREL)
.Case("dtprel_g2", AArch64::S_DTPREL_G2)
.Case("dtprel_g1", AArch64::S_DTPREL_G1)
.Case("dtprel_g1_nc", AArch64::S_DTPREL_G1_NC)
@@ -8463,9 +8465,33 @@ bool AArch64AsmParser::parseDirectiveAeabiAArch64Attr(SMLoc L) {
return false;
}
+bool AArch64AsmParser::parseExprWithSpecifier(const MCExpr *&Res, SMLoc &E) {
+ SMLoc Loc = getLoc();
+ if (getLexer().getKind() != AsmToken::Identifier)
+ return TokError("expected '%' relocation specifier");
+ StringRef Identifier = getParser().getTok().getIdentifier();
+ auto Spec = AArch64::parsePercentSpecifierName(Identifier);
+ if (!Spec)
+ return TokError("invalid relocation specifier");
+
+ getParser().Lex(); // Eat the identifier
+ if (parseToken(AsmToken::LParen, "expected '('"))
+ return true;
+
+ const MCExpr *SubExpr;
+ if (getParser().parseParenExpression(SubExpr, E))
+ return true;
+
+ Res = MCSpecifierExpr::create(SubExpr, Spec, getContext(), Loc);
+ return false;
+}
+
bool AArch64AsmParser::parseDataExpr(const MCExpr *&Res) {
SMLoc EndLoc;
+ if (parseOptionalToken(AsmToken::Percent))
+ return parseExprWithSpecifier(Res, EndLoc);
+
if (getParser().parseExpression(Res))
return true;
MCAsmParser &Parser = getParser();
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
index 892b8da37eb69..36c87be9acb4c 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
@@ -232,6 +232,8 @@ unsigned AArch64ELFObjectWriter::getRelocType(const MCFixup &Fixup,
}
if (RefKind == AArch64::S_AUTH || RefKind == AArch64::S_AUTHADDR)
return ELF::R_AARCH64_AUTH_ABS64;
+ if (RefKind == AArch64::S_DTPREL)
+ return ELF::R_AARCH64_TLS_DTPREL64;
if (RefKind == AArch64::S_FUNCINIT)
return ELF::R_AARCH64_FUNCINIT64;
return ELF::R_AARCH64_ABS64;
@@ -448,6 +450,8 @@ unsigned AArch64ELFObjectWriter::getRelocType(const MCFixup &Fixup,
return R_CLS(MOVW_PREL_G0);
if (RefKind == AArch64::S_PREL_G0_NC)
return R_CLS(MOVW_PREL_G0_NC);
+ if (RefKind == AArch64::S_DTPREL)
+ return ELF::R_AARCH64_TLS_DTPREL64;
if (RefKind == AArch64::S_DTPREL_G2)
return ELF::R_AARCH64_TLSLD_MOVW_DTPREL_G2;
if (RefKind == AArch64::S_DTPREL_G1)
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp
index bc090c6157eef..c2a1bdc541dfa 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp
@@ -114,12 +114,20 @@ StringRef AArch64::getSpecifierName(AArch64::Specifier S) {
case AArch64::S_GOT_AUTH: return ":got_auth:";
case AArch64::S_GOT_AUTH_PAGE: return ":got_auth:";
case AArch64::S_GOT_AUTH_LO12: return ":got_auth_lo12:";
+
+ case AArch64::S_DTPREL: return "%dtprel";
default:
llvm_unreachable("Invalid relocation specifier");
}
// clang-format on
}
+AArch64::Specifier AArch64::parsePercentSpecifierName(StringRef name) {
+ return StringSwitch<AArch64::Specifier>(name)
+ .Case("dtprel", AArch64::S_DTPREL)
+ .Default(0);
+}
+
static bool evaluate(const MCSpecifierExpr &Expr, MCValue &Res,
const MCAssembler *Asm) {
if (!Expr.getSubExpr()->evaluateAsRelocatable(Res, Asm))
@@ -233,8 +241,13 @@ void AArch64MCAsmInfoELF::printSpecifierExpr(
raw_ostream &OS, const MCSpecifierExpr &Expr) const {
if (auto *AE = dyn_cast<AArch64AuthMCExpr>(&Expr))
return AE->print(OS, this);
- OS << AArch64::getSpecifierName(Expr.getSpecifier());
+ auto Str = AArch64::getSpecifierName(Expr.getSpecifier());
+ OS << Str;
+ if (!Str.empty() && Str[0] == '%')
+ OS << '(';
printExpr(OS, *Expr.getSubExpr());
+ if (!Str.empty() && Str[0] == '%')
+ OS << ')';
}
bool AArch64MCAsmInfoELF::evaluateAsRelocatableImpl(
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h
index f2acff54f1665..5bb8ff8599b2a 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h
@@ -184,6 +184,8 @@ enum {
/// (e.g. ":got:", ":lo12:").
StringRef getSpecifierName(Specifier S);
+Specifier parsePercentSpecifierName(StringRef);
+
inline Specifier getSymbolLoc(Specifier S) {
return static_cast<Specifier>(S & AArch64::S_SymLocBits);
}
diff --git a/llvm/test/MC/AArch64/tls-dtprel64.s b/llvm/test/MC/AArch64/tls-dtprel64.s
new file mode 100644
index 0000000000000..7104681f56e2b
--- /dev/null
+++ b/llvm/test/MC/AArch64/tls-dtprel64.s
@@ -0,0 +1,27 @@
+// RUN: llvm-mc -triple=aarch64 -show-encoding < %s | FileCheck %s
+// RUN: llvm-mc -filetype=obj -triple=aarch64 %s | llvm-readobj -r - | FileCheck --check-prefix=CHECK-ELF %s
+
+# CHECK: .xword %dtprel(var)
+# CHECK: .xword %dtprel(var+1)
+# CHECK: .xword %dtprel(.tdata)
+# CHECK: .xword %dtprel(.tdata+1)
+
+# CHECK-ELF: Relocations [
+# CHECK-ELF: Section (5) .rela.debug_info {
+# CHECK-ELF: 0x0 R_AARCH64_TLS_DTPREL64 var 0x0
+# CHECK-ELF: 0x8 R_AARCH64_TLS_DTPREL64 var 0x1
+# CHECK-ELF: 0x10 R_AARCH64_TLS_DTPREL64 .tdata 0x0
+# CHECK-ELF: 0x18 R_AARCH64_TLS_DTPREL64 .tdata 0x1
+# CHECK-ELF: }
+
+.section .tdata,"awT",@progbits
+.skip 8
+.globl var
+var:
+ .word 0
+
+.section .debug_info,"",@progbits
+ .xword %dtprel(var)
+ .xword %dtprel(var+1)
+ .xword %dtprel(.tdata)
+ .xword %dtprel(.tdata+1)
|
…6599) This patch adds support for the %dtprel relocation specifier in the AArch64 assembler. This specifier is used to generate the R_AARCH64_TLS_DTPREL64 relocation, which is used in .debug_info sections to describe the location of thread-local variables. Prerequisite for llvm#146572 (cherry picked from commit 60c1020)
c35c98e to
b132e3b
Compare
github-project-automation
Bot
moved this from Needs Triage
to Needs Merge
in LLVM Release Status
|
@c-rhodes (assuming release manager can only merge) can you please merge this PR. I have three more dependent PR to back port. |
|
correct me if I'm wrong but this is adding support for a new feature? We're quite late in the release cycle already with 22.1.4 being the next release, completion of features is typically expected before rc1: https://llvm.org/docs/HowToReleaseLLVM.html#release-patch-rules |
|
Thanks for the link. I am not sure, this is called a feature, it can be call a critical bug fix to fix the debugging of TLS variables on aarch64. @avikivity might better advocate for back port, can we wait for his response.
|
thanks for the context, I missed that. What is "aarch64 port" referring to? |
|
aarch64 port is aarch64 backend/architecture/platform. Developers writing softwares like Scylladb which uses heavy TLS variables in the program. And they want to develop/debug such programs on aarch64 platform. Without this fix they can not debug the program, making it difficult to develop their application. |
ok thanks. And what are the other 3 PRs that need backporting? |
|
@c-rhodes Let me know if you have more questions, we have maintainer approval to back port. |
|
as per https://llvm.org/docs/HowToReleaseLLVM.html#release-patch-rules ultimately it's the release maintainers responsibility to backport. While I understand you may think the backport is warranted, requests like this put us in a difficult position as they necessitate we skirt the official rules which blurs the lines for ourselves as well other contributors who may feel like a feature they want backporting is also warranted. I'm afraid I'm going to have to reject. |
|
No Issues from my side @c-rhodes, I trust your decision to reject to safe guard release branch! However others are feel free if they can provide better reasoning to back port. |
4 participants
This patch adds support for the %dtprel relocation specifier in the AArch64 assembler. This specifier is used to generate the R_AARCH64_TLS_DTPREL64 relocation, which is used in .debug_info sections to describe the location of thread-local variables.
Prerequisite for #146572
(cherry picked from commit 60c1020)