Skip to content

[EH] Change Walker::TaskFunc back to function pointer #3899

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

Merged
merged 4 commits into from
May 21, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 15 additions & 13 deletions src/cfg/cfg-traversal.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
// but their usage does not overlap in time, and this is more efficient.
std::vector<std::vector<BasicBlock*>> processCatchStack;

// Stack to store the catch indices within catch bodies. To be used in
// doStartCatch and doEndCatch.
std::vector<Index> catchIndexStack;

BasicBlock* startBasicBlock() {
currBasicBlock = ((SubType*)this)->makeBasicBlock();
basicBlocks.push_back(std::unique_ptr<BasicBlock>(currBasicBlock));
Expand Down Expand Up @@ -304,16 +308,20 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
self->processCatchStack.push_back(self->unwindCatchStack.back());
self->unwindCatchStack.pop_back();
self->unwindExprStack.pop_back();
self->catchIndexStack.push_back(0);
}

static void doStartCatch(SubType* self, Expression** currp, Index i) {
static void doStartCatch(SubType* self, Expression** currp) {
// Get the block that starts this catch
self->currBasicBlock = self->processCatchStack.back()[i];
self->currBasicBlock =
self->processCatchStack.back()[self->catchIndexStack.back()];
}

static void doEndCatch(SubType* self, Expression** currp, Index i) {
static void doEndCatch(SubType* self, Expression** currp) {
// We are done with this catch; set the block that ends it
self->processCatchStack.back()[i] = self->currBasicBlock;
self->processCatchStack.back()[self->catchIndexStack.back()] =
self->currBasicBlock;
self->catchIndexStack.back()++;
}

static void doEndTry(SubType* self, Expression** currp) {
Expand All @@ -326,6 +334,7 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
self->link(self->tryStack.back(), self->currBasicBlock);
self->tryStack.pop_back();
self->processCatchStack.pop_back();
self->catchIndexStack.pop_back();
}

static void doEndThrow(SubType* self, Expression** currp) {
Expand Down Expand Up @@ -381,17 +390,10 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
case Expression::Id::TryId: {
self->pushTask(SubType::doEndTry, currp);
auto& catchBodies = curr->cast<Try>()->catchBodies;
using namespace std::placeholders;
for (Index i = 0; i < catchBodies.size(); i++) {
auto doEndCatchI = [i](SubType* self, Expression** currp) {
doEndCatch(self, currp, i);
};
self->pushTask(doEndCatchI, currp);
self->pushTask(doEndCatch, currp);
self->pushTask(SubType::scan, &catchBodies[i]);
auto doStartCatchI = [i](SubType* self, Expression** currp) {
doStartCatch(self, currp, i);
};
self->pushTask(doStartCatchI, currp);
self->pushTask(doStartCatch, currp);
}
self->pushTask(SubType::doStartCatches, currp);
self->pushTask(SubType::scan, &curr->cast<Try>()->body);
Expand Down
2 changes: 1 addition & 1 deletion src/wasm-traversal.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ struct Walker : public VisitorType {
// nested.

// Tasks receive the this pointer and a pointer to the pointer to operate on
using TaskFunc = std::function<void(SubType*, Expression**)>;
typedef void (*TaskFunc)(SubType*, Expression**);

struct Task {
TaskFunc func;
Expand Down
81 changes: 81 additions & 0 deletions test/passes/rse_all-features.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
(type $i32_i32_=>_none (func (param i32 i32)))
(type $i32_f64_=>_none (func (param i32 f64)))
(event $e (attr 0) (param i32))
(event $e2 (attr 0) (param))
(func $basic (param $x i32) (param $y f64)
(local $a f32)
(local $b i64)
Expand Down Expand Up @@ -645,4 +646,84 @@
(i32.const 1)
)
)
(func $nested-catch1
(local $x i32)
(try $try
(do
(throw $e
(i32.const 0)
)
)
(catch $e
(drop
(pop i32)
)
)
(catch $e2
(try $try6
(do
(throw $e
(i32.const 0)
)
)
(catch $e
(drop
(pop i32)
)
)
(catch $e2
(local.set $x
(i32.const 1)
)
)
)
)
)
(local.set $x
(i32.const 1)
)
)
(func $nested-catch2
(local $x i32)
(try $try
(do
(throw $e
(i32.const 0)
)
)
(catch $e
(drop
(pop i32)
)
(local.set $x
(i32.const 1)
)
)
(catch_all
(try $try7
(do
(throw $e
(i32.const 0)
)
)
(catch $e
(drop
(pop i32)
)
(local.set $x
(i32.const 1)
)
)
(catch_all
(local.set $x
(i32.const 1)
)
)
)
)
)
(drop
(i32.const 1)
)
)
)
58 changes: 58 additions & 0 deletions test/passes/rse_all-features.wast
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@
)

(event $e (attr 0) (param i32))
(event $e2 (attr 0))
(func $try1
(local $x i32)
(try
Expand Down Expand Up @@ -416,4 +417,61 @@
;; so the local.set may not run. So this should NOT be dropped.
(local.set $x (i32.const 1))
)
(func $nested-catch1
(local $x i32)
(try
(do
(throw $e (i32.const 0))
)
(catch $e
(drop (pop i32))
)
(catch $e2
(try
(do
(throw $e (i32.const 0))
)
(catch $e
(drop (pop i32))
)
(catch $e2
(local.set $x (i32.const 1))
)
)
)
)
;; This should NOT be dropped because the exception might not be caught by
;; the inner catches, and the local.set above us may not have run, and
;; other possible code paths do not even set the local.
(local.set $x (i32.const 1))
)
(func $nested-catch2
(local $x i32)
(try
(do
(throw $e (i32.const 0))
)
(catch $e
(drop (pop i32))
(local.set $x (i32.const 1))
)
(catch_all
(try
(do
(throw $e (i32.const 0))
)
(catch $e
(drop (pop i32))
(local.set $x (i32.const 1))
)
(catch_all
(local.set $x (i32.const 1))
)
)
)
)
;; This should be dropped because the exception is guaranteed to be caught
;; by one of the catches and it will set the local to 1.
(local.set $x (i32.const 1))
)
)