Skip to content

Commit

Permalink
[WebAssembly] Remove WasmTagType
Browse files Browse the repository at this point in the history
This removes `WasmTagType`. `WasmTagType` contained an attribute and a
signature index:
```
struct WasmTagType {
  uint8_t Attribute;
  uint32_t SigIndex;
};
```

Currently the attribute field is not used and reserved for future use,
and always 0. And that this class contains `SigIndex` as its property is
a little weird in the place, because the tag type's signature index is
not an inherent property of a tag but rather a reference to another
section that changes after linking. This makes tag handling in the
linker also weird that tag-related methods are taking both `WasmTagType`
and `WasmSignature` even though `WasmTagType` contains a signature
index. This is because the signature index changes in linking so it
doesn't have any info at this point. This instead moves `SigIndex` to
`struct WasmTag` itself, as we did for `struct WasmFunction` in D111104.

In this CL, in lib/MC and lib/Object, this now treats tag types in the
same way as function types. Also in YAML, this removes `struct Tag`,
because now it only contains the tag index. Also tags set `SigIndex` in
`WasmImport` union, as functions do.

I think this makes things simpler and makes tag handling more in line
with function handling. These two shares similar properties in that both
of them have signatures, but they are kind of nominal so having the same
signature doesn't mean they are the same element.

Also a drive-by fix: the reserved 'attirubute' part's encoding changed
from uleb32 to uint8 a while ago. This was fixed in lib/MC and
lib/Object but not in YAML. This doesn't change object files because the
field's value is always 0 and its encoding is the same for the both
encoding.

This is effectively NFC; I didn't mark it as such just because it
changed YAML test results.

Reviewed By: sbc100, tlively

Differential Revision: https://reviews.llvm.org/D111086
  • Loading branch information
aheejin committed Oct 6, 2021
1 parent d51f57c commit 3ec1760
Show file tree
Hide file tree
Showing 23 changed files with 68 additions and 165 deletions.
2 changes: 0 additions & 2 deletions lld/include/lld/Common/LLVM.h
Expand Up @@ -45,7 +45,6 @@ class WasmSymbol;

namespace wasm {
struct WasmTag;
struct WasmTagType;
struct WasmFunction;
struct WasmGlobal;
struct WasmGlobalType;
Expand Down Expand Up @@ -97,7 +96,6 @@ using llvm::wasm::WasmSignature;
using llvm::wasm::WasmTable;
using llvm::wasm::WasmTableType;
using llvm::wasm::WasmTag;
using llvm::wasm::WasmTagType;
} // end namespace lld.

namespace std {
Expand Down
5 changes: 1 addition & 4 deletions lld/test/wasm/tag-section.ll
Expand Up @@ -30,10 +30,7 @@ define void @_start() {
; CHECK-NEXT: ReturnTypes: []

; CHECK: - Type: TAG
; CHECK-NEXT: Tags:
; CHECK-NEXT: - Index: 0
; CHECK-NEXT: Attribute: 0
; CHECK-NEXT: SigIndex: 1
; CHECK-NEXT: TagTypes: [ 1 ]

; Global section has to come after tag section
; CHECK: - Type: GLOBAL
Expand Down
7 changes: 1 addition & 6 deletions lld/wasm/InputElement.h
Expand Up @@ -74,14 +74,9 @@ class InputGlobal : public InputElement {
class InputTag : public InputElement {
public:
InputTag(const WasmSignature &s, const WasmTag &t, ObjFile *f)
: InputElement(t.SymbolName, f), signature(s), type(t.Type) {}

const WasmTagType &getType() const { return type; }
: InputElement(t.SymbolName, f), signature(s) {}

const WasmSignature &signature;

private:
WasmTagType type;
};

class InputTable : public InputElement {
Expand Down
7 changes: 3 additions & 4 deletions lld/wasm/InputFiles.cpp
Expand Up @@ -336,10 +336,9 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded(
LLVM_DEBUG(dbgs() << "Synthesizing symbol for table import: " << info->Name
<< "\n");
const WasmGlobalType *globalType = nullptr;
const WasmTagType *tagType = nullptr;
const WasmSignature *signature = nullptr;
auto *wasmSym = make<WasmSymbol>(*info, globalType, &tableImport->Table,
tagType, signature);
auto *wasmSym =
make<WasmSymbol>(*info, globalType, &tableImport->Table, signature);
Symbol *sym = createUndefined(*wasmSym, false);
// We're only sure it's a TableSymbol if the createUndefined succeeded.
if (errorCount())
Expand Down Expand Up @@ -516,7 +515,7 @@ void ObjFile::parse(bool ignoreComdats) {

// Populate `Tags`.
for (const WasmTag &t : wasmObj->tags())
tags.emplace_back(make<InputTag>(types[t.Type.SigIndex], t, this));
tags.emplace_back(make<InputTag>(types[t.SigIndex], t, this));

// Populate `Symbols` based on the symbols in the object.
symbols.reserve(wasmObj->getNumberOfSymbols());
Expand Down
8 changes: 1 addition & 7 deletions lld/wasm/SymbolTable.cpp
Expand Up @@ -169,20 +169,14 @@ static void checkGlobalType(const Symbol *existing, const InputFile *file,
}

static void checkTagType(const Symbol *existing, const InputFile *file,
const WasmTagType *newType,
const WasmSignature *newSig) {
const auto *existingTag = dyn_cast<TagSymbol>(existing);
if (!isa<TagSymbol>(existing)) {
reportTypeError(existing, file, WASM_SYMBOL_TYPE_TAG);
return;
}

const WasmTagType *oldType = cast<TagSymbol>(existing)->getTagType();
const WasmSignature *oldSig = existingTag->signature;
if (newType->Attribute != oldType->Attribute)
error("Tag type mismatch: " + existing->getName() + "\n>>> defined as " +
toString(*oldType) + " in " + toString(existing->getFile()) +
"\n>>> defined as " + toString(*newType) + " in " + toString(file));
if (*newSig != *oldSig)
warn("Tag signature mismatch: " + existing->getName() +
"\n>>> defined as " + toString(*oldSig) + " in " +
Expand Down Expand Up @@ -430,7 +424,7 @@ Symbol *SymbolTable::addDefinedTag(StringRef name, uint32_t flags,
return s;
}

checkTagType(s, file, &tag->getType(), &tag->signature);
checkTagType(s, file, &tag->signature);

if (shouldReplace(s, file, flags))
replaceSym();
Expand Down
1 change: 0 additions & 1 deletion lld/wasm/Symbols.cpp
Expand Up @@ -369,7 +369,6 @@ bool TagSymbol::hasTagIndex() const {
DefinedTag::DefinedTag(StringRef name, uint32_t flags, InputFile *file,
InputTag *tag)
: TagSymbol(name, DefinedTagKind, flags, file,
tag ? &tag->getType() : nullptr,
tag ? &tag->signature : nullptr),
tag(tag) {}

Expand Down
7 changes: 2 additions & 5 deletions lld/wasm/Symbols.h
Expand Up @@ -439,8 +439,6 @@ class TagSymbol : public Symbol {
public:
static bool classof(const Symbol *s) { return s->kind() == DefinedTagKind; }

const WasmTagType *getTagType() const { return tagType; }

// Get/set the tag index
uint32_t getTagIndex() const;
void setTagIndex(uint32_t index);
Expand All @@ -450,10 +448,9 @@ class TagSymbol : public Symbol {

protected:
TagSymbol(StringRef name, Kind k, uint32_t flags, InputFile *f,
const WasmTagType *tagType, const WasmSignature *sig)
: Symbol(name, k, flags, f), signature(sig), tagType(tagType) {}
const WasmSignature *sig)
: Symbol(name, k, flags, f), signature(sig) {}

const WasmTagType *tagType;
uint32_t tagIndex = INVALID_INDEX;
};

Expand Down
8 changes: 3 additions & 5 deletions lld/wasm/SyntheticSections.cpp
Expand Up @@ -226,8 +226,7 @@ void ImportSection::writeBody() {
import.Global = *globalSym->getGlobalType();
} else if (auto *tagSym = dyn_cast<TagSymbol>(sym)) {
import.Kind = WASM_EXTERNAL_TAG;
import.Tag.Attribute = tagSym->getTagType()->Attribute;
import.Tag.SigIndex = out.typeSec->lookupType(*tagSym->signature);
import.SigIndex = out.typeSec->lookupType(*tagSym->signature);
} else {
auto *tableSym = cast<TableSymbol>(sym);
import.Kind = WASM_EXTERNAL_TABLE;
Expand Down Expand Up @@ -332,9 +331,8 @@ void TagSection::writeBody() {

writeUleb128(os, inputTags.size(), "tag count");
for (InputTag *t : inputTags) {
WasmTagType type = t->getType();
type.SigIndex = out.typeSec->lookupType(t->signature);
writeTagType(os, type);
writeUleb128(os, 0, "tag attribute"); // Reserved "attribute" field
writeUleb128(os, out.typeSec->lookupType(t->signature), "sig index");
}
}

Expand Down
18 changes: 2 additions & 16 deletions lld/wasm/WriterUtils.cpp
Expand Up @@ -58,12 +58,6 @@ std::string toString(const WasmGlobalType &type) {
toString(static_cast<ValType>(type.Type));
}

std::string toString(const WasmTagType &type) {
if (type.Attribute == WASM_TAG_ATTRIBUTE_EXCEPTION)
return "exception";
return "unknown";
}

static std::string toString(const llvm::wasm::WasmLimits &limits) {
std::string ret;
ret += "flags=0x" + std::to_string(limits.Flags);
Expand Down Expand Up @@ -204,15 +198,6 @@ void writeGlobalType(raw_ostream &os, const WasmGlobalType &type) {
writeU8(os, type.Mutable, "global mutable");
}

void writeTagType(raw_ostream &os, const WasmTagType &type) {
writeUleb128(os, type.Attribute, "tag attribute");
writeUleb128(os, type.SigIndex, "sig index");
}

void writeTag(raw_ostream &os, const WasmTag &tag) {
writeTagType(os, tag.Type);
}

void writeTableType(raw_ostream &os, const WasmTableType &type) {
writeValueType(os, ValType(type.ElemType), "table type");
writeLimits(os, type.Limits);
Expand All @@ -230,7 +215,8 @@ void writeImport(raw_ostream &os, const WasmImport &import) {
writeGlobalType(os, import.Global);
break;
case WASM_EXTERNAL_TAG:
writeTagType(os, import.Tag);
writeUleb128(os, 0, "tag attribute"); // Reserved "attribute" field
writeUleb128(os, import.SigIndex, "import sig index");
break;
case WASM_EXTERNAL_MEMORY:
writeLimits(os, import.Memory);
Expand Down
5 changes: 0 additions & 5 deletions lld/wasm/WriterUtils.h
Expand Up @@ -55,10 +55,6 @@ void writeLimits(raw_ostream &os, const llvm::wasm::WasmLimits &limits);

void writeGlobalType(raw_ostream &os, const llvm::wasm::WasmGlobalType &type);

void writeTagType(raw_ostream &os, const llvm::wasm::WasmTagType &type);

void writeTag(raw_ostream &os, const llvm::wasm::WasmTag &tag);

void writeTableType(raw_ostream &os, const llvm::wasm::WasmTableType &type);

void writeImport(raw_ostream &os, const llvm::wasm::WasmImport &import);
Expand All @@ -70,7 +66,6 @@ void writeExport(raw_ostream &os, const llvm::wasm::WasmExport &export_);
std::string toString(llvm::wasm::ValType type);
std::string toString(const llvm::wasm::WasmSignature &sig);
std::string toString(const llvm::wasm::WasmGlobalType &type);
std::string toString(const llvm::wasm::WasmTagType &type);
std::string toString(const llvm::wasm::WasmTableType &type);

} // namespace lld
Expand Down
9 changes: 1 addition & 8 deletions llvm/include/llvm/BinaryFormat/Wasm.h
Expand Up @@ -107,15 +107,9 @@ struct WasmGlobal {
StringRef SymbolName; // from the "linking" section
};

struct WasmTagType {
// Kind of tag. Currently only WASM_TAG_ATTRIBUTE_EXCEPTION is possible.
uint8_t Attribute;
uint32_t SigIndex;
};

struct WasmTag {
uint32_t Index;
WasmTagType Type;
uint32_t SigIndex;
StringRef SymbolName; // from the "linking" section
};

Expand All @@ -128,7 +122,6 @@ struct WasmImport {
WasmGlobalType Global;
WasmTableType Table;
WasmLimits Memory;
WasmTagType Tag;
};
};

Expand Down
7 changes: 0 additions & 7 deletions llvm/include/llvm/MC/MCSymbolWasm.h
Expand Up @@ -27,7 +27,6 @@ class MCSymbolWasm : public MCSymbol {
wasm::WasmSignature *Signature = nullptr;
Optional<wasm::WasmGlobalType> GlobalType;
Optional<wasm::WasmTableType> TableType;
Optional<wasm::WasmTagType> TagType;

/// An expression describing how to calculate the size of a symbol. If a
/// symbol has no size this field will be NULL.
Expand Down Expand Up @@ -147,12 +146,6 @@ class MCSymbolWasm : public MCSymbol {
wasm::WasmLimits Limits = {wasm::WASM_LIMITS_FLAG_NONE, 0, 0};
setTableType({uint8_t(VT), Limits});
}

const wasm::WasmTagType &getTagType() const {
assert(TagType.hasValue());
return TagType.getValue();
}
void setTagType(wasm::WasmTagType ET) { TagType = ET; }
};

} // end namespace llvm
Expand Down
5 changes: 1 addition & 4 deletions llvm/include/llvm/Object/Wasm.h
Expand Up @@ -37,15 +37,13 @@ class WasmSymbol {
WasmSymbol(const wasm::WasmSymbolInfo &Info,
const wasm::WasmGlobalType *GlobalType,
const wasm::WasmTableType *TableType,
const wasm::WasmTagType *TagType,
const wasm::WasmSignature *Signature)
: Info(Info), GlobalType(GlobalType), TableType(TableType),
TagType(TagType), Signature(Signature) {}
Signature(Signature) {}

const wasm::WasmSymbolInfo &Info;
const wasm::WasmGlobalType *GlobalType;
const wasm::WasmTableType *TableType;
const wasm::WasmTagType *TagType;
const wasm::WasmSignature *Signature;

bool isTypeFunction() const {
Expand Down Expand Up @@ -274,7 +272,6 @@ class WasmObjectFile : public ObjectFile {
wasm::WasmProducerInfo ProducerInfo;
std::vector<wasm::WasmFeatureEntry> TargetFeatures;
std::vector<wasm::WasmSignature> Signatures;
std::vector<uint32_t> FunctionTypes;
std::vector<wasm::WasmTable> Tables;
std::vector<wasm::WasmLimits> Memories;
std::vector<wasm::WasmGlobal> Globals;
Expand Down
15 changes: 2 additions & 13 deletions llvm/include/llvm/ObjectYAML/WasmYAML.h
Expand Up @@ -77,12 +77,6 @@ struct Global {
wasm::WasmInitExpr InitExpr;
};

struct Tag {
uint32_t Index;
uint32_t Attribute;
uint32_t SigIndex;
};

struct Import {
StringRef Module;
StringRef Field;
Expand All @@ -92,7 +86,7 @@ struct Import {
Global GlobalImport;
Table TableImport;
Limits Memory;
Tag TagImport;
uint32_t TagIndex;
};
};

Expand Down Expand Up @@ -329,7 +323,7 @@ struct TagSection : Section {
return S->Type == wasm::WASM_SEC_TAG;
}

std::vector<Tag> Tags;
std::vector<uint32_t> TagTypes;
};

struct GlobalSection : Section {
Expand Down Expand Up @@ -431,7 +425,6 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::SymbolInfo)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::InitFunction)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::ComdatEntry)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::Comdat)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::Tag)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::WasmYAML::DylinkExport)

namespace llvm {
Expand Down Expand Up @@ -577,10 +570,6 @@ template <> struct ScalarEnumerationTraits<WasmYAML::RelocType> {
static void enumeration(IO &IO, WasmYAML::RelocType &Kind);
};

template <> struct MappingTraits<WasmYAML::Tag> {
static void mapping(IO &IO, WasmYAML::Tag &Tag);
};

template <> struct MappingTraits<WasmYAML::DylinkExport> {
static void mapping(IO &IO, WasmYAML::DylinkExport &Export);
};
Expand Down

0 comments on commit 3ec1760

Please sign in to comment.