[lld] Add infrastructure for handling RISCV vendor-specific relocations. #159987
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This requires adding an additional parameter to getRelExpr(), which unfortunately requires changes in non-RISCV targets. The alternative is to make the getRelExpr() stateful on RISCV, which seems at odds to the design intent. The design presented here keeps the state in the explicitly stateful RelocationScanner.
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-lld Author: Owen Anderson (resistor) ChangesThis requires adding an additional parameter to getRelExpr(), which Patch is 22.67 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159987.diff 18 Files Affected:
diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index 27e77e943c197..749c50fc57628 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -64,8 +64,8 @@ namespace {
class AArch64 : public TargetInfo {
public:
AArch64(Ctx &);
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
RelType getDynRel(RelType type) const override;
int64_t getImplicitAddend(const uint8_t *buf, RelType type) const override;
void writeGotPlt(uint8_t *buf, const Symbol &s) const override;
@@ -131,8 +131,8 @@ AArch64::AArch64(Ctx &ctx) : TargetInfo(ctx) {
needsThunks = true;
}
-RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr AArch64::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_AARCH64_ABS16:
case R_AARCH64_ABS32:
diff --git a/lld/ELF/Arch/AMDGPU.cpp b/lld/ELF/Arch/AMDGPU.cpp
index 52fc779855a36..10356370782a6 100644
--- a/lld/ELF/Arch/AMDGPU.cpp
+++ b/lld/ELF/Arch/AMDGPU.cpp
@@ -32,8 +32,8 @@ class AMDGPU final : public TargetInfo {
uint32_t calcEFlags() const override;
void relocate(uint8_t *loc, const Relocation &rel,
uint64_t val) const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
RelType getDynRel(RelType type) const override;
int64_t getImplicitAddend(const uint8_t *buf, RelType type) const override;
};
@@ -176,8 +176,8 @@ void AMDGPU::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
}
}
-RelExpr AMDGPU::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr AMDGPU::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_AMDGPU_ABS32:
case R_AMDGPU_ABS64:
diff --git a/lld/ELF/Arch/ARM.cpp b/lld/ELF/Arch/ARM.cpp
index 91a673f13d68e..35927f5ea7bad 100644
--- a/lld/ELF/Arch/ARM.cpp
+++ b/lld/ELF/Arch/ARM.cpp
@@ -29,8 +29,8 @@ class ARM final : public TargetInfo {
public:
ARM(Ctx &);
uint32_t calcEFlags() const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
RelType getDynRel(RelType type) const override;
int64_t getImplicitAddend(const uint8_t *buf, RelType type) const override;
void writeGotPlt(uint8_t *buf, const Symbol &s) const override;
@@ -99,8 +99,8 @@ uint32_t ARM::calcEFlags() const {
return EF_ARM_EABI_VER5 | abiFloatType | armBE8;
}
-RelExpr ARM::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr ARM::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_ARM_ABS32:
case R_ARM_MOVW_ABS_NC:
diff --git a/lld/ELF/Arch/AVR.cpp b/lld/ELF/Arch/AVR.cpp
index 5276ff9840591..492733a07434f 100644
--- a/lld/ELF/Arch/AVR.cpp
+++ b/lld/ELF/Arch/AVR.cpp
@@ -44,8 +44,8 @@ class AVR final : public TargetInfo {
public:
AVR(Ctx &ctx) : TargetInfo(ctx) { needsThunks = true; }
uint32_t calcEFlags() const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const override;
bool needsThunk(RelExpr expr, RelType type, const InputFile *file,
uint64_t branchAddr, const Symbol &s,
int64_t a) const override;
@@ -54,8 +54,8 @@ class AVR final : public TargetInfo {
};
} // namespace
-RelExpr AVR::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr AVR::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_AVR_6:
case R_AVR_6_ADIW:
diff --git a/lld/ELF/Arch/Hexagon.cpp b/lld/ELF/Arch/Hexagon.cpp
index 9b33e78731c97..41b084b157801 100644
--- a/lld/ELF/Arch/Hexagon.cpp
+++ b/lld/ELF/Arch/Hexagon.cpp
@@ -33,8 +33,8 @@ class Hexagon final : public TargetInfo {
public:
Hexagon(Ctx &);
uint32_t calcEFlags() const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
RelType getDynRel(RelType type) const override;
int64_t getImplicitAddend(const uint8_t *buf, RelType type) const override;
bool needsThunk(RelExpr expr, RelType type, const InputFile *file,
@@ -99,8 +99,8 @@ static uint32_t applyMask(uint32_t mask, uint32_t data) {
return result;
}
-RelExpr Hexagon::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr Hexagon::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_HEX_NONE:
return R_NONE;
diff --git a/lld/ELF/Arch/LoongArch.cpp b/lld/ELF/Arch/LoongArch.cpp
index db2c71c3b42b9..1cf6a68bd6e9b 100644
--- a/lld/ELF/Arch/LoongArch.cpp
+++ b/lld/ELF/Arch/LoongArch.cpp
@@ -33,8 +33,8 @@ class LoongArch final : public TargetInfo {
void writePlt(uint8_t *buf, const Symbol &sym,
uint64_t pltEntryAddr) const override;
RelType getDynRel(RelType type) const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
bool usesOnlyLowPageBits(RelType type) const override;
void relocate(uint8_t *loc, const Relocation &rel,
uint64_t val) const override;
@@ -416,7 +416,7 @@ RelType LoongArch::getDynRel(RelType type) const {
}
RelExpr LoongArch::getRelExpr(const RelType type, const Symbol &s,
- const uint8_t *loc) const {
+ const uint8_t *loc, StringRef rv_vendor) const {
switch (type) {
case R_LARCH_NONE:
case R_LARCH_MARK_LA:
diff --git a/lld/ELF/Arch/MSP430.cpp b/lld/ELF/Arch/MSP430.cpp
index 03d34436d2959..5580cea603a81 100644
--- a/lld/ELF/Arch/MSP430.cpp
+++ b/lld/ELF/Arch/MSP430.cpp
@@ -31,8 +31,8 @@ namespace {
class MSP430 final : public TargetInfo {
public:
MSP430(Ctx &);
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
void relocate(uint8_t *loc, const Relocation &rel,
uint64_t val) const override;
};
@@ -43,8 +43,8 @@ MSP430::MSP430(Ctx &ctx) : TargetInfo(ctx) {
trapInstr = {0x43, 0x43, 0x43, 0x43};
}
-RelExpr MSP430::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr MSP430::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_MSP430_10_PCREL:
case R_MSP430_16_PCREL:
diff --git a/lld/ELF/Arch/Mips.cpp b/lld/ELF/Arch/Mips.cpp
index f88b021c8ba39..a713980631187 100644
--- a/lld/ELF/Arch/Mips.cpp
+++ b/lld/ELF/Arch/Mips.cpp
@@ -23,8 +23,8 @@ template <class ELFT> class MIPS final : public TargetInfo {
public:
MIPS(Ctx &);
uint32_t calcEFlags() const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const override;
int64_t getImplicitAddend(const uint8_t *buf, RelType type) const override;
RelType getDynRel(RelType type) const override;
void writeGotPlt(uint8_t *buf, const Symbol &s) const override;
@@ -77,7 +77,7 @@ template <class ELFT> uint32_t MIPS<ELFT>::calcEFlags() const {
template <class ELFT>
RelExpr MIPS<ELFT>::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+ const uint8_t *loc, StringRef rv_vendor) const {
// See comment in the calculateMipsRelChain.
if (ELFT::Is64Bits || ctx.arg.mipsN32Abi)
type.v &= 0xff;
diff --git a/lld/ELF/Arch/PPC.cpp b/lld/ELF/Arch/PPC.cpp
index 60a0a38d5f23a..6342574bbd127 100644
--- a/lld/ELF/Arch/PPC.cpp
+++ b/lld/ELF/Arch/PPC.cpp
@@ -25,8 +25,8 @@ namespace {
class PPC final : public TargetInfo {
public:
PPC(Ctx &);
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
RelType getDynRel(RelType type) const override;
int64_t getImplicitAddend(const uint8_t *buf, RelType type) const override;
void writeGotHeader(uint8_t *buf) const override;
@@ -219,8 +219,8 @@ bool PPC::inBranchRange(RelType type, uint64_t src, uint64_t dst) const {
llvm_unreachable("unsupported relocation type used in branch");
}
-RelExpr PPC::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr PPC::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_PPC_NONE:
return R_NONE;
diff --git a/lld/ELF/Arch/PPC64.cpp b/lld/ELF/Arch/PPC64.cpp
index 3cd4a6294e2a8..b406122c9a46f 100644
--- a/lld/ELF/Arch/PPC64.cpp
+++ b/lld/ELF/Arch/PPC64.cpp
@@ -169,8 +169,8 @@ class PPC64 final : public TargetInfo {
PPC64(Ctx &);
int getTlsGdRelaxSkip(RelType type) const override;
uint32_t calcEFlags() const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
RelType getDynRel(RelType type) const override;
int64_t getImplicitAddend(const uint8_t *buf, RelType type) const override;
void writePltHeader(uint8_t *buf) const override;
@@ -990,8 +990,8 @@ void PPC64::relaxTlsIeToLe(uint8_t *loc, const Relocation &rel,
}
}
-RelExpr PPC64::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr PPC64::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_PPC64_NONE:
return R_NONE;
diff --git a/lld/ELF/Arch/RISCV.cpp b/lld/ELF/Arch/RISCV.cpp
index 7f2bfefa5578a..be42d77800c80 100644
--- a/lld/ELF/Arch/RISCV.cpp
+++ b/lld/ELF/Arch/RISCV.cpp
@@ -39,8 +39,8 @@ class RISCV final : public TargetInfo {
void writePlt(uint8_t *buf, const Symbol &sym,
uint64_t pltEntryAddr) const override;
RelType getDynRel(RelType type) const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
void relocate(uint8_t *loc, const Relocation &rel,
uint64_t val) const override;
void relocateAlloc(InputSectionBase &sec, uint8_t *buf) const override;
@@ -56,6 +56,10 @@ class RISCV final : public TargetInfo {
bool synthesizeAlign(uint64_t &dot, InputSection *sec) override;
void finalizeRelax(int passes) const override;
+ // For vendor-specific relocations.
+ void relocateVendor(uint8_t *loc, const Relocation &rel, uint64_t val,
+ StringRef vendor = "") const;
+
// The following two variables are used by synthesized ALIGN relocations.
InputSection *baseSec = nullptr;
// r_offset and r_addend pairs.
@@ -275,7 +279,15 @@ RelType RISCV::getDynRel(RelType type) const {
}
RelExpr RISCV::getRelExpr(const RelType type, const Symbol &s,
- const uint8_t *loc) const {
+ const uint8_t *loc, StringRef rv_vendor) const {
+ if (!rv_vendor.empty()) {
+ // TODO: Dispatch to vendor-specific relocation handling.
+ Err(ctx) << getErrorLoc(ctx, loc) << "unknown vendor-specific relocation ("
+ << type.v << ") in vendor namespace \"" << rv_vendor
+ << "\" against symbol " << &s;
+ return R_NONE;
+ }
+
switch (type) {
case R_RISCV_NONE:
return R_NONE;
@@ -338,6 +350,8 @@ RelExpr RISCV::getRelExpr(const RelType type, const Symbol &s,
case R_RISCV_SET_ULEB128:
case R_RISCV_SUB_ULEB128:
return RE_RISCV_LEB128;
+ case R_RISCV_VENDOR:
+ return R_NONE;
default:
Err(ctx) << getErrorLoc(ctx, loc) << "unknown relocation (" << type.v
<< ") against symbol " << &s;
@@ -555,6 +569,15 @@ void RISCV::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
}
}
+void RISCV::relocateVendor(uint8_t *loc, const Relocation &rel, uint64_t val,
+ StringRef vendor) const {
+ llvm_unreachable("unknown vendor relocation");
+ Err(ctx) << getErrorLoc(ctx, loc) << "unknown relocation type "
+ << llvm::utostr(rel.type.v) << " in vendor namespace \"" << vendor
+ << "\"";
+ return;
+}
+
static bool relaxable(ArrayRef<Relocation> relocs, size_t i) {
return i + 1 != relocs.size() && relocs[i + 1].type == R_RISCV_RELAX;
}
@@ -617,6 +640,26 @@ void RISCV::relocateAlloc(InputSectionBase &sec, uint8_t *buf) const {
uint8_t *loc = buf + rel.offset;
uint64_t val = sec.getRelocTargetVA(ctx, rel, secAddr + rel.offset);
+ if (rel.type == R_RISCV_VENDOR) {
+ // Vendor-specific relocations are indicated by a pair of a R_RISCV_VENDOR
+ // relocation followed by relocation in the vendor's private namespace.
+ // The vendor name is identified by the symbol name referenced by the
+ // R_RISCV_VENDOR relocation.
+ StringRef vendor = rel.sym->getName();
+
+ // Consume the second relocation as well.
+ assert(i != size - 1 &&
+ "R_RISCV_VENDOR relocation cannot be the final relocation!");
+ i += 1;
+
+ const Relocation &rel2 = relocs[i];
+ uint8_t *loc2 = buf + rel2.offset;
+ uint64_t val2 = sec.getRelocTargetVA(ctx, rel2, secAddr + rel2.offset);
+
+ relocateVendor(loc2, rel2, val2, vendor);
+ continue;
+ }
+
switch (rel.expr) {
case R_RELAX_HINT:
continue;
diff --git a/lld/ELF/Arch/SPARCV9.cpp b/lld/ELF/Arch/SPARCV9.cpp
index 86668c50d3c78..c34f433d303ba 100644
--- a/lld/ELF/Arch/SPARCV9.cpp
+++ b/lld/ELF/Arch/SPARCV9.cpp
@@ -21,8 +21,8 @@ namespace {
class SPARCV9 final : public TargetInfo {
public:
SPARCV9(Ctx &);
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
void writePlt(uint8_t *buf, const Symbol &sym,
uint64_t pltEntryAddr) const override;
void relocate(uint8_t *loc, const Relocation &rel,
@@ -44,8 +44,8 @@ SPARCV9::SPARCV9(Ctx &ctx) : TargetInfo(ctx) {
defaultImageBase = 0x100000;
}
-RelExpr SPARCV9::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr SPARCV9::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_SPARC_32:
case R_SPARC_UA32:
diff --git a/lld/ELF/Arch/SystemZ.cpp b/lld/ELF/Arch/SystemZ.cpp
index a9125806c0952..8b3ecb0fb8f98 100644
--- a/lld/ELF/Arch/SystemZ.cpp
+++ b/lld/ELF/Arch/SystemZ.cpp
@@ -24,8 +24,8 @@ class SystemZ : public TargetInfo {
public:
SystemZ(Ctx &);
int getTlsGdRelaxSkip(RelType type) const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
RelType getDynRel(RelType type) const override;
void writeGotHeader(uint8_t *buf) const override;
void writeGotPlt(uint8_t *buf, const Symbol &s) const override;
@@ -79,8 +79,8 @@ SystemZ::SystemZ(Ctx &ctx) : TargetInfo(ctx) {
defaultImageBase = 0x1000000;
}
-RelExpr SystemZ::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr SystemZ::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_390_NONE:
return R_NONE;
diff --git a/lld/ELF/Arch/X86.cpp b/lld/ELF/Arch/X86.cpp
index c1980d6e0538f..d75c089d0b767 100644
--- a/lld/ELF/Arch/X86.cpp
+++ b/lld/ELF/Arch/X86.cpp
@@ -23,8 +23,8 @@ class X86 : public TargetInfo {
public:
X86(Ctx &);
int getTlsGdRelaxSkip(RelType type) const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
int64_t getImplicitAddend(const uint8_t *buf, RelType type) const override;
void writeGotPltHeader(uint8_t *buf) const override;
RelType getDynRel(RelType type) const override;
@@ -74,8 +74,8 @@ int X86::getTlsGdRelaxSkip(RelType type) const {
return type == R_386_TLS_GOTDESC || type == R_386_TLS_DESC_CALL ? 1 : 2;
}
-RelExpr X86::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr X86::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_386_8:
case R_386_16:
diff --git a/lld/ELF/Arch/X86_64.cpp b/lld/ELF/Arch/X86_64.cpp
index 488f4803b2cb4..a6eb7bae4696a 100644
--- a/lld/ELF/Arch/X86_64.cpp
+++ b/lld/ELF/Arch/X86_64.cpp
@@ -28,8 +28,8 @@ class X86_64 : public TargetInfo {
public:
X86_64(Ctx &);
int getTlsGdRelaxSkip(RelType type) const override;
- RelExpr getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const override;
+ RelExpr getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor = "") const override;
RelType getDynRel(RelType type) const override;
void writeGotPltHeader(uint8_t *buf) const override;
void writeGotPlt(uint8_t *buf, const Symbol &s) const override;
@@ -361,8 +361,8 @@ bool X86_64::relaxOnce(int pass) const {
return changed;
}
-RelExpr X86_64::getRelExpr(RelType type, const Symbol &s,
- const uint8_t *loc) const {
+RelExpr X86_64::getRelExpr(RelType type, const Symbol &s, const uint8_t *loc,
+ StringRef rv_vendor) const {
switch (type) {
case R_X86_64_8:
case R_X86_64_16:
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 6f55bac2ecf16..f58985115d826 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -457,6 +457,7 @@ class RelocationScanner {
Ctx &ctx;
InputSectionBase *sec;
OffsetGetter getter;
+ StringRef rv_vendor = "";
// End of relocations, used by Mips/PPC64.
const void *end = nullptr;
@@ -1510,13 +1511,20 @@ void RelocationScanner::scanOne(typename Relocs<RelTy>::const_iterator &i) {
++i;
}
}
+
+ // Stash the RISCV vendor namespace for the subsequent relocation.
+ if (LLVM_UNLIKELY(ctx.arg.emachine == EM_RISCV) && type...
[truncated]
|
|
It sounds like you want a separate method that's only implemented on RISC-V that takes the vendor as an argument, perhaps? This definitely isn't the right interface. |
|
Some ideas, probably incoherent:
I agree with Jessica that I don't think a RISC-V specific string on the public interface is great, especially as it's basically seeping into only one part of the interface for relocations, rather than being something more universal. |
I don't think we need to remap it to an enum at this stage, because the target is going to provide a |
I'm fine with doing this or @lenary 's suggestion on type erasure. I suppose another option would be to make Do folks feel like a type-erased |
|
I went ahead and implemented @jrtc27 's recommendation, but I'm happy to switch it to the other version if desired. |
lenary
reviewed
Sep 23, 2025
lenary
approved these changes
Sep 23, 2025
jrtc27
reviewed
Sep 23, 2025
jrtc27
reviewed
Sep 23, 2025
| @@ -0,0 +1,28 @@ | |||
| # RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcibi,+xandesperf %s -filetype=obj -o %t.o | |||
There was a problem hiding this comment.
I’m not convinced this test makes much sense, in theory we should support all these relocations and then you wouldn’t be able to test this. Surely a better test would be to actually implement a vendor relocation?
There was a problem hiding this comment.
The intention here is to test the infrastructure for parsing the RV vendor-specific relocations. Note that it tests that this parsing is done correctly by testing for the vendor names as part of the error message.
It's fine if this test gets whittled down as the relocations here gain implementations. The test doesn't have to live forever, if all of them eventually get implemented it can be removed entirely. Would it be helpful if I added a comment to that effect in the test file?
I had originally intended to test this with a fake vendor-specific relocation, but I think that would require a YAML test, and I had issues with lit refusing to execute them.
There was a problem hiding this comment.
What about something like
.local INVALID_VENDOR
.set INVALID_VENDOR, 0
.reloc 1f, R_RISCV_VENDOR, INVALID_VENDOR
.reloc 1f, R_RISCV_CUSTOM0
1:
There was a problem hiding this comment.
Thank you for the suggestion! With a little fiddling that worked out nicely. PTAL.
MaskRay
reviewed
Sep 24, 2025
MaskRay
reviewed
Sep 24, 2025
| } | ||
|
|
||
| // Stash the RISCV vendor namespace for the subsequent relocation. | ||
| if (LLVM_UNLIKELY(ctx.arg.emachine == EM_RISCV && type == R_RISCV_VENDOR)) { |
There was a problem hiding this comment.
I am not sure the PR is useful. It adds some stuff that is not testable.
There was a problem hiding this comment.
Every new line, except the error path for non-RV targets, is executed when running the included test, and the correctness of the parsing is verified by checking the vendor name in the error message.
MaskRay
requested changes
Sep 24, 2025
There was a problem hiding this comment.
Requesting changes in case this gets landed. I'll try moving RelocationScanner to a .h file and instantiating it in all targets. I have a long-standing WIP on this https://github.com/MaskRay/llvm-project/tree/wip-lld-rel
|
@MaskRay Any update on that? If it's not imminent, would it possible to move ahead with this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This requires adding an additional parameter to getRelExpr(), which
unfortunately requires changes in non-RISCV targets. The alternative
is to make the getRelExpr() stateful on RISCV, which seems at odds to
the design intent. The design presented here keeps the state in the
explicitly stateful RelocationScanner.