[ELF] Refactor RelocScan::scan to be target-specific #163138
Merged
+446
−354
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
MaskRay
added
the
skip-precommit-approval
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-backend-mips Author: Fangrui Song (MaskRay) Changes
This refactoring prepares the codebase for better target-specific optimizations and easier addition of target-specific behavior. Patch is 37.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163138.diff 7 Files Affected:
diff --git a/lld/ELF/Arch/Mips.cpp b/lld/ELF/Arch/Mips.cpp
index f88b021c8ba39..87fdf69890197 100644
--- a/lld/ELF/Arch/Mips.cpp
+++ b/lld/ELF/Arch/Mips.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "InputFiles.h"
+#include "RelocScan.h"
#include "Symbols.h"
#include "SyntheticSections.h"
#include "Target.h"
@@ -31,6 +32,9 @@ template <class ELFT> class MIPS final : public TargetInfo {
void writePltHeader(uint8_t *buf) const override;
void writePlt(uint8_t *buf, const Symbol &sym,
uint64_t pltEntryAddr) const override;
+ template <class RelTy>
+ void scanSectionImpl(InputSectionBase &, Relocs<RelTy>);
+ void scanSection(InputSectionBase &) override;
bool needsThunk(RelExpr expr, RelType type, const InputFile *file,
uint64_t branchAddr, const Symbol &s,
int64_t a) const override;
@@ -570,6 +574,107 @@ static uint64_t fixupCrossModeJump(Ctx &ctx, uint8_t *loc, RelType type,
return val;
}
+template <class RelTy>
+static RelType getMipsN32RelType(Ctx &ctx, RelTy *&rel, RelTy *end) {
+ uint32_t type = 0;
+ uint64_t offset = rel->r_offset;
+ int n = 0;
+ while (rel != end && rel->r_offset == offset)
+ type |= (rel++)->getType(ctx.arg.isMips64EL) << (8 * n++);
+ return type;
+}
+
+static RelType getMipsPairType(RelType type, bool isLocal) {
+ switch (type) {
+ case R_MIPS_HI16:
+ return R_MIPS_LO16;
+ case R_MIPS_GOT16:
+ // In case of global symbol, the R_MIPS_GOT16 relocation does not
+ // have a pair. Each global symbol has a unique entry in the GOT
+ // and a corresponding instruction with help of the R_MIPS_GOT16
+ // relocation loads an address of the symbol. In case of local
+ // symbol, the R_MIPS_GOT16 relocation creates a GOT entry to hold
+ // the high 16 bits of the symbol's value. A paired R_MIPS_LO16
+ // relocations handle low 16 bits of the address. That allows
+ // to allocate only one GOT entry for every 64 KiB of local data.
+ return isLocal ? R_MIPS_LO16 : R_MIPS_NONE;
+ case R_MICROMIPS_GOT16:
+ return isLocal ? R_MICROMIPS_LO16 : R_MIPS_NONE;
+ case R_MIPS_PCHI16:
+ return R_MIPS_PCLO16;
+ case R_MICROMIPS_HI16:
+ return R_MICROMIPS_LO16;
+ default:
+ return R_MIPS_NONE;
+ }
+}
+
+template <class ELFT>
+template <class RelTy>
+void MIPS<ELFT>::scanSectionImpl(InputSectionBase &sec, Relocs<RelTy> rels) {
+ RelocScan rs(ctx, &sec);
+ sec.relocations.reserve(rels.size());
+ RelType type;
+ for (auto it = rels.begin(); it != rels.end();) {
+ auto it0 = it;
+ const RelTy &rel = *it;
+ if constexpr (ELFT::Is64Bits) {
+ type = it->getType(ctx.arg.isMips64EL);
+ ++it;
+ } else {
+ if (ctx.arg.mipsN32Abi) {
+ type = getMipsN32RelType(ctx, it, rels.end());
+ } else {
+ type = it->getType(ctx.arg.isMips64EL);
+ ++it;
+ }
+ }
+
+ uint32_t symIndex = rel.getSymbol(ctx.arg.isMips64EL);
+ Symbol &sym = sec.getFile<ELFT>()->getSymbol(symIndex);
+ RelExpr expr =
+ ctx.target->getRelExpr(type, sym, sec.content().data() + rel.r_offset);
+
+ auto addend = rs.getAddend<ELFT>(rel, type);
+ if (expr == RE_MIPS_GOTREL && sym.isLocal()) {
+ addend += sec.getFile<ELFT>()->mipsGp0;
+ } else if (!RelTy::HasAddend) {
+ // MIPS has an odd notion of "paired" relocations to calculate addends.
+ // For example, if a relocation is of R_MIPS_HI16, there must be a
+ // R_MIPS_LO16 relocation after that, and an addend is calculated using
+ // the two relocations.
+ RelType pairTy = getMipsPairType(type, sym.isLocal());
+ if (pairTy != R_MIPS_NONE) {
+ const uint8_t *buf = sec.content().data();
+ // To make things worse, paired relocations might not be contiguous in
+ // the relocation table, so we need to do linear search. *sigh*
+ bool found = false;
+ for (auto *ri = &rel; ri != rels.end(); ++ri) {
+ if (ri->getType(ctx.arg.isMips64EL) == pairTy &&
+ ri->getSymbol(ctx.arg.isMips64EL) == symIndex) {
+ addend += ctx.target->getImplicitAddend(buf + ri->r_offset, pairTy);
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ Warn(ctx) << "can't find matching " << pairTy << " relocation for "
+ << type;
+ }
+ }
+ rs.scan<ELFT, RelTy>(it0, type, addend);
+ }
+}
+
+template <class ELFT> void MIPS<ELFT>::scanSection(InputSectionBase &sec) {
+ auto relocs = sec.template relsOrRelas<ELFT>();
+ if (relocs.areRelocsRel())
+ scanSectionImpl(sec, relocs.rels);
+ else
+ scanSectionImpl(sec, relocs.relas);
+}
+
template <class ELFT>
void MIPS<ELFT>::relocate(uint8_t *loc, const Relocation &rel,
uint64_t val) const {
diff --git a/lld/ELF/Arch/PPC64.cpp b/lld/ELF/Arch/PPC64.cpp
index 550c091624bb5..660baa6101e83 100644
--- a/lld/ELF/Arch/PPC64.cpp
+++ b/lld/ELF/Arch/PPC64.cpp
@@ -8,6 +8,7 @@
#include "InputFiles.h"
#include "OutputSections.h"
+#include "RelocScan.h"
#include "SymbolTable.h"
#include "Symbols.h"
#include "SyntheticSections.h"
@@ -178,6 +179,10 @@ class PPC64 final : public TargetInfo {
uint64_t pltEntryAddr) const override;
void writeIplt(uint8_t *buf, const Symbol &sym,
uint64_t pltEntryAddr) const override;
+ template <class ELFT, class RelTy>
+ void scanSectionImpl(InputSectionBase &, Relocs<RelTy>);
+ template <class ELFT> void scanSection1(InputSectionBase &);
+ void scanSection(InputSectionBase &) override;
void relocate(uint8_t *loc, const Relocation &rel,
uint64_t val) const override;
void writeGotHeader(uint8_t *buf) const override;
@@ -1257,6 +1262,132 @@ static bool isTocOptType(RelType type) {
}
}
+// R_PPC64_TLSGD/R_PPC64_TLSLD is required to mark `bl __tls_get_addr` for
+// General Dynamic/Local Dynamic code sequences. If a GD/LD GOT relocation is
+// found but no R_PPC64_TLSGD/R_PPC64_TLSLD is seen, we assume that the
+// instructions are generated by very old IBM XL compilers. Work around the
+// issue by disabling GD/LD to IE/LE relaxation.
+template <class RelTy>
+static void checkPPC64TLSRelax(InputSectionBase &sec, Relocs<RelTy> rels) {
+ // Skip if sec is synthetic (sec.file is null) or if sec has been marked.
+ if (!sec.file || sec.file->ppc64DisableTLSRelax)
+ return;
+ bool hasGDLD = false;
+ for (const RelTy &rel : rels) {
+ RelType type = rel.getType(false);
+ switch (type) {
+ case R_PPC64_TLSGD:
+ case R_PPC64_TLSLD:
+ return; // Found a marker
+ case R_PPC64_GOT_TLSGD16:
+ case R_PPC64_GOT_TLSGD16_HA:
+ case R_PPC64_GOT_TLSGD16_HI:
+ case R_PPC64_GOT_TLSGD16_LO:
+ case R_PPC64_GOT_TLSLD16:
+ case R_PPC64_GOT_TLSLD16_HA:
+ case R_PPC64_GOT_TLSLD16_HI:
+ case R_PPC64_GOT_TLSLD16_LO:
+ hasGDLD = true;
+ break;
+ }
+ }
+ if (hasGDLD) {
+ sec.file->ppc64DisableTLSRelax = true;
+ Warn(sec.file->ctx)
+ << sec.file
+ << ": disable TLS relaxation due to R_PPC64_GOT_TLS* relocations "
+ "without "
+ "R_PPC64_TLSGD/R_PPC64_TLSLD relocations";
+ }
+}
+
+template <class ELFT, class RelTy>
+void PPC64::scanSectionImpl(InputSectionBase &sec, Relocs<RelTy> rels) {
+ RelocScan rs(ctx, &sec);
+ sec.relocations.reserve(rels.size());
+ checkPPC64TLSRelax<RelTy>(sec, rels);
+ for (auto it = rels.begin(); it != rels.end(); ++it) {
+ const RelTy &rel = *it;
+ uint64_t offset = rel.r_offset;
+ uint32_t symIndex = rel.getSymbol(false);
+ Symbol &sym = sec.getFile<ELFT>()->getSymbol(symIndex);
+ RelType type = rel.getType(false);
+ RelExpr expr =
+ ctx.target->getRelExpr(type, sym, sec.content().data() + offset);
+ if (expr == R_NONE)
+ continue;
+ if (sym.isUndefined() && symIndex != 0 &&
+ maybeReportUndefined(ctx, cast<Undefined>(sym), sec, offset))
+ continue;
+
+ auto addend = getAddend<ELFT>(rel);
+ if (ctx.arg.isPic && type == R_PPC64_TOC)
+ addend += getPPC64TocBase(ctx);
+
+ // We can separate the small code model relocations into 2 categories:
+ // 1) Those that access the compiler generated .toc sections.
+ // 2) Those that access the linker allocated got entries.
+ // lld allocates got entries to symbols on demand. Since we don't try to
+ // sort the got entries in any way, we don't have to track which objects
+ // have got-based small code model relocs. The .toc sections get placed
+ // after the end of the linker allocated .got section and we do sort those
+ // so sections addressed with small code model relocations come first.
+ if (type == R_PPC64_TOC16 || type == R_PPC64_TOC16_DS)
+ sec.file->ppc64SmallCodeModelTocRelocs = true;
+
+ // Record the TOC entry (.toc + addend) as not relaxable. See the comment in
+ // PPC64::relocateAlloc().
+ if (type == R_PPC64_TOC16_LO && sym.isSection() && isa<Defined>(sym) &&
+ cast<Defined>(sym).section->name == ".toc")
+ ctx.ppc64noTocRelax.insert({&sym, addend});
+
+ if ((type == R_PPC64_TLSGD && expr == R_TLSDESC_CALL) ||
+ (type == R_PPC64_TLSLD && expr == R_TLSLD_HINT)) {
+ auto it1 = it;
+ ++it1;
+ if (it1 == rels.end()) {
+ auto diag = Err(ctx);
+ diag << "R_PPC64_TLSGD/R_PPC64_TLSLD may not be the last "
+ "relocation";
+ printLocation(diag, sec, sym, offset);
+ continue;
+ }
+
+ // Offset the 4-byte aligned R_PPC64_TLSGD by one byte in the NOTOC
+ // case, so we can discern it later from the toc-case.
+ if (it1->getType(/*isMips64EL=*/false) == R_PPC64_REL24_NOTOC)
+ ++offset;
+ }
+
+ if (oneof<R_GOTREL, RE_PPC64_TOCBASE, RE_PPC64_RELAX_TOC>(expr))
+ ctx.in.got->hasGotOffRel.store(true, std::memory_order_relaxed);
+
+ if (sym.isTls()) {
+ if (unsigned processed =
+ rs.handleTlsRelocation(expr, type, offset, sym, addend)) {
+ it += processed - 1;
+ continue;
+ }
+ }
+ rs.process(expr, type, offset, sym, addend);
+ }
+}
+
+template <class ELFT> void PPC64::scanSection1(InputSectionBase &sec) {
+ auto relocs = sec.template relsOrRelas<ELFT>();
+ if (relocs.areRelocsCrel())
+ scanSectionImpl<ELFT>(sec, relocs.crels);
+ else
+ scanSectionImpl<ELFT>(sec, relocs.relas);
+}
+
+void PPC64::scanSection(InputSectionBase &sec) {
+ if (ctx.arg.isLE)
+ scanSection1<ELF64LE>(sec);
+ else
+ scanSection1<ELF64BE>(sec);
+}
+
void PPC64::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
RelType type = rel.type;
bool shouldTocOptimize = isTocOptType(type);
diff --git a/lld/ELF/RelocScan.h b/lld/ELF/RelocScan.h
new file mode 100644
index 0000000000000..94c8b4b3eb201
--- /dev/null
+++ b/lld/ELF/RelocScan.h
@@ -0,0 +1,128 @@
+//===------------------------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLD_ELF_RELOCSCAN_H
+#define LLD_ELF_RELOCSCAN_H
+
+#include "Config.h"
+#include "InputFiles.h"
+#include "InputSection.h"
+#include "Relocations.h"
+#include "SyntheticSections.h"
+#include "Target.h"
+
+using namespace llvm;
+using namespace llvm::ELF;
+using namespace llvm::object;
+
+namespace lld::elf {
+
+// Build a bitmask with one bit set for each 64 subset of RelExpr.
+inline constexpr uint64_t buildMask() { return 0; }
+
+template <typename... Tails>
+inline constexpr uint64_t buildMask(int head, Tails... tails) {
+ return (0 <= head && head < 64 ? uint64_t(1) << head : 0) |
+ buildMask(tails...);
+}
+
+// Return true if `Expr` is one of `Exprs`.
+// There are more than 64 but less than 128 RelExprs, so we divide the set of
+// exprs into [0, 64) and [64, 128) and represent each range as a constant
+// 64-bit mask. Then we decide which mask to test depending on the value of
+// expr and use a simple shift and bitwise-and to test for membership.
+template <RelExpr... Exprs> bool oneof(RelExpr expr) {
+ assert(0 <= expr && (int)expr < 128 &&
+ "RelExpr is too large for 128-bit mask!");
+
+ if (expr >= 64)
+ return (uint64_t(1) << (expr - 64)) & buildMask((Exprs - 64)...);
+ return (uint64_t(1) << expr) & buildMask(Exprs...);
+}
+
+// This class encapsulates states needed to scan relocations for one
+// InputSectionBase.
+class RelocScan {
+public:
+ Ctx &ctx;
+ InputSectionBase *sec;
+
+ RelocScan(Ctx &ctx, InputSectionBase *sec = nullptr) : ctx(ctx), sec(sec) {}
+ template <class ELFT, class RelTy>
+ void scan(typename Relocs<RelTy>::const_iterator &i, RelType type,
+ int64_t addend);
+ void scanEhSection(EhInputSection &s);
+
+ template <class ELFT, class RelTy>
+ int64_t getAddend(const RelTy &r, RelType type);
+ bool isStaticLinkTimeConstant(RelExpr e, RelType type, const Symbol &sym,
+ uint64_t relOff) const;
+ void process(RelExpr expr, RelType type, uint64_t offset, Symbol &sym,
+ int64_t addend) const;
+ unsigned handleTlsRelocation(RelExpr expr, RelType type, uint64_t offset,
+ Symbol &sym, int64_t addend);
+};
+
+template <class ELFT, class RelTy>
+int64_t RelocScan::getAddend(const RelTy &r, RelType type) {
+ return RelTy::HasAddend ? elf::getAddend<ELFT>(r)
+ : ctx.target->getImplicitAddend(
+ sec->content().data() + r.r_offset, type);
+}
+
+template <class ELFT, class RelTy>
+void RelocScan::scan(typename Relocs<RelTy>::const_iterator &it, RelType type,
+ int64_t addend) {
+ const RelTy &rel = *it;
+ uint32_t symIndex = rel.getSymbol(ctx.arg.isMips64EL);
+ Symbol &sym = sec->getFile<ELFT>()->getSymbol(symIndex);
+ uint64_t offset = rel.r_offset;
+
+ RelExpr expr =
+ ctx.target->getRelExpr(type, sym, sec->content().data() + offset);
+
+ // Ignore R_*_NONE and other marker relocations.
+ if (expr == R_NONE)
+ return;
+
+ // Error if the target symbol is undefined. Symbol index 0 may be used by
+ // marker relocations, e.g. R_*_NONE and R_ARM_V4BX. Don't error on them.
+ if (sym.isUndefined() && symIndex != 0 &&
+ maybeReportUndefined(ctx, cast<Undefined>(sym), *sec, offset))
+ return;
+
+ // If the relocation does not emit a GOT or GOTPLT entry but its computation
+ // uses their addresses, we need GOT or GOTPLT to be created.
+ //
+ // The 5 types that relative GOTPLT are all x86 and x86-64 specific.
+ if (oneof<R_GOTPLTONLY_PC, R_GOTPLTREL, R_GOTPLT, R_PLT_GOTPLT,
+ R_TLSDESC_GOTPLT, R_TLSGD_GOTPLT>(expr)) {
+ ctx.in.gotPlt->hasGotPltOffRel.store(true, std::memory_order_relaxed);
+ } else if (oneof<R_GOTONLY_PC, R_GOTREL, RE_PPC32_PLTREL>(expr)) {
+ ctx.in.got->hasGotOffRel.store(true, std::memory_order_relaxed);
+ }
+
+ // Process TLS relocations, including TLS optimizations. Note that
+ // R_TPREL and R_TPREL_NEG relocations are resolved in processAux.
+ //
+ // Some RISCV TLSDESC relocations reference a local NOTYPE symbol,
+ // but we need to process them in handleTlsRelocation.
+ if (sym.isTls() || oneof<R_TLSDESC_PC, R_TLSDESC_CALL>(expr)) {
+ if (unsigned processed =
+ handleTlsRelocation(expr, type, offset, sym, addend)) {
+ it += processed - 1;
+ return;
+ }
+ }
+
+ process(expr, type, offset, sym, addend);
+}
+
+} // namespace lld::elf
+
+#endif
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 84b9b5e983662..3a845b613cb2a 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -30,6 +30,7 @@
#include "InputFiles.h"
#include "LinkerScript.h"
#include "OutputSections.h"
+#include "RelocScan.h"
#include "SymbolTable.h"
#include "Symbols.h"
#include "SyntheticSections.h"
@@ -58,8 +59,8 @@ static void printDefinedLocation(ELFSyncStream &s, const Symbol &sym) {
// >>> defined in /home/alice/src/foo.o
// >>> referenced by bar.c:12 (/home/alice/src/bar.c:12)
// >>> /home/alice/src/bar.o:(.text+0x1)
-static void printLocation(ELFSyncStream &s, InputSectionBase &sec,
- const Symbol &sym, uint64_t off) {
+void elf::printLocation(ELFSyncStream &s, InputSectionBase &sec,
+ const Symbol &sym, uint64_t off) {
printDefinedLocation(s, sym);
s << "\n>>> referenced by ";
auto tell = s.tell();
@@ -111,54 +112,6 @@ void elf::reportRangeError(Ctx &ctx, uint8_t *loc, int64_t v, int n,
}
}
-// Build a bitmask with one bit set for each 64 subset of RelExpr.
-static constexpr uint64_t buildMask() { return 0; }
-
-template <typename... Tails>
-static constexpr uint64_t buildMask(int head, Tails... tails) {
- return (0 <= head && head < 64 ? uint64_t(1) << head : 0) |
- buildMask(tails...);
-}
-
-// Return true if `Expr` is one of `Exprs`.
-// There are more than 64 but less than 128 RelExprs, so we divide the set of
-// exprs into [0, 64) and [64, 128) and represent each range as a constant
-// 64-bit mask. Then we decide which mask to test depending on the value of
-// expr and use a simple shift and bitwise-and to test for membership.
-template <RelExpr... Exprs> static bool oneof(RelExpr expr) {
- assert(0 <= expr && (int)expr < 128 &&
- "RelExpr is too large for 128-bit mask!");
-
- if (expr >= 64)
- return (uint64_t(1) << (expr - 64)) & buildMask((Exprs - 64)...);
- return (uint64_t(1) << expr) & buildMask(Exprs...);
-}
-
-static RelType getMipsPairType(RelType type, bool isLocal) {
- switch (type) {
- case R_MIPS_HI16:
- return R_MIPS_LO16;
- case R_MIPS_GOT16:
- // In case of global symbol, the R_MIPS_GOT16 relocation does not
- // have a pair. Each global symbol has a unique entry in the GOT
- // and a corresponding instruction with help of the R_MIPS_GOT16
- // relocation loads an address of the symbol. In case of local
- // symbol, the R_MIPS_GOT16 relocation creates a GOT entry to hold
- // the high 16 bits of the symbol's value. A paired R_MIPS_LO16
- // relocations handle low 16 bits of the address. That allows
- // to allocate only one GOT entry for every 64 KiB of local data.
- return isLocal ? R_MIPS_LO16 : R_MIPS_NONE;
- case R_MICROMIPS_GOT16:
- return isLocal ? R_MICROMIPS_LO16 : R_MIPS_NONE;
- case R_MIPS_PCHI16:
- return R_MIPS_PCLO16;
- case R_MICROMIPS_HI16:
- return R_MICROMIPS_LO16;
- default:
- return R_MIPS_NONE;
- }
-}
-
// True if non-preemptable symbol always has the same value regardless of where
// the DSO is loaded.
bool elf::isAbsolute(const Symbol &sym) {
@@ -424,73 +377,8 @@ class OffsetGetter {
ArrayRef<EhSectionPiece> cies, fdes;
ArrayRef<EhSectionPiece>::iterator i, j;
};
-
-// This class encapsulates states needed to scan relocations for one
-// InputSectionBase.
-class RelocationScanner {
-public:
- RelocationScanner(Ctx &ctx) : ctx(ctx) {}
- template <class ELFT> void scanSection(InputSectionBase &s);
- template <class ELFT> void scanEhSection(EhInputSection &s);
-
-private:
- Ctx &ctx;
- InputSectionBase *sec;
-
- // End of relocations, used by Mips/PPC64.
- const void *end = nullptr;
-
- template <class RelTy> RelType getMipsN32RelType(RelTy *&rel) const;
- template <class ELFT, class RelTy>
- int64_t computeMipsAddend(const RelTy &rel, RelExpr expr, bool isLocal) const;
- bool isStaticLinkTimeConstant(RelExpr e, RelType type, const Symbol &sym,
- uint64_t relOff) const;
- void process(RelExpr expr, RelType type, uint64_t offset, Symbol &sym,
- int64_t addend) const;
- unsigned handleTlsRelocation(RelExpr expr, RelType type, uint64_t offset,
- Symbol &sym, int64_t addend);
-
- template <class ELFT, class RelTy>
- void scan(typename Relocs<RelTy>::const_iterator &i);
- template <class ELFT, class RelTy> void scanSectionImpl(Relocs<RelTy> rels);
-};
} // namespace
-// MIPS has an odd notion of "paired" relocations to calculate addends.
-// For example, if a relocation is of R_MIPS_HI16, there must be a
-// R_MIPS_LO16 relocation after that, and an addend is calc...
[truncated]
|
smithp35
reviewed
Oct 13, 2025
| virtual bool inBranchRange(RelType type, uint64_t src, | ||
| uint64_t dst) const; | ||
|
|
||
| template <class ELFT, class RelTy> |
There was a problem hiding this comment.
Can we leave a comment about what each of these functions does and the relationship between them?
It looks like scanSection and scanSection1 do not need to be (re)implemented if invokeELFT can be used from the generic implementation.
It looks like the most important one is scanSectionImpl and that overrides the generic TargetInfo's implementation. May be worth describing what the requirements are or implementation guidance for a target implementation.
There was a problem hiding this comment.
Thanks for the suggestion. Added comments.
There was a problem hiding this comment.
Did these comments not get added? It's very hard for me to tell what each of these is supposed to mean, or what I should be overriding to reimplement #159987 on top of this.
There was a problem hiding this comment.
The latest revision has these comments. MIPS and PPC64 demonstrate how scanSection is overridden. We probably also need comments in RelocScan.h.
// Function for scanning relocation. Typically overridden by targets that
// require special type or addend adjustment.
virtual void scanSection(InputSectionBase &); 2 refs | 2 derived
// Called by scanSection as a default implementation for specific ELF
// relocation types.
template <class ELFT> void scanSection1(InputSectionBase &); 0 ref | 2 refs
template <class ELFT, class RelTy> 0 ref | 1 ref
void scanSectionImpl(InputSectionBase &, Relocs<RelTy>); 3 refs
In the future, we probably should make RelocScan fully target-specific, eliminating ctx.arg.emachine overhead. Newer linkers mold and wild use arch as a template parameter, making relocation scanner substantially faster at the cost of binary size.
There was a problem hiding this comment.
In the future, we probably should make RelocScan fully target-specific, eliminating
ctx.arg.emachineoverhead. Newer linkers mold and wild use arch as a template parameter, making relocation scanner substantially faster at the cost of binary size.
If we made this change sooner, we could avoid having to copy-paste the boilerplate into each target by just making all of the existing emachine checks if constexpr
| // branch-to-branch optimization. | ||
| if (is_contained({EM_RISCV, EM_LOONGARCH}, ctx.arg.emachine) || | ||
| (ctx.arg.emachine == EM_PPC64 && sec->name == ".toc") || | ||
| (ctx.arg.emachine == EM_PPC64 && sec.name == ".toc") || |
There was a problem hiding this comment.
This could be made into a target specific query, for example
if (needsSortedRelocations(ctx, sec))
...
Or possibly even a preScan and postScan so that we could have
preScan(ctx, rels);
for (auto it = rels.begin(); it != rels.end(); ++it) {
auto type = it->getType(false);
rs.scan<ELFT, RelTy>(it, type, rs.getAddend<ELFT>(*it, type));
}
postScan(ctx, sec);
There was a problem hiding this comment.
Thanks for the suggestion.
Due to --branch-to-branch needsSortedRelocations will have to check branchToBranch internally.
if (needsSortedRelocations(ctx, sec))
...
For now I am inclined to keep this unchanged.
rels = sortRels(rels, storage);
can be removed once we make some adjustment to Arch/SystemZ.cpp, eliminating the need for preScan.
- Extract RelocScan to RelocScan.h. The file includes Target.h, and cannot be merged with Relocations.h - Add MIPS and PPC64 specific relocation scanners, removing runtime checks for other targets. This refactoring prepares the codebase for better target-specific optimizations and easier addition of target-specific behavior.
smithp35
approved these changes
Oct 13, 2025
akadutta
pushed a commit
to akadutta/llvm-project
that referenced
this pull request
Oct 14, 2025
- Extract RelocScan to RelocScan.h. The file includes Target.h, and cannot be merged with Relocations.h - Add MIPS and PPC64 specific relocation scanners, removing runtime checks for other targets. This refactoring prepares the codebase for better target-specific optimizations and easier addition of target-specific behavior.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 refactoring prepares the codebase for better target-specific optimizations and easier addition of target-specific behavior.