[lld][WebAssembly] Move input vectors from symtab to ctx. NFC #78640
Conversation
This file contains 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
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-wasm Author: Sam Clegg (sbc100) ChangesAlso convert from std::vector to SmallVector. This matches the ELF linker where these were moved into the ctx object in 9a57216 and converted to SmallVector in ba948c5. Full diff: https://github.com/llvm/llvm-project/pull/78640.diff 8 Files Affected:
diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index cc8a1dcb69afe06..dc7ca265e9a2cb1 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -23,6 +23,13 @@ enum class CodeGenOptLevel;
namespace lld::wasm {
class InputFile;
+class StubFile;
+class ObjFile;
+class SharedFile;
+class BitcodeFile;
+class InputTable;
+class InputGlobal;
+class InputFunction;
class Symbol;
// For --unresolved-symbols.
@@ -108,6 +115,14 @@ extern Configuration *config;
// The Ctx object hold all other (non-configuration) global state.
struct Ctx {
+ llvm::SmallVector<ObjFile *, 0> objectFiles;
+ llvm::SmallVector<StubFile *, 0> stubFiles;
+ llvm::SmallVector<SharedFile *, 0> sharedFiles;
+ llvm::SmallVector<BitcodeFile *, 0> bitcodeFiles;
+ llvm::SmallVector<InputFunction *, 0> syntheticFunctions;
+ llvm::SmallVector<InputGlobal *, 0> syntheticGlobals;
+ llvm::SmallVector<InputTable *, 0> syntheticTables;
+
// True if we are creating position-independent code.
bool isPic;
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 05a96860966ea3d..4a4f9a96227946d 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -899,7 +899,7 @@ static void createOptionalSymbols() {
static void processStubLibrariesPreLTO() {
log("-- processStubLibrariesPreLTO");
- for (auto &stub_file : symtab->stubFiles) {
+ for (auto &stub_file : ctx.stubFiles) {
LLVM_DEBUG(llvm::dbgs()
<< "processing stub file: " << stub_file->getName() << "\n");
for (auto [name, deps]: stub_file->symbolDependencies) {
@@ -924,7 +924,7 @@ static void processStubLibraries() {
bool depsAdded = false;
do {
depsAdded = false;
- for (auto &stub_file : symtab->stubFiles) {
+ for (auto &stub_file : ctx.stubFiles) {
LLVM_DEBUG(llvm::dbgs()
<< "processing stub file: " << stub_file->getName() << "\n");
for (auto [name, deps]: stub_file->symbolDependencies) {
@@ -1075,7 +1075,7 @@ static void wrapSymbols(ArrayRef<WrappedSymbol> wrapped) {
}
// Update pointers in input files.
- parallelForEach(symtab->objectFiles, [&](InputFile *file) {
+ parallelForEach(ctx.objectFiles, [&](InputFile *file) {
MutableArrayRef<Symbol *> syms = file->getMutableSymbols();
for (size_t i = 0, e = syms.size(); i != e; ++i)
if (Symbol *s = map.lookup(syms[i]))
@@ -1091,7 +1091,7 @@ static void splitSections() {
// splitIntoPieces needs to be called on each MergeInputChunk
// before calling finalizeContents().
LLVM_DEBUG(llvm::dbgs() << "splitSections\n");
- parallelForEach(symtab->objectFiles, [](ObjFile *file) {
+ parallelForEach(ctx.objectFiles, [](ObjFile *file) {
for (InputChunk *seg : file->segments) {
if (auto *s = dyn_cast<MergeInputChunk>(seg))
s->splitIntoPieces();
@@ -1263,7 +1263,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
// We only need to add libcall symbols to the link before LTO if the symbol's
// definition is in bitcode. Any other required libcall symbols will be added
// to the link after LTO when we add the LTO object file to the link.
- if (!symtab->bitcodeFiles.empty())
+ if (!ctx.bitcodeFiles.empty())
for (auto *s : lto::LTO::getRuntimeLibcallSymbols())
handleLibcall(s);
if (errorCount())
diff --git a/lld/wasm/MapFile.cpp b/lld/wasm/MapFile.cpp
index 288c189854dbd12..c96b64cb648381c 100644
--- a/lld/wasm/MapFile.cpp
+++ b/lld/wasm/MapFile.cpp
@@ -51,7 +51,7 @@ static void writeHeader(raw_ostream &os, int64_t vma, uint64_t lma,
// Returns a list of all symbols that we want to print out.
static std::vector<Symbol *> getSymbols() {
std::vector<Symbol *> v;
- for (InputFile *file : symtab->objectFiles)
+ for (InputFile *file : ctx.objectFiles)
for (Symbol *b : file->getSymbols())
if (auto *dr = dyn_cast<Symbol>(b))
if ((!isa<SectionSymbol>(dr)) && dr->isLive() &&
diff --git a/lld/wasm/MarkLive.cpp b/lld/wasm/MarkLive.cpp
index 773af2f7e2360ec..b8ab7741ff1cb31 100644
--- a/lld/wasm/MarkLive.cpp
+++ b/lld/wasm/MarkLive.cpp
@@ -97,7 +97,7 @@ void MarkLive::run() {
enqueue(WasmSym::callDtors);
// Enqueue constructors in objects explicitly live from the command-line.
- for (const ObjFile *obj : symtab->objectFiles)
+ for (const ObjFile *obj : ctx.objectFiles)
if (obj->isLive())
enqueueInitFunctions(obj);
@@ -151,7 +151,7 @@ void markLive() {
// Report garbage-collected sections.
if (config->printGcSections) {
- for (const ObjFile *obj : symtab->objectFiles) {
+ for (const ObjFile *obj : ctx.objectFiles) {
for (InputChunk *c : obj->functions)
if (!c->live)
message("removing unused section " + toString(c));
@@ -168,13 +168,13 @@ void markLive() {
if (!t->live)
message("removing unused section " + toString(t));
}
- for (InputChunk *c : symtab->syntheticFunctions)
+ for (InputChunk *c : ctx.syntheticFunctions)
if (!c->live)
message("removing unused section " + toString(c));
- for (InputGlobal *g : symtab->syntheticGlobals)
+ for (InputGlobal *g : ctx.syntheticGlobals)
if (!g->live)
message("removing unused section " + toString(g));
- for (InputTable *t : symtab->syntheticTables)
+ for (InputTable *t : ctx.syntheticTables)
if (!t->live)
message("removing unused section " + toString(t));
}
@@ -192,7 +192,7 @@ bool MarkLive::isCallCtorsLive() {
// If there are any init functions, mark `__wasm_call_ctors` live so that
// it can call them.
- for (const ObjFile *file : symtab->objectFiles) {
+ for (const ObjFile *file : ctx.objectFiles) {
const WasmLinkingData &l = file->getWasmObj()->linkingData();
for (const WasmInitFunc &f : l.InitFunctions) {
auto *sym = file->getFunctionSymbol(f.Symbol);
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index 9dd599e18975e31..9988490e14b0bc1 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -34,14 +34,14 @@ void SymbolTable::addFile(InputFile *file, StringRef symName) {
// .so file
if (auto *f = dyn_cast<SharedFile>(file)) {
- sharedFiles.push_back(f);
+ ctx.sharedFiles.push_back(f);
return;
}
// stub file
if (auto *f = dyn_cast<StubFile>(file)) {
f->parse();
- stubFiles.push_back(f);
+ ctx.stubFiles.push_back(f);
return;
}
@@ -52,7 +52,7 @@ void SymbolTable::addFile(InputFile *file, StringRef symName) {
if (auto *f = dyn_cast<BitcodeFile>(file)) {
// This order, first adding to `bitcodeFiles` and then parsing is necessary.
// See https://github.com/llvm/llvm-project/pull/73095
- bitcodeFiles.push_back(f);
+ ctx.bitcodeFiles.push_back(f);
f->parse(symName);
return;
}
@@ -60,7 +60,7 @@ void SymbolTable::addFile(InputFile *file, StringRef symName) {
// Regular object file
auto *f = cast<ObjFile>(file);
f->parse(false);
- objectFiles.push_back(f);
+ ctx.objectFiles.push_back(f);
}
// This function is where all the optimizations of link-time
@@ -74,18 +74,18 @@ void SymbolTable::compileBitcodeFiles() {
// Prevent further LTO objects being included
BitcodeFile::doneLTO = true;
- if (bitcodeFiles.empty())
+ if (ctx.bitcodeFiles.empty())
return;
// Compile bitcode files and replace bitcode symbols.
lto.reset(new BitcodeCompiler);
- for (BitcodeFile *f : bitcodeFiles)
+ for (BitcodeFile *f : ctx.bitcodeFiles)
lto->add(*f);
for (StringRef filename : lto->compile()) {
auto *obj = make<ObjFile>(MemoryBufferRef(filename, "lto.tmp"), "");
obj->parse(true);
- objectFiles.push_back(obj);
+ ctx.objectFiles.push_back(obj);
}
}
@@ -218,7 +218,7 @@ DefinedFunction *SymbolTable::addSyntheticFunction(StringRef name,
InputFunction *function) {
LLVM_DEBUG(dbgs() << "addSyntheticFunction: " << name << "\n");
assert(!find(name));
- syntheticFunctions.emplace_back(function);
+ ctx.syntheticFunctions.emplace_back(function);
return replaceSymbol<DefinedFunction>(insertName(name).first, name,
flags, nullptr, function);
}
@@ -255,7 +255,7 @@ DefinedGlobal *SymbolTable::addSyntheticGlobal(StringRef name, uint32_t flags,
LLVM_DEBUG(dbgs() << "addSyntheticGlobal: " << name << " -> " << global
<< "\n");
assert(!find(name));
- syntheticGlobals.emplace_back(global);
+ ctx.syntheticGlobals.emplace_back(global);
return replaceSymbol<DefinedGlobal>(insertName(name).first, name, flags,
nullptr, global);
}
@@ -267,7 +267,7 @@ DefinedGlobal *SymbolTable::addOptionalGlobalSymbol(StringRef name,
return nullptr;
LLVM_DEBUG(dbgs() << "addOptionalGlobalSymbol: " << name << " -> " << global
<< "\n");
- syntheticGlobals.emplace_back(global);
+ ctx.syntheticGlobals.emplace_back(global);
return replaceSymbol<DefinedGlobal>(s, name, WASM_SYMBOL_VISIBILITY_HIDDEN,
nullptr, global);
}
@@ -280,7 +280,7 @@ DefinedTable *SymbolTable::addSyntheticTable(StringRef name, uint32_t flags,
assert(!s || s->isUndefined());
if (!s)
s = insertName(name).first;
- syntheticTables.emplace_back(table);
+ ctx.syntheticTables.emplace_back(table);
return replaceSymbol<DefinedTable>(s, name, flags, nullptr, table);
}
@@ -855,7 +855,7 @@ InputFunction *SymbolTable::replaceWithUnreachable(Symbol *sym,
StringRef debugName) {
auto *func = make<SyntheticFunction>(sig, sym->getName(), debugName);
func->setBody(unreachableFn);
- syntheticFunctions.emplace_back(func);
+ ctx.syntheticFunctions.emplace_back(func);
// Mark new symbols as local. For relocatable output we don't want them
// to be exported outside the object file.
replaceSymbol<DefinedFunction>(sym, debugName, WASM_SYMBOL_BINDING_LOCAL,
diff --git a/lld/wasm/SymbolTable.h b/lld/wasm/SymbolTable.h
index 59eda1c0b6740c2..c5518ee23da26da 100644
--- a/lld/wasm/SymbolTable.h
+++ b/lld/wasm/SymbolTable.h
@@ -101,14 +101,6 @@ class SymbolTable {
void handleWeakUndefines();
DefinedFunction *createUndefinedStub(const WasmSignature &sig);
- std::vector<ObjFile *> objectFiles;
- std::vector<StubFile *> stubFiles;
- std::vector<SharedFile *> sharedFiles;
- std::vector<BitcodeFile *> bitcodeFiles;
- std::vector<InputFunction *> syntheticFunctions;
- std::vector<InputGlobal *> syntheticGlobals;
- std::vector<InputTable *> syntheticTables;
-
private:
std::pair<Symbol *, bool> insert(StringRef name, const InputFile *file);
std::pair<Symbol *, bool> insertName(StringRef name);
diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index f95174fe6f2695c..72e255951608e1b 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -57,7 +57,7 @@ class SubSection {
bool DylinkSection::isNeeded() const {
return ctx.isPic ||
config->unresolvedSymbols == UnresolvedPolicy::ImportDynamic ||
- !symtab->sharedFiles.empty();
+ !ctx.sharedFiles.empty();
}
void DylinkSection::writeBody() {
@@ -72,10 +72,10 @@ void DylinkSection::writeBody() {
sub.writeTo(os);
}
- if (symtab->sharedFiles.size()) {
+ if (ctx.sharedFiles.size()) {
SubSection sub(WASM_DYLINK_NEEDED);
- writeUleb128(sub.os, symtab->sharedFiles.size(), "Needed");
- for (auto *so : symtab->sharedFiles)
+ writeUleb128(sub.os, ctx.sharedFiles.size(), "Needed");
+ for (auto *so : ctx.sharedFiles)
writeStr(sub.os, llvm::sys::path::filename(so->getName()), "so name");
sub.writeTo(os);
}
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 7c19a1baf4c0451..d1a06c9ac9c2aee 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -133,7 +133,7 @@ class Writer {
void Writer::calculateCustomSections() {
log("calculateCustomSections");
bool stripDebug = config->stripDebug || config->stripAll;
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
for (InputChunk *section : file->customSections) {
// Exclude COMDAT sections that are not selected for inclusion
if (section->discarded)
@@ -207,7 +207,7 @@ void Writer::createRelocSections() {
}
void Writer::populateProducers() {
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
const WasmProducerInfo &info = file->getWasmObj()->getProducerInfo();
out.producersSec->addInfo(info);
}
@@ -591,7 +591,7 @@ void Writer::populateTargetFeatures() {
}
// Find the sets of used, required, and disallowed features
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
StringRef fileName(file->getName());
for (auto &feature : file->getWasmObj()->getTargetFeatures()) {
switch (feature.Prefix) {
@@ -654,7 +654,7 @@ void Writer::populateTargetFeatures() {
}
// Validate the required and disallowed constraints for each file
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
StringRef fileName(file->getName());
SmallSet<std::string, 8> objectFeatures;
for (const auto &feature : file->getWasmObj()->getTargetFeatures()) {
@@ -832,7 +832,7 @@ void Writer::populateSymtab() {
if (sym->isUsedInRegularObj && sym->isLive())
out.linkingSec->addToSymtab(sym);
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
LLVM_DEBUG(dbgs() << "Local symtab entries: " << file->getName() << "\n");
for (Symbol *sym : file->getSymbols())
if (sym->isLocal() && !isa<SectionSymbol>(sym) && sym->isLive())
@@ -848,7 +848,7 @@ void Writer::calculateTypes() {
// 4. The signatures of all imported tags
// 5. The signatures of all defined tags
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
ArrayRef<WasmSignature> types = file->getWasmObj()->types();
for (uint32_t i = 0; i < types.size(); i++)
if (file->typeIsUsed[i])
@@ -939,7 +939,7 @@ static void finalizeIndirectFunctionTable() {
}
static void scanRelocations() {
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
LLVM_DEBUG(dbgs() << "scanRelocations: " << file->getName() << "\n");
for (InputChunk *chunk : file->functions)
scanRelocations(chunk);
@@ -955,37 +955,37 @@ void Writer::assignIndexes() {
// global are effected by the number of imports.
out.importSec->seal();
- for (InputFunction *func : symtab->syntheticFunctions)
+ for (InputFunction *func : ctx.syntheticFunctions)
out.functionSec->addFunction(func);
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
LLVM_DEBUG(dbgs() << "Functions: " << file->getName() << "\n");
for (InputFunction *func : file->functions)
out.functionSec->addFunction(func);
}
- for (InputGlobal *global : symtab->syntheticGlobals)
+ for (InputGlobal *global : ctx.syntheticGlobals)
out.globalSec->addGlobal(global);
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
LLVM_DEBUG(dbgs() << "Globals: " << file->getName() << "\n");
for (InputGlobal *global : file->globals)
out.globalSec->addGlobal(global);
}
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
LLVM_DEBUG(dbgs() << "Tags: " << file->getName() << "\n");
for (InputTag *tag : file->tags)
out.tagSec->addTag(tag);
}
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
LLVM_DEBUG(dbgs() << "Tables: " << file->getName() << "\n");
for (InputTable *table : file->tables)
out.tableSec->addTable(table);
}
- for (InputTable *table : symtab->syntheticTables)
+ for (InputTable *table : ctx.syntheticTables)
out.tableSec->addTable(table);
out.globalSec->assignIndexes();
@@ -1022,7 +1022,7 @@ OutputSegment *Writer::createOutputSegment(StringRef name) {
}
void Writer::createOutputSegments() {
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
for (InputChunk *segment : file->segments) {
if (!segment->live)
continue;
@@ -1639,7 +1639,7 @@ void Writer::calculateInitFunctions() {
if (!config->relocatable && !WasmSym::callCtors->isLive())
return;
- for (ObjFile *file : symtab->objectFiles) {
+ for (ObjFile *file : ctx.objectFiles) {
const WasmLinkingData &l = file->getWasmObj()->linkingData();
for (const WasmInitFunc &f : l.InitFunctions) {
FunctionSymbol *sym = file->getFunctionSymbol(f.Symbol);
|
MaskRay
approved these changes
Jan 18, 2024
|
This change has apparently injected crashes in Halide when we use LLD in library form for WebAssembly... SISGSEV in calls to MarkLive::enqueueInitFunctions: |
|
...hmm, do we ever clear those newly added vectors in |
|
Yep, that's it, if I clear those reset the Ctx to initial state at the top of each call to link() it heals it. I'll prep a PR if no one is already doing so. |
|
(The ELF linker might need a similar fix?) |
|
The ELF linker already handles clearing all the stuff. I think the fix is straight forward, but writing tests is the tricky part.. I'll see how this is tested in ELF and try to replicate. |
|
Ah, OK: I'll skip the PR if you are already on it. |
|
(But may I suggest that a quick fix-or-rollback would be appropriate if adding tests will take a while, because in this state, using wasm-lld as a library is extremely broken.) |
sbc100
added a commit
to sbc100/llvm-project
that referenced
this pull request
Jan 19, 2024
This mirrors who the ELF linker works. I wasn't able to find anywhere where this is currently tested. Followup to llvm#78640, which triggered a regression.
sbc100
added a commit
that referenced
this pull request
Jan 19, 2024
This mirrors how the ELF linker works. I wasn't able to find anywhere where this is currently tested. Followup to #78640, which triggered a regression.
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.
Also convert from std::vector to SmallVector.
This matches the ELF linker where these were moved into the ctx object in 9a57216 and converted to SmallVector in ba948c5.