Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
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
23 changes: 20 additions & 3 deletions src/passes/Outlining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,23 @@ struct ReconstructStringifyWalker
} else if (auto curr = reason.getLoopStart()) {
ODBG(desc = "Loop Start at ");
ASSERT_OK(existingBuilder.visitLoopStart(curr->loop));
} else if (auto curr = reason.getTryStart()) {
// We preserve the name of the tryy because IRBuilder expects
// visitTryStart() to be called on an empty Try, during the normal case of
// parsing.
auto name = curr->tryy->name;
ASSERT_OK(existingBuilder.visitTryStart(curr->tryy, Name()));
DBG(desc = "Try Start at ");
curr->tryy->name = name;
} else if (auto curr = reason.getCatchStart()) {
ASSERT_OK(existingBuilder.visitCatch(curr->tag));
DBG(desc = "Catch Start at ");
} else if (reason.getCatchAllStart()) {
ASSERT_OK(existingBuilder.visitCatchAll());
DBG(desc = "Catch All Start at");
} else if (auto curr = reason.getTryTableStart()) {
ODBG(desc = "Try Table Start at ");
ASSERT_OK(existingBuilder.visitTryTableStart(curr->tryt));
} else if (reason.getEnd()) {
ODBG(desc = "End at ");
ASSERT_OK(existingBuilder.visitEnd());
Expand Down Expand Up @@ -332,9 +349,9 @@ struct Outlining : public Pass {
substrings = StringifyProcessor::dedupe(substrings);
// Remove substrings with overlapping indices.
substrings = StringifyProcessor::filterOverlaps(substrings);
// Remove substrings with branch and return instructions until an analysis
// is performed to see if the intended destination of the branch is included
// in the substring to be outlined.
// Remove substrings with branch, return, and try_table instructions until
// an analysis is performed to see if the intended destination of the branch
// is included in the substring to be outlined.
substrings =
StringifyProcessor::filterBranches(substrings, stringify.exprs);
// Remove substrings with local.set instructions until Outlining is extended
Expand Down
3 changes: 2 additions & 1 deletion src/passes/hash-stringify-walker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,8 @@ std::vector<SuffixTree::RepeatedSubstring> StringifyProcessor::filterBranches(
const std::vector<Expression*>& exprs) {
return StringifyProcessor::filter(
substrings, exprs, [](const Expression* curr) {
return Properties::isBranch(curr) || curr->is<Return>();
return Properties::isBranch(curr) || curr->is<Return>() ||
curr->is<TryTable>();
Comment on lines +272 to +273
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@aheejin, separately from this PR, I think it would make sense for Properties::isBranch() to be true for TryTable expressions. Is that something you've thought about?

});
}

Expand Down
23 changes: 17 additions & 6 deletions src/passes/stringify-walker-impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,18 @@ template<typename SubType> void StringifyWalker<SubType>::dequeueControlFlow() {
}
case Expression::Id::TryId: {
auto* tryy = curr->cast<Try>();
addUniqueSymbol(SeparatorReason::makeTryBodyStart());

addUniqueSymbol(SeparatorReason::makeTryStart(tryy));
Super::walk(tryy->body);
addUniqueSymbol(SeparatorReason::makeEnd());
for (auto& child : tryy->catchBodies) {
addUniqueSymbol(SeparatorReason::makeTryCatchStart());
Super::walk(child);
addUniqueSymbol(SeparatorReason::makeEnd());
for (size_t i = 0; i < tryy->catchBodies.size(); i++) {
if (tryy->hasCatchAll() && i == tryy->catchBodies.size() - 1) {
addUniqueSymbol(SeparatorReason::makeCatchAllStart());
} else {
addUniqueSymbol(SeparatorReason::makeCatchStart(tryy->catchTags[i]));
}
Super::walk(tryy->catchBodies[i]);
}
addUniqueSymbol(SeparatorReason::makeEnd());
break;
}
case Expression::Id::LoopId: {
Expand All @@ -114,6 +118,13 @@ template<typename SubType> void StringifyWalker<SubType>::dequeueControlFlow() {
addUniqueSymbol(SeparatorReason::makeEnd());
break;
}
case Expression::Id::TryTableId: {
auto* tryt = curr->cast<TryTable>();
addUniqueSymbol(SeparatorReason::makeTryTableStart(tryt));
Super::walk(tryt->body);
addUniqueSymbol(SeparatorReason::makeEnd());
break;
}
default: {
assert(Properties::isControlFlowStructure(curr));
WASM_UNREACHABLE("unexpected expression");
Expand Down
56 changes: 40 additions & 16 deletions src/passes/stringify-walker.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,19 @@ struct StringifyWalker
Loop* loop;
};

struct TryBodyStart {};
struct TryStart {
Try* tryy;
};

struct CatchStart {
Name tag;
};

struct TryCatchStart {};
struct CatchAllStart {};

struct TryTableStart {
TryTable* tryt;
};

struct End {
Expression* curr;
Expand All @@ -101,8 +111,10 @@ struct StringifyWalker
IfStart,
ElseStart,
LoopStart,
TryBodyStart,
TryCatchStart,
TryStart,
CatchStart,
CatchAllStart,
TryTableStart,
End>;

Separator reason;
Expand All @@ -124,23 +136,31 @@ struct StringifyWalker
static SeparatorReason makeLoopStart(Loop* loop) {
return SeparatorReason(LoopStart{loop});
}
static SeparatorReason makeTryCatchStart() {
return SeparatorReason(TryCatchStart{});
static SeparatorReason makeTryStart(Try* tryy) {
return SeparatorReason(TryStart{tryy});
}
static SeparatorReason makeCatchStart(Name tag) {
return SeparatorReason(CatchStart{tag});
}
static SeparatorReason makeCatchAllStart() {
return SeparatorReason(CatchAllStart{});
}
static SeparatorReason makeTryBodyStart() {
return SeparatorReason(TryBodyStart{});
static SeparatorReason makeTryTableStart(TryTable* tryt) {
return SeparatorReason(TryTableStart{tryt});
}
static SeparatorReason makeEnd() { return SeparatorReason(End{}); }
FuncStart* getFuncStart() { return std::get_if<FuncStart>(&reason); }
BlockStart* getBlockStart() { return std::get_if<BlockStart>(&reason); }
IfStart* getIfStart() { return std::get_if<IfStart>(&reason); }
ElseStart* getElseStart() { return std::get_if<ElseStart>(&reason); }
LoopStart* getLoopStart() { return std::get_if<LoopStart>(&reason); }
TryBodyStart* getTryBodyStart() {
return std::get_if<TryBodyStart>(&reason);
TryStart* getTryStart() { return std::get_if<TryStart>(&reason); }
CatchStart* getCatchStart() { return std::get_if<CatchStart>(&reason); }
CatchAllStart* getCatchAllStart() {
return std::get_if<CatchAllStart>(&reason);
}
TryCatchStart* getTryCatchStart() {
return std::get_if<TryCatchStart>(&reason);
TryTableStart* getTryTableStart() {
return std::get_if<TryTableStart>(&reason);
}
End* getEnd() { return std::get_if<End>(&reason); }
};
Expand All @@ -158,10 +178,14 @@ struct StringifyWalker
return o << "Else Start";
} else if (reason.getLoopStart()) {
return o << "Loop Start";
} else if (reason.getTryBodyStart()) {
return o << "Try Body Start";
} else if (reason.getTryCatchStart()) {
return o << "Try Catch Start";
} else if (reason.getTryStart()) {
return o << "Try Start";
} else if (reason.getCatchStart()) {
return o << "Catch Start";
} else if (reason.getCatchAllStart()) {
return o << "Catch All Start";
} else if (reason.getTryTableStart()) {
return o << "Try Table Start";
} else if (reason.getEnd()) {
return o << "End";
}
Expand Down
28 changes: 22 additions & 6 deletions src/wasm-ir-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -306,14 +306,17 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
struct TryScope {
Try* tryy;
Name originalLabel;
Index index;
};
struct CatchScope {
Try* tryy;
Name originalLabel;
Index index;
};
struct CatchAllScope {
Try* tryy;
Name originalLabel;
Index index;
};
struct TryTableScope {
TryTable* trytable;
Expand Down Expand Up @@ -395,16 +398,17 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
static ScopeCtx makeLoop(Loop* loop, Type inputType) {
return ScopeCtx(LoopScope{loop}, inputType);
}
static ScopeCtx makeTry(Try* tryy, Name originalLabel, Type inputType) {
return ScopeCtx(TryScope{tryy, originalLabel}, inputType);
static ScopeCtx
makeTry(Try* tryy, Name originalLabel, Type inputType, Index index) {
return ScopeCtx(TryScope{tryy, originalLabel, index}, inputType);
}
static ScopeCtx makeCatch(ScopeCtx&& scope, Try* tryy) {
scope.scope = CatchScope{tryy, scope.getOriginalLabel()};
static ScopeCtx makeCatch(ScopeCtx&& scope, Try* tryy, Index index) {
scope.scope = CatchScope{tryy, scope.getOriginalLabel(), index};
scope.resetForDelimiter(/*keepInput=*/false);
return scope;
}
static ScopeCtx makeCatchAll(ScopeCtx&& scope, Try* tryy) {
scope.scope = CatchAllScope{tryy, scope.getOriginalLabel()};
static ScopeCtx makeCatchAll(ScopeCtx&& scope, Try* tryy, Index index) {
scope.scope = CatchAllScope{tryy, scope.getOriginalLabel(), index};
scope.resetForDelimiter(/*keepInput=*/false);
return scope;
}
Expand Down Expand Up @@ -530,6 +534,18 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
}
WASM_UNREACHABLE("unexpected scope kind");
}
Index getIndex() {
if (auto* tryScope = std::get_if<TryScope>(&scope)) {
return tryScope->index;
}
if (auto* catchScope = std::get_if<CatchScope>(&scope)) {
return catchScope->index;
}
if (auto* catchAllScope = std::get_if<CatchAllScope>(&scope)) {
return catchAllScope->index;
}
WASM_UNREACHABLE("unexpected scope kind");
}
Type getLabelType() {
// Loops receive their input type rather than their output type.
return getLoop() ? inputType : getResultType();
Expand Down
29 changes: 22 additions & 7 deletions src/wasm/wasm-ir-builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ Result<> IRBuilder::visitLoopStart(Loop* loop, Type inputType) {

Result<> IRBuilder::visitTryStart(Try* tryy, Name label, Type inputType) {
applyDebugLoc(tryy);
return pushScope(ScopeCtx::makeTry(tryy, label, inputType));
return pushScope(ScopeCtx::makeTry(tryy, label, inputType, 0));
}

Result<>
Expand Down Expand Up @@ -942,21 +942,28 @@ Result<> IRBuilder::visitCatch(Name tag) {
if (!tryy) {
return Err{"unexpected catch"};
}
auto index = scope.getIndex();
auto expr = finishScope();
CHECK_ERR(expr);
if (wasTry) {
tryy->body = *expr;
} else {
tryy->catchBodies.push_back(*expr);
if (tryy->catchBodies.size() < index) {
tryy->catchBodies.resize(tryy->catchBodies.size() + 1);
}
tryy->catchBodies[index - 1] = *expr;
}
if (tryy->catchTags.size() == index) {
tryy->catchTags.resize(tryy->catchTags.size() + 1);
}
tryy->catchTags.push_back(tag);
tryy->catchTags[index] = tag;

if (binaryPos && func) {
auto& delimiterLocs = func->delimiterLocations[tryy];
delimiterLocs[delimiterLocs.size()] = lastBinaryPos - codeSectionOffset;
}

CHECK_ERR(pushScope(ScopeCtx::makeCatch(std::move(scope), tryy)));
CHECK_ERR(pushScope(ScopeCtx::makeCatch(std::move(scope), tryy, ++index)));
// Push a pop for the exception payload if necessary.
auto params = wasm.getTag(tag)->params();
if (params != Type::none) {
Expand All @@ -980,20 +987,24 @@ Result<> IRBuilder::visitCatchAll() {
if (!tryy) {
return Err{"unexpected catch"};
}
auto index = scope.getIndex();
auto expr = finishScope();
CHECK_ERR(expr);
if (wasTry) {
tryy->body = *expr;
} else {
tryy->catchBodies.push_back(*expr);
if (tryy->catchBodies.size() < index) {
tryy->catchBodies.resize(tryy->catchBodies.size() + 1);
}
tryy->catchBodies[index - 1] = *expr;
}

if (binaryPos && func) {
auto& delimiterLocs = func->delimiterLocations[tryy];
delimiterLocs[delimiterLocs.size()] = lastBinaryPos - codeSectionOffset;
}

return pushScope(ScopeCtx::makeCatchAll(std::move(scope), tryy));
return pushScope(ScopeCtx::makeCatchAll(std::move(scope), tryy, ++index));
}

Result<> IRBuilder::visitDelegate(Index label) {
Expand Down Expand Up @@ -1123,7 +1134,11 @@ Result<> IRBuilder::visitEnd() {
push(maybeWrapForLabel(tryy));
} else if (Try * tryy;
(tryy = scope.getCatch()) || (tryy = scope.getCatchAll())) {
tryy->catchBodies.push_back(*expr);
auto index = scope.getIndex();
if (tryy->catchBodies.size() < index) {
tryy->catchBodies.resize(tryy->catchBodies.size() + 1);
}
tryy->catchBodies[index - 1] = *expr;
tryy->name = scope.label;
tryy->finalize(tryy->type);
push(maybeWrapForLabel(tryy));
Expand Down
13 changes: 5 additions & 8 deletions test/gtest/stringify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,16 @@ adding unique symbol for End
adding unique symbol for If Start
in visitExpression for i32.const 30
adding unique symbol for End
adding unique symbol for Try Body Start
adding unique symbol for Try Start
in visitExpression for nop
adding unique symbol for End
adding unique symbol for Try Catch Start
adding unique symbol for Catch Start
in visitExpression for block
adding unique symbol for End
adding unique symbol for Try Catch Start
adding unique symbol for Catch Start
in visitExpression for block
adding unique symbol for End
adding unique symbol for Try Body Start
adding unique symbol for Try Start
in visitExpression for nop
adding unique symbol for End
adding unique symbol for Try Catch Start
adding unique symbol for Catch Start
in visitExpression for block
adding unique symbol for End
adding unique symbol for Block Start
Expand Down
32 changes: 32 additions & 0 deletions test/lit/passes/outlining.wast
Original file line number Diff line number Diff line change
Expand Up @@ -1164,3 +1164,35 @@
)
)
)

;; Tests TryTable instructions are correctly filtered from being outlined.
;; The (drop (i32.const 0)) instructions were added to form an outlineable
;; sequence with the block that contains the try_table.
(module
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like we need to run the test update script to get the output here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

oops, fixed, thanks

(type $0 (func))
(tag $tag$0 (type $0))
(func $a (result (ref exn))
(loop $label1 (result (ref exn))
(drop
(i32.const 0)
)
(block (result (ref exn))
(try_table (catch_all $label1)
(throw $tag$0)
)
)
)
)
(func $b (result (ref exn))
(loop $label1 (result (ref exn))
(drop
(i32.const 0)
)
(block (result (ref exn))
(try_table (catch_all $label1)
(throw $tag$0)
)
)
)
)
)
Loading