[lld] bd48127 - [WebAssembly] Use llvm::Optional to store optional symbol attributes. NFC.
Sam Clegg via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 19 17:25:45 PST 2020
Author: Sam Clegg
Date: 2020-02-19T17:25:33-08:00
New Revision: bd4812776bc73ca27bf6c68629a20f03268cdd6e
URL: https://github.com/llvm/llvm-project/commit/bd4812776bc73ca27bf6c68629a20f03268cdd6e
DIFF: https://github.com/llvm/llvm-project/commit/bd4812776bc73ca27bf6c68629a20f03268cdd6e.diff
LOG: [WebAssembly] Use llvm::Optional to store optional symbol attributes. NFC.
The changes the in-memory representation of wasm symbols such that their
optional ImportName and ImportModule use llvm::Optional.
ImportName is set whenever WASM_SYMBOL_EXPLICIT_NAME flag is set.
ImportModule (for imports) is currently always set since it defaults to
"env".
In the future we can possibly extent to binary format distingish
import which have explit module names.
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D74109
Added:
Modified:
lld/wasm/Driver.cpp
lld/wasm/InputFiles.cpp
lld/wasm/LTO.cpp
lld/wasm/SymbolTable.cpp
lld/wasm/SymbolTable.h
lld/wasm/Symbols.h
lld/wasm/SyntheticSections.cpp
llvm/include/llvm/BinaryFormat/Wasm.h
llvm/include/llvm/MC/MCSymbolWasm.h
llvm/lib/Object/WasmObjectFile.cpp
llvm/test/MC/WebAssembly/debug-info.ll
llvm/test/tools/llvm-readobj/wasm/symbols.test
llvm/test/tools/llvm-readobj/wasm/wasm-imports.test
llvm/tools/llvm-readobj/WasmDumper.cpp
Removed:
################################################################################
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 1a1d4782797c..f9ea83add7c6 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -452,7 +452,7 @@ static void handleLibcall(StringRef name) {
static UndefinedGlobal *
createUndefinedGlobal(StringRef name, llvm::wasm::WasmGlobalType *type) {
auto *sym = cast<UndefinedGlobal>(symtab->addUndefinedGlobal(
- name, name, defaultModule, WASM_SYMBOL_UNDEFINED, nullptr, type));
+ name, None, None, WASM_SYMBOL_UNDEFINED, nullptr, type));
config->allowUndefinedSymbols.insert(sym->getName());
sym->isUsedInRegularObj = true;
return sym;
@@ -590,7 +590,7 @@ struct WrappedSymbol {
};
static Symbol *addUndefined(StringRef name) {
- return symtab->addUndefinedFunction(name, "", "", WASM_SYMBOL_UNDEFINED,
+ return symtab->addUndefinedFunction(name, None, None, WASM_SYMBOL_UNDEFINED,
nullptr, nullptr, false);
}
diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index d71a2b4f083d..2aab5bf10e61 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -526,7 +526,7 @@ static Symbol *createBitcodeSymbol(const std::vector<bool> &keptComdats,
if (objSym.isUndefined() || excludedByComdat) {
flags |= WASM_SYMBOL_UNDEFINED;
if (objSym.isExecutable())
- return symtab->addUndefinedFunction(name, "", "", flags, &f, nullptr,
+ return symtab->addUndefinedFunction(name, None, None, flags, &f, nullptr,
true);
return symtab->addUndefinedData(name, flags, &f);
}
diff --git a/lld/wasm/LTO.cpp b/lld/wasm/LTO.cpp
index 615b4c9dc944..0b7d8cb12d0e 100644
--- a/lld/wasm/LTO.cpp
+++ b/lld/wasm/LTO.cpp
@@ -77,8 +77,8 @@ BitcodeCompiler::~BitcodeCompiler() = default;
static void undefine(Symbol *s) {
if (auto f = dyn_cast<DefinedFunction>(s))
- replaceSymbol<UndefinedFunction>(f, f->getName(), "", "", 0, f->getFile(),
- f->signature);
+ replaceSymbol<UndefinedFunction>(f, f->getName(), None, None, 0,
+ f->getFile(), f->signature);
else if (isa<DefinedData>(s))
replaceSymbol<UndefinedData>(s, s->getName(), 0, s->getFile());
else
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index ce74b6ad7ea5..24187f81ed16 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -404,31 +404,33 @@ Symbol *SymbolTable::addDefinedEvent(StringRef name, uint32_t flags,
// become available when the LTO object is read. In this case we silently
// replace the empty attributes with the valid ones.
template <typename T>
-static void setImportAttributes(T *existing, StringRef importName,
- StringRef importModule, InputFile *file) {
- if (!importName.empty()) {
- if (existing->importName.empty())
+static void setImportAttributes(T *existing, Optional<StringRef> importName,
+ Optional<StringRef> importModule,
+ InputFile *file) {
+ if (importName) {
+ if (!existing->importName)
existing->importName = importName;
if (existing->importName != importName)
error("import name mismatch for symbol: " + toString(*existing) +
- "\n>>> defined as " + existing->importName + " in " +
- toString(existing->getFile()) + "\n>>> defined as " + importName +
+ "\n>>> defined as " + *existing->importName + " in " +
+ toString(existing->getFile()) + "\n>>> defined as " + *importName +
" in " + toString(file));
}
- if (!importModule.empty()) {
- if (existing->importModule.empty())
+ if (importModule) {
+ if (!existing->importModule)
existing->importModule = importModule;
if (existing->importModule != importModule)
error("import module mismatch for symbol: " + toString(*existing) +
- "\n>>> defined as " + existing->importModule + " in " +
- toString(existing->getFile()) + "\n>>> defined as " + importModule +
- " in " + toString(file));
+ "\n>>> defined as " + *existing->importModule + " in " +
+ toString(existing->getFile()) + "\n>>> defined as " +
+ *importModule + " in " + toString(file));
}
}
-Symbol *SymbolTable::addUndefinedFunction(StringRef name, StringRef importName,
- StringRef importModule,
+Symbol *SymbolTable::addUndefinedFunction(StringRef name,
+ Optional<StringRef> importName,
+ Optional<StringRef> importModule,
uint32_t flags, InputFile *file,
const WasmSignature *sig,
bool isCalledDirectly) {
@@ -497,9 +499,10 @@ Symbol *SymbolTable::addUndefinedData(StringRef name, uint32_t flags,
return s;
}
-Symbol *SymbolTable::addUndefinedGlobal(StringRef name, StringRef importName,
- StringRef importModule, uint32_t flags,
- InputFile *file,
+Symbol *SymbolTable::addUndefinedGlobal(StringRef name,
+ Optional<StringRef> importName,
+ Optional<StringRef> importModule,
+ uint32_t flags, InputFile *file,
const WasmGlobalType *type) {
LLVM_DEBUG(dbgs() << "addUndefinedGlobal: " << name << "\n");
assert(flags & WASM_SYMBOL_UNDEFINED);
diff --git a/lld/wasm/SymbolTable.h b/lld/wasm/SymbolTable.h
index 622359b73eb3..9803ad439f9c 100644
--- a/lld/wasm/SymbolTable.h
+++ b/lld/wasm/SymbolTable.h
@@ -15,6 +15,7 @@
#include "lld/Common/LLVM.h"
#include "llvm/ADT/CachedHashString.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
namespace lld {
namespace wasm {
@@ -59,14 +60,18 @@ class SymbolTable {
Symbol *addDefinedEvent(StringRef name, uint32_t flags, InputFile *file,
InputEvent *e);
- Symbol *addUndefinedFunction(StringRef name, StringRef importName,
- StringRef importModule, uint32_t flags,
- InputFile *file, const WasmSignature *signature,
+ Symbol *addUndefinedFunction(StringRef name,
+ llvm::Optional<StringRef> importName,
+ llvm::Optional<StringRef> importModule,
+ uint32_t flags, InputFile *file,
+ const WasmSignature *signature,
bool isCalledDirectly);
Symbol *addUndefinedData(StringRef name, uint32_t flags, InputFile *file);
- Symbol *addUndefinedGlobal(StringRef name, StringRef importName,
- StringRef importModule, uint32_t flags,
- InputFile *file, const WasmGlobalType *type);
+ Symbol *addUndefinedGlobal(StringRef name,
+ llvm::Optional<StringRef> importName,
+ llvm::Optional<StringRef> importModule,
+ uint32_t flags, InputFile *file,
+ const WasmGlobalType *type);
void addLazy(ArchiveFile *f, const llvm::object::Archive::Symbol *sym);
diff --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h
index ec3c72dd988e..3d0c15851dc5 100644
--- a/lld/wasm/Symbols.h
+++ b/lld/wasm/Symbols.h
@@ -11,6 +11,7 @@
#include "Config.h"
#include "lld/Common/LLVM.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/Object/Archive.h"
#include "llvm/Object/Wasm.h"
@@ -203,20 +204,21 @@ class DefinedFunction : public FunctionSymbol {
class UndefinedFunction : public FunctionSymbol {
public:
- UndefinedFunction(StringRef name, StringRef importName,
- StringRef importModule, uint32_t flags,
+ UndefinedFunction(StringRef name, llvm::Optional<StringRef> importName,
+ llvm::Optional<StringRef> importModule, uint32_t flags,
InputFile *file = nullptr,
const WasmSignature *type = nullptr,
bool isCalledDirectly = true)
: FunctionSymbol(name, UndefinedFunctionKind, flags, file, type),
- importName(importName), importModule(importModule), isCalledDirectly(isCalledDirectly) {}
+ importName(importName), importModule(importModule),
+ isCalledDirectly(isCalledDirectly) {}
static bool classof(const Symbol *s) {
return s->kind() == UndefinedFunctionKind;
}
- StringRef importName;
- StringRef importModule;
+ llvm::Optional<StringRef> importName;
+ llvm::Optional<StringRef> importModule;
bool isCalledDirectly;
};
@@ -335,8 +337,9 @@ class DefinedGlobal : public GlobalSymbol {
class UndefinedGlobal : public GlobalSymbol {
public:
- UndefinedGlobal(StringRef name, StringRef importName, StringRef importModule,
- uint32_t flags, InputFile *file = nullptr,
+ UndefinedGlobal(StringRef name, llvm::Optional<StringRef> importName,
+ llvm::Optional<StringRef> importModule, uint32_t flags,
+ InputFile *file = nullptr,
const WasmGlobalType *type = nullptr)
: GlobalSymbol(name, UndefinedGlobalKind, flags, file, type),
importName(importName), importModule(importModule) {}
@@ -345,8 +348,8 @@ class UndefinedGlobal : public GlobalSymbol {
return s->kind() == UndefinedGlobalKind;
}
- StringRef importName;
- StringRef importModule;
+ llvm::Optional<StringRef> importName;
+ llvm::Optional<StringRef> importModule;
};
// Wasm events are features that suspend the current execution and transfer the
@@ -510,7 +513,7 @@ union SymbolUnion {
// It is important to keep the size of SymbolUnion small for performance and
// memory usage reasons. 96 bytes is a soft limit based on the size of
// UndefinedFunction on a 64-bit system.
-static_assert(sizeof(SymbolUnion) <= 96, "SymbolUnion too large");
+static_assert(sizeof(SymbolUnion) <= 112, "SymbolUnion too large");
void printTraceSymbol(Symbol *sym);
void printTraceSymbolUndefined(StringRef name, const InputFile* file);
diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index ba04543211ca..68c1cf6fb0bf 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -156,11 +156,11 @@ void ImportSection::writeBody() {
for (const Symbol *sym : importedSymbols) {
WasmImport import;
if (auto *f = dyn_cast<UndefinedFunction>(sym)) {
- import.Field = f->importName;
- import.Module = f->importModule;
+ import.Field = f->importName ? *f->importName : sym->getName();
+ import.Module = f->importModule ? *f->importModule : defaultModule;
} else if (auto *g = dyn_cast<UndefinedGlobal>(sym)) {
- import.Field = g->importName;
- import.Module = g->importModule;
+ import.Field = g->importName ? *g->importName : sym->getName();
+ import.Module = g->importModule ? *g->importModule : defaultModule;
} else {
import.Field = sym->getName();
import.Module = defaultModule;
diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index 59f99cc8cd37..e39760e59ec2 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -15,6 +15,7 @@
#define LLVM_BINARYFORMAT_WASM_H
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -178,9 +179,12 @@ struct WasmSymbolInfo {
StringRef Name;
uint8_t Kind;
uint32_t Flags;
- StringRef ImportModule; // For undefined symbols the module of the import
- StringRef ImportName; // For undefined symbols the name of the import
- StringRef ExportName; // For symbols to be exported from the final module
+ // For undefined symbols the module of the import
+ Optional<StringRef> ImportModule;
+ // For undefined symbols the name of the import
+ Optional<StringRef> ImportName;
+ // For symbols to be exported from the final module
+ Optional<StringRef> ExportName;
union {
// For function or global symbols, the index in function or global index
// space.
diff --git a/llvm/include/llvm/MC/MCSymbolWasm.h b/llvm/include/llvm/MC/MCSymbolWasm.h
index a260d5d0a714..7f83e93a1d27 100644
--- a/llvm/include/llvm/MC/MCSymbolWasm.h
+++ b/llvm/include/llvm/MC/MCSymbolWasm.h
@@ -31,8 +31,6 @@ class MCSymbolWasm : public MCSymbol {
const MCExpr *SymbolSize = nullptr;
public:
- // Use a module name of "env" for now, for compatibility with existing tools.
- // This is temporary, and may change, as the ABI is not yet stable.
MCSymbolWasm(const StringMapEntry<bool> *Name, bool isTemporary)
: MCSymbol(SymbolKindWasm, Name, isTemporary) {}
static bool classof(const MCSymbol *S) { return S->isWasm(); }
@@ -71,10 +69,15 @@ class MCSymbolWasm : public MCSymbol {
bool isComdat() const { return IsComdat; }
void setComdat(bool isComdat) { IsComdat = isComdat; }
+ bool hasImportModule() const { return ImportModule.hasValue(); }
const StringRef getImportModule() const {
if (ImportModule.hasValue()) {
return ImportModule.getValue();
}
+ // Use a default module name of "env" for now, for compatibility with
+ // existing tools.
+ // TODO(sbc): Find a way to specify a default value in the object format
+ // without picking a hardcoded value like this.
return "env";
}
void setImportModule(StringRef Name) {
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 2e42324629f5..ca11ec3ad7bb 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -508,13 +508,16 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) {
Function.SymbolName = Info.Name;
} else {
wasm::WasmImport &Import = *ImportedFunctions[Info.ElementIndex];
- if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0)
+ if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0) {
Info.Name = readString(Ctx);
- else
+ Info.ImportName = Import.Field;
+ } else {
Info.Name = Import.Field;
+ }
Signature = &Signatures[Import.SigIndex];
- Info.ImportName = Import.Field;
- Info.ImportModule = Import.Module;
+ if (!Import.Module.empty()) {
+ Info.ImportModule = Import.Module;
+ }
}
break;
@@ -537,13 +540,17 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) {
Global.SymbolName = Info.Name;
} else {
wasm::WasmImport &Import = *ImportedGlobals[Info.ElementIndex];
- if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0)
+ if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0) {
Info.Name = readString(Ctx);
- else
+ Info.ImportName = Import.Field;
+ } else {
Info.Name = Import.Field;
+ }
GlobalType = &Import.Global;
Info.ImportName = Import.Field;
- Info.ImportModule = Import.Module;
+ if (!Import.Module.empty()) {
+ Info.ImportModule = Import.Module;
+ }
}
break;
@@ -597,14 +604,17 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) {
} else {
wasm::WasmImport &Import = *ImportedEvents[Info.ElementIndex];
- if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0)
+ if ((Info.Flags & wasm::WASM_SYMBOL_EXPLICIT_NAME) != 0) {
Info.Name = readString(Ctx);
- else
+ Info.ImportName = Import.Field;
+ } else {
Info.Name = Import.Field;
+ }
EventType = &Import.Event;
Signature = &Signatures[EventType->SigIndex];
- Info.ImportName = Import.Field;
- Info.ImportModule = Import.Module;
+ if (!Import.Module.empty()) {
+ Info.ImportModule = Import.Module;
+ }
}
break;
}
diff --git a/llvm/test/MC/WebAssembly/debug-info.ll b/llvm/test/MC/WebAssembly/debug-info.ll
index 4b54e1524a43..852d9ee2a93b 100644
--- a/llvm/test/MC/WebAssembly/debug-info.ll
+++ b/llvm/test/MC/WebAssembly/debug-info.ll
@@ -185,8 +185,6 @@
; CHECK-NEXT: Flags [ (0x10)
; CHECK-NEXT: UNDEFINED (0x10)
; CHECK-NEXT: ]
-; CHECK-NEXT: ImportName:
-; CHECK-NEXT: ImportModule:
; CHECK-NEXT: }
; CHECK-NEXT: Symbol {
; CHECK-NEXT: Name: ptr2
diff --git a/llvm/test/tools/llvm-readobj/wasm/symbols.test b/llvm/test/tools/llvm-readobj/wasm/symbols.test
index c55285344dba..52786737c965 100644
--- a/llvm/test/tools/llvm-readobj/wasm/symbols.test
+++ b/llvm/test/tools/llvm-readobj/wasm/symbols.test
@@ -26,7 +26,6 @@
# CHECK-NEXT: Flags [ (0x10)
# CHECK-NEXT: UNDEFINED (0x10)
# CHECK-NEXT: ]
-# CHECK-NEXT: ImportName: puts
# CHECK-NEXT: ImportModule: env
# CHECK-NEXT: ElementIndex: 0x0
# CHECK-NEXT: }
@@ -44,7 +43,6 @@
# CHECK-NEXT: Flags [ (0x10)
# CHECK-NEXT: UNDEFINED (0x10)
# CHECK-NEXT: ]
-# CHECK-NEXT: ImportName: SomeOtherFunction
# CHECK-NEXT: ImportModule: env
# CHECK-NEXT: ElementIndex: 0x1
# CHECK-NEXT: }
diff --git a/llvm/test/tools/llvm-readobj/wasm/wasm-imports.test b/llvm/test/tools/llvm-readobj/wasm/wasm-imports.test
index 6eb19673d99b..a6d8ca7b506d 100644
--- a/llvm/test/tools/llvm-readobj/wasm/wasm-imports.test
+++ b/llvm/test/tools/llvm-readobj/wasm/wasm-imports.test
@@ -107,7 +107,6 @@ Sections:
# CHECK-NEXT: Flags [ (0x10)
# CHECK-NEXT: UNDEFINED (0x10)
# CHECK-NEXT: ]
-# CHECK-NEXT: ImportName: foo
# CHECK-NEXT: ImportModule: red
# CHECK-NEXT: ElementIndex: 0x0
# CHECK-NEXT: }
diff --git a/llvm/tools/llvm-readobj/WasmDumper.cpp b/llvm/tools/llvm-readobj/WasmDumper.cpp
index dfab9f40d71b..bc163a27462b 100644
--- a/llvm/tools/llvm-readobj/WasmDumper.cpp
+++ b/llvm/tools/llvm-readobj/WasmDumper.cpp
@@ -227,8 +227,12 @@ void WasmDumper::printSymbol(const SymbolRef &Sym) {
W.printFlags("Flags", Symbol.Info.Flags, makeArrayRef(WasmSymbolFlags));
if (Symbol.Info.Flags & wasm::WASM_SYMBOL_UNDEFINED) {
- W.printString("ImportName", Symbol.Info.ImportName);
- W.printString("ImportModule", Symbol.Info.ImportModule);
+ if (Symbol.Info.ImportName) {
+ W.printString("ImportName", *Symbol.Info.ImportName);
+ }
+ if (Symbol.Info.ImportModule) {
+ W.printString("ImportModule", *Symbol.Info.ImportModule);
+ }
}
if (Symbol.Info.Kind != wasm::WASM_SYMBOL_TYPE_DATA) {
W.printHex("ElementIndex", Symbol.Info.ElementIndex);
More information about the llvm-commits
mailing list