Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store HeapTypes in Tags #7220

Merged
merged 3 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/binaryen-c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4770,7 +4770,7 @@ BinaryenTagRef BinaryenAddTag(BinaryenModuleRef module,
BinaryenType results) {
auto* ret = new Tag();
ret->setExplicitName(name);
ret->sig = Signature(Type(params), Type(results));
ret->type = Signature(Type(params), Type(results));
((Module*)module)->addTag(ret);
return ret;
}
Expand Down Expand Up @@ -4874,7 +4874,7 @@ void BinaryenAddTagImport(BinaryenModuleRef module,
tag->name = internalName;
tag->module = externalModuleName;
tag->base = externalBaseName;
tag->sig = Signature(Type(params), Type(results));
tag->type = Signature(Type(params), Type(results));
((Module*)module)->addTag(std::move(tag));
} else {
// already exists so just set module and base
Expand Down Expand Up @@ -5838,11 +5838,11 @@ const char* BinaryenTagGetName(BinaryenTagRef tag) {
return ((Tag*)tag)->name.str.data();
}
BinaryenType BinaryenTagGetParams(BinaryenTagRef tag) {
return ((Tag*)tag)->sig.params.getID();
return ((Tag*)tag)->params().getID();
}

BinaryenType BinaryenTagGetResults(BinaryenTagRef tag) {
return ((Tag*)tag)->sig.results.getID();
return ((Tag*)tag)->results().getID();
}

//
Expand Down
4 changes: 2 additions & 2 deletions src/ir/child-typer.h
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
void visitTryTable(TryTable* curr) { note(&curr->body, curr->type); }

void visitThrow(Throw* curr) {
auto type = wasm.getTag(curr->tag)->sig.params;
auto type = wasm.getTag(curr->tag)->params();
assert(curr->operands.size() == type.size());
for (size_t i = 0; i < type.size(); ++i) {
note(&curr->operands[i], type[i]);
Expand Down Expand Up @@ -1113,7 +1113,7 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
}

void visitSuspend(Suspend* curr) {
auto params = wasm.getTag(curr->tag)->sig.params;
auto params = wasm.getTag(curr->tag)->params();
assert(params.size() == curr->operands.size());
for (size_t i = 0; i < params.size(); ++i) {
note(&curr->operands[i], params[i]);
Expand Down
2 changes: 1 addition & 1 deletion src/ir/eh-utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ void handleBlockNestedPop(Try* try_, Function* func, Module& wasm) {
for (Index i = 0; i < try_->catchTags.size(); i++) {
Name tagName = try_->catchTags[i];
auto* tag = wasm.getTag(tagName);
if (tag->sig.params == Type::none) {
if (tag->params() == Type::none) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion src/ir/module-splitting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ void ModuleSplitter::shareImportableItems() {

for (auto& tag : primary.tags) {
auto secondaryTag = std::make_unique<Tag>();
secondaryTag->sig = tag->sig;
secondaryTag->type = tag->type;
makeImportExport(*tag, *secondaryTag, "tag", ExternalKind::Tag);
secondary.addTag(std::move(secondaryTag));
}
Expand Down
4 changes: 2 additions & 2 deletions src/ir/module-utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ Tag* copyTag(Tag* tag, Module& out) {
auto* ret = new Tag();
ret->name = tag->name;
ret->hasExplicitName = tag->hasExplicitName;
ret->sig = tag->sig;
ret->type = tag->type;
ret->module = tag->module;
ret->base = tag->base;
out.addTag(ret);
Expand Down Expand Up @@ -474,7 +474,7 @@ InsertOrderedMap<HeapType, HeapTypeInfo> collectHeapTypeInfo(
info.note(curr->type);
}
for (auto& curr : wasm.tags) {
info.note(curr->sig);
info.note(curr->type);
}
for (auto& curr : wasm.tables) {
info.note(curr->type);
Expand Down
4 changes: 2 additions & 2 deletions src/ir/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ struct InfoCollector
auto tag = curr->catchTags[tagIndex];
auto* body = curr->catchBodies[tagIndex];

auto params = getModule()->getTag(tag)->sig.params;
auto params = getModule()->getTag(tag)->params();
if (params.size() == 0) {
continue;
}
Expand Down Expand Up @@ -1189,7 +1189,7 @@ struct InfoCollector

Index exnrefIndex = 0;
if (tag.is()) {
auto params = getModule()->getTag(tag)->sig.params;
auto params = getModule()->getTag(tag)->params();

for (Index i = 0; i < params.size(); i++) {
if (isRelevant(params[i])) {
Expand Down
2 changes: 1 addition & 1 deletion src/ir/subtype-exprs.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
}
}
void visitThrow(Throw* curr) {
Type params = self()->getModule()->getTag(curr->tag)->sig.params;
Type params = self()->getModule()->getTag(curr->tag)->params();
assert(params.size() == curr->operands.size());
for (size_t i = 0, size = curr->operands.size(); i < size; ++i) {
self()->noteSubtype(curr->operands[i], params[i]);
Expand Down
6 changes: 1 addition & 5 deletions src/ir/type-updating.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,6 @@ void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) {
return type;
}

Signature getNew(Signature sig) {
return Signature(getNew(sig.params), getNew(sig.results));
}

void visitExpression(Expression* curr) {
// local.get and local.tee are special in that their type is tied to the
// type of the local in the function, which is tied to the signature. That
Expand Down Expand Up @@ -304,7 +300,7 @@ void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) {
global->type = updater.getNew(global->type);
}
for (auto& tag : wasm.tags) {
tag->sig = updater.getNew(tag->sig);
tag->type = updater.getNew(tag->type);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/parser/contexts.h
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ struct ParseModuleTypesCtx : TypeParserCtx<ParseModuleTypesCtx>,
if (!use.type.isSignature()) {
return in.err(pos, "tag type must be a signature");
}
t->sig = use.type.getSignature();
t->type = use.type;
return Ok{};
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/passes/MergeBlocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ struct ProblemFinder
// make more sense in TupleOptimization though as it would need
// to track uses of parts of a tuple.
if (!tryy->catchTags[i] ||
getModule()->getTag(tryy->catchTags[i])->sig.params.size() == 0) {
getModule()->getTag(tryy->catchTags[i])->params().size() == 0) {
// There must be a ref here, otherwise there is no value being sent
// at all, and we should not be running ProblemFinder at all.
assert(tryy->catchRefs[i]);
Expand Down
44 changes: 27 additions & 17 deletions src/passes/Print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
void visitTag(Tag* curr);
void visitImportedTag(Tag* curr);
void visitDefinedTag(Tag* curr);
void printTagType(HeapType type);
void printTableHeader(Table* curr);
void visitTable(Table* curr);
void visitElementSegment(ElementSegment* curr);
Expand Down Expand Up @@ -3185,32 +3186,41 @@ void PrintSExpression::visitImportedTag(Tag* curr) {
emitImportHeader(curr);
o << "(tag ";
curr->name.print(o);
if (curr->sig.params != Type::none) {
o << maybeSpace;
printParamType(curr->sig.params);
}
if (curr->sig.results != Type::none) {
o << maybeSpace;
printResultType(curr->sig.results);
}
o << "))";
o << maybeNewLine;
o << maybeSpace;
printTagType(curr->type);
o << "))" << maybeNewLine;
}

void PrintSExpression::visitDefinedTag(Tag* curr) {
doIndent(o, indent);
o << '(';
printMedium(o, "tag ");
curr->name.print(o);
if (curr->sig.params != Type::none) {
o << maybeSpace;
printParamType(curr->sig.params);
o << maybeSpace;
printTagType(curr->type);
o << ')' << maybeNewLine;
}

void PrintSExpression::printTagType(HeapType type) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like handleSignature (used for Function) is similar - can we share this code perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take another look at it. At first glance the existence of parameter names for functions made it seem writing separate code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I think it's more trouble than it's worth right now.

o << "(type ";
printHeapType(type);
o << ')';
if (auto params = type.getSignature().params; params != Type::none) {
o << maybeSpace << "(param";
for (auto t : params) {
o << ' ';
printType(t);
}
o << ')';
}
if (curr->sig.results != Type::none) {
o << maybeSpace;
printResultType(curr->sig.results);
if (auto results = type.getSignature().results; results != Type::none) {
o << maybeSpace << "(result";
for (auto t : results) {
o << ' ';
printType(t);
}
o << ')';
}
o << ")" << maybeNewLine;
}

void PrintSExpression::printTableHeader(Table* curr) {
Expand Down
2 changes: 1 addition & 1 deletion src/passes/TranslateEH.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ struct TranslateToExnref : public WalkerPass<PostWalker<TranslateToExnref>> {
auto* catchBody = curr->catchBodies[i];
Type tagType = Type::none;
if (tryTable->catchTags[i]) {
tagType = wasm->getTag(tryTable->catchTags[i])->sig.params;
tagType = wasm->getTag(tryTable->catchTags[i])->params();
}

// This is to be the body of the next(outer) level block
Expand Down
6 changes: 3 additions & 3 deletions src/tools/fuzzing/fuzzing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2177,7 +2177,7 @@ Expression* TranslateToFuzzReader::makeTry(Type type) {
// Catch bodies (aside from a catch-all) begin with a pop.
Expression* prefix = nullptr;
if (i < numTags) {
auto tagType = wasm.getTag(catchTags[i])->sig.params;
auto tagType = wasm.getTag(catchTags[i])->params();
if (tagType != Type::none) {
auto* pop = builder.makePop(tagType);
// Capture the pop in a local, so that it can be used later.
Expand Down Expand Up @@ -2223,7 +2223,7 @@ Expression* TranslateToFuzzReader::makeTryTable(Type type) {
// Look for a specific tag.
auto& tag = pick(wasm.tags);
tagName = tag->name;
tagType = tag->sig.params;
tagType = tag->params();
} else {
// Add a catch_all at the end, some of the time (but all of the time if we
// have nothing else).
Expand Down Expand Up @@ -4793,7 +4793,7 @@ Expression* TranslateToFuzzReader::makeThrow(Type type) {
addTag();
}
auto* tag = pick(wasm.tags).get();
auto tagType = tag->sig.params;
auto tagType = tag->params();
std::vector<Expression*> operands;
for (auto t : tagType) {
operands.push_back(make(t));
Expand Down
6 changes: 3 additions & 3 deletions src/tools/wasm-merge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,10 @@ void fuseImportsAndExports() {
kindModuleExportMaps[ExternalKind::Tag][import->module][import->base];
if (internalName.is()) {
auto* export_ = merged.getTag(internalName);
if (HeapType(export_->sig) != HeapType(import->sig)) {
if (export_->type != import->type) {
reportTypeMismatch(valid, "tag", import);
std::cerr << "export type " << export_->sig
<< " is different from import type " << import->sig << ".\n";
std::cerr << "export type " << export_->type
<< " is different from import type " << import->type << ".\n";
}
}
});
Expand Down
4 changes: 2 additions & 2 deletions src/wasm-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,10 @@ class Builder {
return glob;
}

static std::unique_ptr<Tag> makeTag(Name name, Signature sig) {
static std::unique_ptr<Tag> makeTag(Name name, HeapType type) {
auto tag = std::make_unique<Tag>();
tag->name = name;
tag->sig = sig;
tag->type = type;
return tag;
}

Expand Down
4 changes: 3 additions & 1 deletion src/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -2297,7 +2297,9 @@ class Global : public Importable {

class Tag : public Importable {
public:
Signature sig;
HeapType type;
Type params() { return type.getSignature().params; }
tlively marked this conversation as resolved.
Show resolved Hide resolved
Type results() { return type.getSignature().results; }
};

// "Opaque" data, not part of the core wasm spec, that is held in binaries.
Expand Down
4 changes: 2 additions & 2 deletions src/wasm/wasm-binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ void WasmBinaryWriter::writeImports() {
writeImportHeader(tag);
o << U32LEB(int32_t(ExternalKind::Tag));
o << uint8_t(0); // Reserved 'attribute' field. Always 0.
o << U32LEB(getTypeIndex(tag->sig));
o << U32LEB(getTypeIndex(tag->type));
});
ModuleUtils::iterImportedMemories(*wasm, [&](Memory* memory) {
writeImportHeader(memory);
Expand Down Expand Up @@ -854,7 +854,7 @@ void WasmBinaryWriter::writeTags() {
o << U32LEB(num);
ModuleUtils::iterDefinedTags(*wasm, [&](Tag* tag) {
o << uint8_t(0); // Reserved 'attribute' field. Always 0.
o << U32LEB(getTypeIndex(tag->sig));
o << U32LEB(getTypeIndex(tag->type));
});

finishSection(start);
Expand Down
6 changes: 3 additions & 3 deletions src/wasm/wasm-ir-builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ Result<> IRBuilder::visitCatch(Name tag) {

CHECK_ERR(pushScope(ScopeCtx::makeCatch(std::move(scope), tryy)));
// Push a pop for the exception payload if necessary.
auto params = wasm.getTag(tag)->sig.params;
auto params = wasm.getTag(tag)->params();
if (params != Type::none) {
// Note that we have a pop to help determine later whether we need to run
// the fixup for pops within blocks.
Expand Down Expand Up @@ -1815,7 +1815,7 @@ Result<> IRBuilder::makeTryTable(Name label,
Result<> IRBuilder::makeThrow(Name tag) {
Throw curr(wasm.allocator);
curr.tag = tag;
curr.operands.resize(wasm.getTag(tag)->sig.params.size());
curr.operands.resize(wasm.getTag(tag)->params().size());
CHECK_ERR(visitThrow(&curr));
push(builder.makeThrow(tag, curr.operands));
return Ok{};
Expand Down Expand Up @@ -2343,7 +2343,7 @@ Result<> IRBuilder::makeResume(HeapType ct,
Result<> IRBuilder::makeSuspend(Name tag) {
Suspend curr(wasm.allocator);
curr.tag = tag;
curr.operands.resize(wasm.getTag(tag)->sig.params.size());
curr.operands.resize(wasm.getTag(tag)->params().size());
CHECK_ERR(visitSuspend(&curr));

std::vector<Expression*> operands(curr.operands.begin(), curr.operands.end());
Expand Down
Loading
Loading