Skip to content

Commit

Permalink
[Exceptions] Finish interpreter + optimizer support for try_table. (#…
Browse files Browse the repository at this point in the history
…6814)

* Add interpreter support for exnref values.
* Fix optimization passes to support try_table.
* Enable the interpreter (but not in V8, see code) on exceptions.
  • Loading branch information
sjrd authored Aug 20, 2024
1 parent 2c9c74d commit 340ad71
Show file tree
Hide file tree
Showing 41 changed files with 2,764 additions and 420 deletions.
13 changes: 7 additions & 6 deletions scripts/fuzz_opt.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,6 @@ def is_git_repo():
'typed_continuations_contnew.wast',
'typed_continuations_contbind.wast',
'typed_continuations_suspend.wast',
# New EH implementation is in progress
'exception-handling.wast',
'translate-to-new-eh.wast',
'rse-eh.wast',
]


Expand Down Expand Up @@ -841,7 +837,9 @@ def can_run(self, wasm):
# V8 does not support shared memories when running with
# shared-everything enabled, so do not fuzz shared-everything
# for now.
return all_disallowed(['shared-everything'])
# Due to the V8 bug https://issues.chromium.org/issues/332931390
# we do not fuzz exception-handling either.
return all_disallowed(['shared-everything', 'exception-handling'])

def can_compare_to_self(self):
# With nans, VM differences can confuse us, so only very simple VMs
Expand Down Expand Up @@ -1649,6 +1647,9 @@ def get_random_opts():
print('avoiding --flatten due to multivalue + reference types not supporting it (spilling of non-nullable tuples)')
print('TODO: Resolving https://github.com/WebAssembly/binaryen/issues/4824 may fix this')
continue
if '--enable-exception-handling' in FEATURE_OPTS:
print('avoiding --flatten due to exception-handling not supporting it (requires blocks with results)')
continue
if '--gc' not in FEATURE_OPTS:
print('avoiding --flatten due to GC not supporting it (spilling of non-nullable locals)')
continue
Expand Down Expand Up @@ -1707,7 +1708,7 @@ def get_random_opts():
# some features depend on other features, so if a required feature is
# disabled, its dependent features need to be disabled as well.
IMPLIED_FEATURE_OPTS = {
'--disable-reference-types': ['--disable-gc', '--disable-strings'],
'--disable-reference-types': ['--disable-gc', '--disable-exception-handling', '--disable-strings'],
'--disable-gc': ['--disable-strings'],
}

Expand Down
7 changes: 6 additions & 1 deletion src/ir/ReFinalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,12 @@ void ReFinalize::visitTableFill(TableFill* curr) { curr->finalize(); }
void ReFinalize::visitTableCopy(TableCopy* curr) { curr->finalize(); }
void ReFinalize::visitTableInit(TableInit* curr) { curr->finalize(); }
void ReFinalize::visitTry(Try* curr) { curr->finalize(); }
void ReFinalize::visitTryTable(TryTable* curr) { curr->finalize(); }
void ReFinalize::visitTryTable(TryTable* curr) {
curr->finalize();
for (size_t i = 0; i < curr->catchDests.size(); i++) {
updateBreakValueType(curr->catchDests[i], curr->sentTypes[i]);
}
}
void ReFinalize::visitThrow(Throw* curr) { curr->finalize(); }
void ReFinalize::visitRethrow(Rethrow* curr) { curr->finalize(); }
void ReFinalize::visitThrowRef(ThrowRef* curr) { curr->finalize(); }
Expand Down
26 changes: 26 additions & 0 deletions src/ir/effects.h
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,14 @@ class EffectAnalyzer {
self->pushTask(doStartTry, currp);
return;
}
if (auto* tryTable = curr->dynCast<TryTable>()) {
// We need to increment try depth before starting.
self->pushTask(doEndTryTable, currp);
self->pushTask(doVisitTryTable, currp);
self->pushTask(scan, &tryTable->body);
self->pushTask(doStartTryTable, currp);
return;
}
PostWalker<InternalAnalyzer, OverriddenVisitor<InternalAnalyzer>>::scan(
self, currp);
}
Expand Down Expand Up @@ -472,6 +480,24 @@ class EffectAnalyzer {
self->parent.catchDepth--;
}

static void doStartTryTable(InternalAnalyzer* self, Expression** currp) {
auto* curr = (*currp)->cast<TryTable>();
// We only count 'try_table's with a 'catch_all' because instructions
// within a 'try_table' without a 'catch_all' can still throw outside of
// the try.
if (curr->hasCatchAll()) {
self->parent.tryDepth++;
}
}

static void doEndTryTable(InternalAnalyzer* self, Expression** currp) {
auto* curr = (*currp)->cast<TryTable>();
if (curr->hasCatchAll()) {
assert(self->parent.tryDepth > 0 && "try depth cannot be negative");
self->parent.tryDepth--;
}
}

void visitBlock(Block* curr) {
if (curr->name.is()) {
parent.breakTargets.erase(curr->name); // these were internal breaks
Expand Down
6 changes: 6 additions & 0 deletions src/ir/linear-execution.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ struct LinearExecutionWalker : public PostWalker<SubType, VisitorType> {
self->pushTask(SubType::scan, &curr->cast<Try>()->body);
break;
}
case Expression::Id::TryTableId: {
self->pushTask(SubType::doVisitTryTable, currp);
self->pushTask(SubType::doNoteNonLinear, currp);
self->pushTask(SubType::scan, &curr->cast<TryTable>()->body);
break;
}
case Expression::Id::ThrowId: {
self->pushTask(SubType::doVisitThrow, currp);
self->pushTask(SubType::doNoteNonLinear, currp);
Expand Down
34 changes: 32 additions & 2 deletions src/ir/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1135,8 +1135,38 @@ struct InfoCollector
}
}
void visitTryTable(TryTable* curr) {
// TODO: optimize when possible
addRoot(curr);
receiveChildValue(curr->body, curr);

// Connect caught tags with their branch targets, and materialize non-null
// exnref values.
auto numTags = curr->catchTags.size();
for (Index tagIndex = 0; tagIndex < numTags; tagIndex++) {
auto tag = curr->catchTags[tagIndex];
auto target = curr->catchDests[tagIndex];

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

for (Index i = 0; i < params.size(); i++) {
if (isRelevant(params[i])) {
info.links.push_back(
{TagLocation{tag, i},
BreakTargetLocation{getFunction(), target, i}});
}
}

exnrefIndex = params.size();
}

if (curr->catchRefs[tagIndex]) {
auto location = CaughtExnRefLocation{};
addRoot(location,
PossibleContents::fromType(Type(HeapType::exn, NonNullable)));
info.links.push_back(
{location, BreakTargetLocation{getFunction(), target, exnrefIndex}});
}
}
}
void visitThrow(Throw* curr) {
auto& operands = curr->operands;
Expand Down
16 changes: 16 additions & 0 deletions src/ir/possible-contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,15 @@ struct TagLocation {
}
};

// The location of an exnref materialized by a catch_ref or catch_all_ref clause
// of a try_table. No data is stored here. exnrefs contain a tag and a payload
// at run-time, as well as potential metadata such as stack traces, but we don't
// track that. So this is the same as NullLocation in a way: we just need *a*
// source of contents for places that receive an exnref.
struct CaughtExnRefLocation {
bool operator==(const CaughtExnRefLocation& other) const { return true; }
};

// A null value. This is used as the location of the default value of a var in a
// function, a null written to a struct field in struct.new_with_default, etc.
struct NullLocation {
Expand Down Expand Up @@ -520,6 +529,7 @@ using Location = std::variant<ExpressionLocation,
SignatureResultLocation,
DataLocation,
TagLocation,
CaughtExnRefLocation,
NullLocation,
ConeReadLocation>;

Expand Down Expand Up @@ -608,6 +618,12 @@ template<> struct hash<wasm::TagLocation> {
}
};

template<> struct hash<wasm::CaughtExnRefLocation> {
size_t operator()(const wasm::CaughtExnRefLocation& loc) const {
return std::hash<const char*>()("caught-exnref-location");
}
};

template<> struct hash<wasm::NullLocation> {
size_t operator()(const wasm::NullLocation& loc) const {
return std::hash<wasm::Type>{}(loc.type);
Expand Down
19 changes: 19 additions & 0 deletions src/literal.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace wasm {

class Literals;
struct GCData;
struct ExnData;

class Literal {
// store only integers, whose bits are deterministic. floats
Expand All @@ -44,6 +45,7 @@ class Literal {
int64_t i64;
uint8_t v128[16];
// funcref function name. `isNull()` indicates a `null` value.
// TODO: handle cross-module calls using something other than a Name here.
Name func;
// A reference to GC data, either a Struct or an Array. For both of those we
// store the referred data as a Literals object (which is natural for an
Expand All @@ -56,6 +58,8 @@ class Literal {
// reference as its sole value even though internal i31 references do not
// have a gcData.
std::shared_ptr<GCData> gcData;
// A reference to Exn data.
std::shared_ptr<ExnData> exnData;
};

public:
Expand Down Expand Up @@ -85,6 +89,7 @@ class Literal {
assert(type.isSignature());
}
explicit Literal(std::shared_ptr<GCData> gcData, HeapType type);
explicit Literal(std::shared_ptr<ExnData> exnData);
explicit Literal(std::string_view string);
Literal(const Literal& other);
Literal& operator=(const Literal& other);
Expand All @@ -96,6 +101,7 @@ class Literal {
// Whether this is GC data, that is, something stored on the heap (aside from
// a null or i31). This includes structs, arrays, and also strings.
bool isData() const { return type.isData(); }
bool isExn() const { return type.isExn(); }
bool isString() const { return type.isString(); }

bool isNull() const { return type.isNull(); }
Expand Down Expand Up @@ -303,6 +309,7 @@ class Literal {
return func;
}
std::shared_ptr<GCData> getGCData() const;
std::shared_ptr<ExnData> getExnData() const;

// careful!
int32_t* geti32Ptr() {
Expand Down Expand Up @@ -742,6 +749,18 @@ struct GCData {
GCData(HeapType type, Literals values) : type(type), values(values) {}
};

// The data of a (ref exn) literal.
struct ExnData {
// The tag of this exn data.
// TODO: handle cross-module calls using something other than a Name here.
Name tag;

// The payload of this exn data.
Literals payload;

ExnData(Name tag, Literals payload) : tag(tag), payload(payload) {}
};

} // namespace wasm

namespace std {
Expand Down
21 changes: 12 additions & 9 deletions src/passes/CodeFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,15 +343,18 @@ struct CodeFolding : public WalkerPass<ControlFlowWalker<CodeFolding>> {
if (effects.danglingPop) {
return false;
}
// When an expression can throw and it is within a try scope, taking it
// out of the try scope changes the program's behavior, because the
// expression that would otherwise have been caught by the try now
// throws up to the next try scope or even up to the caller. We restrict
// the move if 'outOf' contains a 'try' anywhere in it. This is a
// conservative approximation because there can be cases that 'try' is
// within the expression that may throw so it is safe to take the
// expression out.
if (effects.throws() && !FindAll<Try>(outOf).list.empty()) {
// When an expression can throw and it is within a try/try_table scope,
// taking it out of the try/try_table scope changes the program's
// behavior, because the expression that would otherwise have been
// caught by the try/try_table now throws up to the next try/try_table
// scope or even up to the caller. We restrict the move if 'outOf'
// contains a 'try' or 'try_table' anywhere in it. This is a
// conservative approximation because there can be cases that
// 'try'/'try_table' is within the expression that may throw so it is
// safe to take the expression out.
// TODO: optimize this check to avoid two FindAlls.
if (effects.throws() &&
(FindAll<Try>(outOf).has() || FindAll<TryTable>(outOf).has())) {
return false;
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/passes/DeadCodeElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,12 @@ struct DeadCodeElimination
tryy->body->type == Type::unreachable && allCatchesUnreachable) {
typeUpdater.changeType(tryy, Type::unreachable);
}
} else if (auto* tryTable = curr->dynCast<TryTable>()) {
// try_table can finish normally only if its body finishes normally.
if (tryTable->type != Type::unreachable &&
tryTable->body->type == Type::unreachable) {
typeUpdater.changeType(tryTable, Type::unreachable);
}
} else {
WASM_UNREACHABLE("unimplemented DCE control flow structure");
}
Expand Down
6 changes: 3 additions & 3 deletions src/passes/SimplifyLocals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,9 @@ struct SimplifyLocals
Expression** currp) {
Expression* curr = *currp;

// Certain expressions cannot be sinked into 'try', and so at the start of
// 'try' we forget about them.
if (curr->is<Try>()) {
// Certain expressions cannot be sinked into 'try'/'try_table', and so at
// the start of 'try'/'try_table' we forget about them.
if (curr->is<Try>() || curr->is<TryTable>()) {
std::vector<Index> invalidated;
for (auto& [index, info] : self->sinkables) {
// Expressions that may throw cannot be moved into a try (which might
Expand Down
11 changes: 10 additions & 1 deletion src/passes/Vacuum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
// Some instructions have special handling in visit*, and we should do
// nothing for them here.
if (curr->is<Drop>() || curr->is<Block>() || curr->is<If>() ||
curr->is<Loop>() || curr->is<Try>()) {
curr->is<Loop>() || curr->is<Try>() || curr->is<TryTable>()) {
return curr;
}
// Check if this expression itself has side effects, ignoring children.
Expand Down Expand Up @@ -435,6 +435,15 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
}
}

void visitTryTable(TryTable* curr) {
// If try_table's body does not throw, the whole try_table can be replaced
// with the try_table's body.
if (!EffectAnalyzer(getPassOptions(), *getModule(), curr->body).throws()) {
replaceCurrent(curr->body);
return;
}
}

void visitFunction(Function* curr) {
auto* optimized =
optimize(curr->body, curr->getResults() != Type::none, true);
Expand Down
Loading

0 comments on commit 340ad71

Please sign in to comment.