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.

Sign up for GitHub

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

Jump to bottom

[EH] Add instructions for new proposal #6181

Merged
merged 15 commits into from
Dec 19, 2023
Merged

[EH] Add instructions for new proposal #6181

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cdb8eb9
[EH] Add instructions for new proposal
aheejin Oct 2, 2023
424c799
clang-format
aheejin Dec 14, 2023
168b984
simplify
aheejin Dec 14, 2023
9026623
Merge branch 'main' into new_eh_inst
aheejin Dec 14, 2023
8bb08e5
validator todo
aheejin Dec 14, 2023
54b4e1c
Fix stale type error
aheejin Dec 14, 2023
51e4efd
Add comments
aheejin Dec 14, 2023
6eaa08c
binaryen.js test fix
aheejin Dec 14, 2023
b3888f8
Fix binaryen.js instruction ID shift
aheejin Dec 14, 2023
19a3a12
Add catchRefs and sentTypes to wasm-delegations-fields.def
aheejin Dec 14, 2023
5fe611d
Merge branch 'main' into new_eh_inst
aheejin Dec 14, 2023
a1568de
Fix
aheejin Dec 14, 2023
89a5e46
Address comments
aheejin Dec 15, 2023
21407d7
Fix
aheejin Dec 15, 2023
5420500
Revert branch-utils
aheejin Dec 16, 2023
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
4 changes: 3 additions & 1 deletion scripts/gen-s-parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,10 @@
#
# exception handling instructions
("try", "makeTry(s)"),
("try_table", "makeTryTable(s)"),
("throw", "makeThrow(s)"),
("rethrow", "makeRethrow(s)"),
("throw_ref", "makeThrowRef(s)"),
# Multivalue pseudoinstructions
("tuple.make", "makeTupleMake(s)"),
("tuple.extract", "makeTupleExtract(s)"),
Expand Down Expand Up @@ -714,7 +716,7 @@ def instruction_parser(new_parser=False):
inst_length = 0
for inst, expr in instructions:
if new_parser and inst in {"block", "loop", "if", "try", "then",
"else"}:
"else", "try_table"}:
# These are either control flow handled manually or not real
# instructions. Skip them.
continue
Expand Down
47 changes: 37 additions & 10 deletions src/gen-s-parser.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3393,15 +3393,31 @@ switch (buf[0]) {
case 'e':
if (op == "then"sv) { return makeThenOrElse(s); }
goto parse_error;
case 'r':
if (op == "throw"sv) { return makeThrow(s); }
case 'r': {
switch (buf[5]) {
case '\0':
if (op == "throw"sv) { return makeThrow(s); }
goto parse_error;
case '_':
if (op == "throw_ref"sv) { return makeThrowRef(s); }
goto parse_error;
default: goto parse_error;
}
}
default: goto parse_error;
}
}
case 'r': {
switch (buf[3]) {
case '\0':
if (op == "try"sv) { return makeTry(s); }
goto parse_error;
case '_':
if (op == "try_table"sv) { return makeTryTable(s); }
goto parse_error;
default: goto parse_error;
}
}
case 'r':
if (op == "try"sv) { return makeTry(s); }
goto parse_error;
case 'u': {
switch (buf[6]) {
case 'd':
Expand Down Expand Up @@ -8646,12 +8662,23 @@ switch (buf[0]) {
default: goto parse_error;
}
}
case 'h':
if (op == "throw"sv) {
CHECK_ERR(makeThrow(ctx, pos));
return Ok{};
case 'h': {
switch (buf[5]) {
case '\0':
if (op == "throw"sv) {
CHECK_ERR(makeThrow(ctx, pos));
return Ok{};
}
goto parse_error;
case '_':
if (op == "throw_ref"sv) {
CHECK_ERR(makeThrowRef(ctx, pos));
return Ok{};
}
goto parse_error;
default: goto parse_error;
}
goto parse_error;
}
case 'u': {
switch (buf[6]) {
case 'd':
Expand Down
8 changes: 8 additions & 0 deletions src/ir/LocalStructuralDominance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,14 @@ LocalStructuralDominance::LocalStructuralDominance(Function* func,
currp = &curr->cast<Try>()->body;
continue;
}
case Expression::Id::TryTableId: {
self->pushTask(Scanner::doEndScope, currp);
// Just call the task immediately.
doBeginScope(self, currp);
// Immediately continue in the try_table.
currp = &curr->cast<TryTable>()->body;
continue;
}

default: {
// Control flow structures have been handled. This is an expression,
Expand Down
2 changes: 2 additions & 0 deletions src/ir/ReFinalize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,10 @@ void ReFinalize::visitTableGrow(TableGrow* curr) { curr->finalize(); }
void ReFinalize::visitTableFill(TableFill* curr) { curr->finalize(); }
void ReFinalize::visitTableCopy(TableCopy* curr) { curr->finalize(); }
void ReFinalize::visitTry(Try* curr) { curr->finalize(); }
void ReFinalize::visitTryTable(TryTable* curr) { curr->finalize(); }
void ReFinalize::visitThrow(Throw* curr) { curr->finalize(); }
void ReFinalize::visitRethrow(Rethrow* curr) { curr->finalize(); }
void ReFinalize::visitThrowRef(ThrowRef* curr) { curr->finalize(); }
void ReFinalize::visitNop(Nop* curr) { curr->finalize(); }
void ReFinalize::visitUnreachable(Unreachable* curr) { curr->finalize(); }
void ReFinalize::visitPop(Pop* curr) { curr->finalize(); }
Expand Down
11 changes: 11 additions & 0 deletions src/ir/branch-utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ void operateOnScopeNameUsesAndSentTypes(Expression* expr, T func) {
func(name, sw->value ? sw->value->type : Type::none);
} else if (auto* br = expr->dynCast<BrOn>()) {
func(name, br->getSentType());
} else if (auto* tt = expr->dynCast<TryTable>()) {
for (Index i = 0; i < tt->catchTags.size(); i++) {
auto dest = tt->catchDests[i];
if (dest == name) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this comparing to name? The name should only be provided to func (which can modify it) IIUC.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry I think I was confused. Fixed it.

Copy link
Member Author

Choose a reason for hiding this comment

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

No I think the original code was correct... name given here is a block label, and we should run func only when the name is the label we are targeting. For example:

(block $l-catch-ref (result i32 exnref)
  ...
  (block $l-catch (result i32)
    ...
    (try_table (catch $e-i32 $l-catch) (catch_ref $e-i32 $l-catch-ref)
      ...
    )
  )
)

In this case, when analyzing TryTable, we shouldn't send both i32 and (i32 exnref) to a given name, which will mean the given name has two different types. We send the type only when the given name matches its sentType's label.

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks. So this is called multiple times for a single TryTable, once per name, and we need to handle the right name each time.

Maybe worth handling TryTable outside of the operateOnScopeNameUses to avoid the potentially quadratic work? But if we think that will be incredibly rare maybe just a comment is enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

Even though theoretically there can be any number of catches, in practice there is no reason to use more than two (one for one of catch/catch_ref and the other for one of catch_all/catch_all_ref), unless you are trying to catch multiple language tags. And in LLVM we don't even generate two catches: We only generate one catch or catch_all for a single try due to the structure of Windows EH IR, and I think this will be also the case for the new spec. Even though we assume a non-LLVM multi-language handling case I doubt this will go more than 3.

func(name, tt->sentTypes[i]);
}
}
} else {
assert(expr->is<Try>() || expr->is<Rethrow>()); // delegate or rethrow
}
Expand All @@ -97,6 +104,10 @@ void operateOnScopeNameUsesAndSentValues(Expression* expr, T func) {
func(name, sw->value);
} else if (auto* br = expr->dynCast<BrOn>()) {
func(name, br->ref);
} else if (auto* tt = expr->dynCast<TryTable>()) {
// The values are supplied by throwing instructions, so we are unable to
// know what they will be here.
func(name, nullptr);
Copy link
Member

Choose a reason for hiding this comment

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

Before, nullptr meant "no value" but now it can also be "there is a value, but it is not known statically", which worried me initially but after reading the uses in Heap2Local and possible-contents.cpp that looks ok. IIUC these branches are from throws which we can't identify statically (and might even be in another function). Please update the comment on line 92, though, to document this API change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

} else {
assert(expr->is<Try>() || expr->is<Rethrow>()); // delegate or rethrow
}
Expand Down
5 changes: 5 additions & 0 deletions src/ir/cost.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,10 @@ struct CostAnalyzer : public OverriddenVisitor<CostAnalyzer, CostType> {
// We assume no exception will be thrown in most cases
return visit(curr->body);
}
CostType visitTryTable(TryTable* curr) {
// We assume no exception will be thrown in most cases
return visit(curr->body);
}
CostType visitThrow(Throw* curr) {
CostType ret = Unacceptable;
for (auto* child : curr->operands) {
Expand All @@ -592,6 +596,7 @@ struct CostAnalyzer : public OverriddenVisitor<CostAnalyzer, CostType> {
return ret;
}
CostType visitRethrow(Rethrow* curr) { return Unacceptable; }
CostType visitThrowRef(ThrowRef* curr) { return Unacceptable; }
CostType visitTupleMake(TupleMake* curr) {
CostType ret = 0;
for (auto* child : curr->operands) {
Expand Down
10 changes: 10 additions & 0 deletions src/ir/effects.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,11 @@ class EffectAnalyzer {
parent.delegateTargets.insert(curr->delegateTarget);
}
}
void visitTryTable(TryTable* curr) {
for (auto name : curr->catchDests) {
parent.breakTargets.insert(name);
}
}
void visitThrow(Throw* curr) {
if (parent.tryDepth == 0) {
parent.throws_ = true;
Expand All @@ -715,6 +720,11 @@ class EffectAnalyzer {
if (parent.tryDepth == 0) {
parent.throws_ = true;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems to have truncated Rethrow's handling?

Copy link
Member Author

@aheejin aheejin Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No that was intentional. So current rethrow doesn't have an argument that can be null, so we shouldn't have set implicitTrap. I think this code was added when we had the exnref and rethrow had an exnref as an argument (pre-Sep 2020), and then not deleted after we switched to the version of rethrow that takes an immediate.

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks! Makes sense.

void visitThrowRef(ThrowRef* curr) {
if (parent.tryDepth == 0) {
parent.throws_ = true;
}
// traps when the arg is null
parent.implicitTrap = true;
}
Expand Down
8 changes: 8 additions & 0 deletions src/ir/possible-contents.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,10 @@ struct InfoCollector
#endif
}
}
void visitTryTable(TryTable* curr) {
// TODO: optimize when possible
addRoot(curr);
}
void visitThrow(Throw* curr) {
auto& operands = curr->operands;
if (!isRelevant(operands)) {
Expand All @@ -1165,6 +1169,10 @@ struct InfoCollector
}
}
void visitRethrow(Rethrow* curr) {}
void visitThrowRef(ThrowRef* curr) {
// TODO: optimize when possible
addRoot(curr);
Copy link
Member

Choose a reason for hiding this comment

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

I think this is unreachable, like rethrow? So there is no value to add a root here for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed it, thanks.

Copy link
Member

Choose a reason for hiding this comment

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

I still see addRoot here - maybe a change wasn't pushed?

Copy link
Member

Choose a reason for hiding this comment

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

Wait, sorry, I had to refresh...

}

void visitTupleMake(TupleMake* curr) {
if (isRelevant(curr->type)) {
Expand Down
6 changes: 3 additions & 3 deletions src/ir/properties.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ inline bool isSymmetric(Binary* binary) {

inline bool isControlFlowStructure(Expression* curr) {
return curr->is<Block>() || curr->is<If>() || curr->is<Loop>() ||
curr->is<Try>();
curr->is<Try>() || curr->is<TryTable>();
}

// Check if an expression is a control flow construct with a name, which implies
Expand Down Expand Up @@ -478,8 +478,8 @@ inline bool isResultFallthrough(Expression* curr) {
// unreachable, for example, but then there is no meaningful answer to give
// anyhow.
return curr->is<LocalSet>() || curr->is<Block>() || curr->is<If>() ||
curr->is<Loop>() || curr->is<Try>() || curr->is<Select>() ||
curr->is<Break>();
curr->is<Loop>() || curr->is<Try>() || curr->is<TryTable>() ||
curr->is<Select>() || curr->is<Break>();
}

inline bool canEmitSelectWithArms(Expression* ifTrue, Expression* ifFalse) {
Expand Down
2 changes: 2 additions & 0 deletions src/ir/subtype-exprs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
self()->noteSubtype(body, curr);
}
}
void visitTryTable(TryTable* curr) { self()->noteSubtype(curr->body, curr); }
void visitThrow(Throw* curr) {
Type params = self()->getModule()->getTag(curr->tag)->sig.params;
assert(params.size() == curr->operands.size());
Expand All @@ -220,6 +221,7 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
}
}
void visitRethrow(Rethrow* curr) {}
void visitThrowRef(ThrowRef* curr) {}
void visitTupleMake(TupleMake* curr) {}
void visitTupleExtract(TupleExtract* curr) {}
void visitRefI31(RefI31* curr) {}
Expand Down
5 changes: 5 additions & 0 deletions src/parser/parsers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ template<typename Ctx> Result<> makeTableFill(Ctx&, Index);
template<typename Ctx> Result<> makeTableCopy(Ctx&, Index);
template<typename Ctx> Result<> makeThrow(Ctx&, Index);
template<typename Ctx> Result<> makeRethrow(Ctx&, Index);
template<typename Ctx> Result<> makeThrowRef(Ctx&, Index);
template<typename Ctx> Result<> makeTupleMake(Ctx&, Index);
template<typename Ctx> Result<> makeTupleExtract(Ctx&, Index);
template<typename Ctx> Result<> makeTupleDrop(Ctx&, Index);
Expand Down Expand Up @@ -1511,6 +1512,10 @@ template<typename Ctx> Result<> makeRethrow(Ctx& ctx, Index pos) {
return ctx.makeRethrow(pos, *label);
}

template<typename Ctx> Result<> makeThrowRef(Ctx& ctx, Index pos) {
return ctx.in.err("unimplemented instruction");
}

template<typename Ctx> Result<> makeTupleMake(Ctx& ctx, Index pos) {
return ctx.in.err("unimplemented instruction");
}
Expand Down
34 changes: 34 additions & 0 deletions src/passes/Print.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
void visitIf(If* curr);
void visitLoop(Loop* curr);
void visitTry(Try* curr);
void visitTryTable(TryTable* curr);
void maybePrintUnreachableReplacement(Expression* curr, Type type);
void maybePrintUnreachableOrNullReplacement(Expression* curr, Type type);
void visitCallRef(CallRef* curr) {
Expand Down Expand Up @@ -1974,6 +1975,25 @@ struct PrintExpressionContents
printBlockType(Signature(Type::none, curr->type));
}
}
void visitTryTable(TryTable* curr) {
printMedium(o, "try_table");
if (curr->type.isConcrete()) {
o << ' ';
printBlockType(Signature(Type::none, curr->type));
}
for (Index i = 0; i < curr->catchTags.size(); i++) {
o << " (";
if (curr->catchTags[i]) {
printMedium(o, curr->catchRefs[i] ? "catch_ref " : "catch ");
printName(curr->catchTags[i], o);
o << ' ';
} else {
printMedium(o, curr->catchRefs[i] ? "catch_all_ref " : "catch_all ");
}
printName(curr->catchDests[i], o);
o << ')';
}
}
void visitThrow(Throw* curr) {
printMedium(o, "throw ");
printName(curr->tag, o);
Expand All @@ -1982,6 +2002,7 @@ struct PrintExpressionContents
printMedium(o, "rethrow ");
printName(curr->target, o);
}
void visitThrowRef(ThrowRef* curr) { printMedium(o, "throw_ref "); }
void visitNop(Nop* curr) { printMinor(o, "nop"); }
void visitUnreachable(Unreachable* curr) { printMinor(o, "unreachable"); }
void visitPop(Pop* curr) {
Expand Down Expand Up @@ -2728,6 +2749,19 @@ void PrintSExpression::visitTry(Try* curr) {
}
}

void PrintSExpression::visitTryTable(TryTable* curr) {
controlFlowDepth++;
o << '(';
printExpressionContents(curr);
incIndent();
maybePrintImplicitBlock(curr->body, true);
decIndent();
if (full) {
o << " ;; end if";
}
controlFlowDepth--;
}

void PrintSExpression::maybePrintUnreachableReplacement(Expression* curr,
Type type) {
// See the parallel function
Expand Down
2 changes: 2 additions & 0 deletions src/passes/TypeGeneralizing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,10 @@ struct TransferFn : OverriddenVisitor<TransferFn> {
}

void visitTry(Try* curr) { WASM_UNREACHABLE("TODO"); }
void visitTryTable(TryTable* curr) { WASM_UNREACHABLE("TODO"); }
void visitThrow(Throw* curr) { WASM_UNREACHABLE("TODO"); }
void visitRethrow(Rethrow* curr) { WASM_UNREACHABLE("TODO"); }
void visitThrowRef(ThrowRef* curr) { WASM_UNREACHABLE("TODO"); }
void visitTupleMake(TupleMake* curr) { WASM_UNREACHABLE("TODO"); }
void visitTupleExtract(TupleExtract* curr) { WASM_UNREACHABLE("TODO"); }

Expand Down
12 changes: 10 additions & 2 deletions src/wasm-binary.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,11 +1170,17 @@ enum ASTNodes {
// exception handling opcodes

Try = 0x06,
Catch = 0x07,
CatchAll = 0x19,
Catch_P3 = 0x07, // Old Phase 3 'catch'
CatchAll_P3 = 0x19, // Old Phase 3 'catch_all'
Delegate = 0x18,
Throw = 0x08,
Rethrow = 0x09,
TryTable = 0x1f,
Catch = 0x00,
CatchRef = 0x01,
CatchAll = 0x02,
CatchAllRef = 0x03,
ThrowRef = 0x0a,

// typed function references opcodes

Expand Down Expand Up @@ -1909,8 +1915,10 @@ class WasmBinaryReader {
void visitTableGet(TableGet* curr);
void visitTableSet(TableSet* curr);
void visitTryOrTryInBlock(Expression*& out);
void visitTryTable(TryTable* curr);
void visitThrow(Throw* curr);
void visitRethrow(Rethrow* curr);
void visitThrowRef(ThrowRef* curr);
void visitCallRef(CallRef* curr);
void visitRefAsCast(RefCast* curr, uint32_t code);
void visitRefAs(RefAs* curr, uint8_t code);
Expand Down
25 changes: 25 additions & 0 deletions src/wasm-builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,31 @@ class Builder {
Try* makeTry(Name name, Expression* body, Name delegateTarget, Type type) {
return makeTry(name, body, {}, {}, delegateTarget, type, true);
}
TryTable* makeTryTable(Expression* body,
const std::vector<Name>& catchTags,
const std::vector<Name>& catchDests,
const std::vector<bool>& catchRefs) {
auto* ret = wasm.allocator.alloc<TryTable>();
ret->body = body;
ret->catchTags.set(catchTags);
ret->catchDests.set(catchDests);
ret->catchRefs.set(catchRefs);
ret->finalize(&wasm);
return ret;
}
TryTable* makeTryTable(Expression* body,
const std::vector<Name>& catchTags,
const std::vector<Name>& catchDests,
const std::vector<bool>& catchRefs,
Type type) {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could unify pairs of functions like this using std::optional<Type> type = std::nullopt? Anyhow, that's separate from this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will try that in a follow-up.

auto* ret = wasm.allocator.alloc<TryTable>();
ret->body = body;
ret->catchTags.set(catchTags);
ret->catchDests.set(catchDests);
ret->catchRefs.set(catchRefs);
ret->finalize(type, &wasm);
return ret;
}
Throw* makeThrow(Tag* tag, const std::vector<Expression*>& args) {
return makeThrow(tag->name, args);
}
Expand Down
Loading