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

Skip nop instructions #638

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Skip nop instructions #638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
433db1c
Skip nop instructions
chfast Oct 20, 2020
7a3936e
Skip block and loop instructions
chfast Oct 20, 2020
b15a01f
Do not omit end instructions
chfast Oct 20, 2020
d733ace
Skip end instructions
chfast Oct 20, 2020
51d2580
Simplify end implementation
chfast Oct 20, 2020
da8c473
test: Update end-related tests
chfast Nov 12, 2020
2bdbbc5
test: Add a test where only final end is produces in parser
chfast Nov 12, 2020
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
12 changes: 3 additions & 9 deletions lib/fizzy/execute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,10 +528,6 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
{
case Instr::unreachable:
goto trap;
case Instr::nop:
case Instr::block:
case Instr::loop:
break;
case Instr::if_:
{
if (stack.pop().as<uint32_t>() != 0)
Expand All @@ -553,14 +549,12 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
}
case Instr::end:
{
// End execution if it's a final end instruction.
if (pc == &code.instructions[code.instructions.size()])
goto end;
break;
assert(pc == &code.instructions[code.instructions.size()]);
goto end;
}
case Instr::br:
case Instr::br_if:
case Instr::return_:
case Instr::return_: // TODO: Replace return with br
{
const auto arity = read<uint32_t>(pc);

Expand Down
24 changes: 14 additions & 10 deletions lib/fizzy/parser_expr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
}

case Instr::nop:
continue;

case Instr::i32_eqz:
case Instr::i32_eq:
case Instr::i32_ne:
Expand Down Expand Up @@ -462,7 +464,7 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
// Push label with immediates offset after arity.
control_stack.emplace(Instr::block, block_type, static_cast<int>(operand_stack.size()),
code.instructions.size());
break;
continue;
}

case Instr::loop:
Expand All @@ -472,7 +474,7 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f

control_stack.emplace(Instr::loop, loop_type, static_cast<int>(operand_stack.size()),
code.instructions.size());
break;
continue;
}

case Instr::if_:
Expand Down Expand Up @@ -529,12 +531,10 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f

if (frame.instruction != Instr::loop) // If end of block/if/else instruction.
{
// In case it's an outermost implicit function block,
// we want br to jump to the final end of the function.
// Otherwise jump to the next instruction after block's end.
const auto target_pc = control_stack.size() == 1 ?
static_cast<uint32_t>(code.instructions.size()) :
static_cast<uint32_t>(code.instructions.size() + 1);
// The position of the "end":
// for the outermost implicit function block this is the function's end instruction,
// otherwise this is the next instruction after the block's end.
const auto target_pc = static_cast<uint32_t>(code.instructions.size());

if (frame.instruction == Instr::if_ || frame.instruction == Instr::else_)
{
Expand All @@ -558,10 +558,14 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
control_stack.pop(); // Pop the current frame.

if (control_stack.empty())
{
continue_parsing = false;
else if (frame_type.has_value())
break;
}

if (frame_type.has_value())
push_operand(operand_stack, *frame_type);
break;
continue;
}

case Instr::br:
Expand Down
157 changes: 87 additions & 70 deletions test/unittests/parser_expr_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,25 @@ TEST(parser_expr, instr_loop)
{
const auto loop_void = "03400b0b"_bytes;
const auto [code1, pos1] = parse_expr(loop_void);
EXPECT_THAT(code1.instructions, ElementsAre(Instr::loop, Instr::end, Instr::end));
EXPECT_THAT(code1.instructions, ElementsAre(Instr::end));
EXPECT_EQ(code1.max_stack_height, 0);

const auto loop_i32 = "037f41000b1a0b"_bytes;
const auto [code2, pos2] = parse_expr(loop_i32);
EXPECT_THAT(code2.instructions, ElementsAre(Instr::loop, Instr::i32_const, 0, 0, 0, 0,
Instr::end, Instr::drop, Instr::end));
EXPECT_THAT(
code2.instructions, ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::drop, Instr::end));
EXPECT_EQ(code2.max_stack_height, 1);

const auto loop_f32 = "037d43000000000b1a0b"_bytes;
const auto [code3, pos3] = parse_expr(loop_f32);
EXPECT_THAT(code3.instructions, ElementsAre(Instr::loop, Instr::f32_const, 0, 0, 0, 0,
Instr::end, Instr::drop, Instr::end));
EXPECT_THAT(
code3.instructions, ElementsAre(Instr::f32_const, 0, 0, 0, 0, Instr::drop, Instr::end));
EXPECT_EQ(code3.max_stack_height, 1);

const auto loop_f64 = "037c4400000000000000000b1a0b"_bytes;
const auto [code4, pos4] = parse_expr(loop_f64);
EXPECT_THAT(code4.instructions, ElementsAre(Instr::loop, Instr::f64_const, 0, 0, 0, 0, 0, 0, 0,
0, Instr::end, Instr::drop, Instr::end));
EXPECT_THAT(code4.instructions,
ElementsAre(Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::drop, Instr::end));
EXPECT_EQ(code4.max_stack_height, 1);
}

Expand All @@ -64,18 +64,17 @@ TEST(parser_expr, instr_block)

const auto empty = "010102400b0b"_bytes;
const auto [code1, pos1] = parse_expr(empty);
EXPECT_THAT(code1.instructions,
ElementsAre(Instr::nop, Instr::nop, Instr::block, Instr::end, Instr::end));
EXPECT_THAT(code1.instructions, ElementsAre(Instr::end));

const auto block_i64 = "027e42000b1a0b"_bytes;
const auto [code2, pos2] = parse_expr(block_i64);
EXPECT_THAT(code2.instructions, ElementsAre(Instr::block, Instr::i64_const, 0, 0, 0, 0, 0, 0, 0,
0, Instr::end, Instr::drop, Instr::end));
EXPECT_THAT(code2.instructions,
ElementsAre(Instr::i64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::drop, Instr::end));

const auto block_f64 = "027c4400000000000000000b1a0b"_bytes;
const auto [code3, pos3] = parse_expr(block_f64);
EXPECT_THAT(code3.instructions, ElementsAre(Instr::block, Instr::f64_const, 0, 0, 0, 0, 0, 0, 0,
0, Instr::end, Instr::drop, Instr::end));
EXPECT_THAT(code3.instructions,
ElementsAre(Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::drop, Instr::end));
}

TEST(parser_expr, instr_block_input_buffer_overflow)
Expand All @@ -94,8 +93,8 @@ TEST(parser_expr, loop_br)
const auto module = parse(wasm);

EXPECT_THAT(module->codesec[0].instructions,
ElementsAre(Instr::loop, Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 0, 0, 0, 0,
/*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end));
ElementsAre(Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 0, 0, 0, 0,
/*stack_drop:*/ 0, 0, 0, 0, Instr::end));

/* wat2wasm
(func
Expand All @@ -109,9 +108,8 @@ TEST(parser_expr, loop_br)
const auto module_parent_stack = parse(wasm_parent_stack);

EXPECT_THAT(module_parent_stack->codesec[0].instructions,
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::loop, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 5, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop,
Instr::end));
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 5, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::drop, Instr::end));

/* wat2wasm
(func
Expand All @@ -127,9 +125,8 @@ TEST(parser_expr, loop_br)
const auto module_arity = parse(wasm_arity);

EXPECT_THAT(module_arity->codesec[0].instructions,
ElementsAre(Instr::loop, Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 0, 0, 0, 0, /*stack_drop:*/ 1, 0, 0, 0, Instr::end, Instr::drop,
Instr::end));
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 0, 0, 0, 0, /*stack_drop:*/ 1, 0, 0, 0, Instr::drop, Instr::end));
}

TEST(parser_expr, loop_return)
Expand All @@ -140,10 +137,9 @@ TEST(parser_expr, loop_return)
const auto wasm = from_hex("0061736d01000000010401600000030201000a0801060003400f0b0b");
const auto module = parse(wasm);

EXPECT_THAT(
module->codesec[0].instructions, ElementsAre(Instr::loop, Instr::return_,
/*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 15, 0, 0, 0,
/*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end));
EXPECT_THAT(module->codesec[0].instructions,
ElementsAre(Instr::return_, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 13, 0, 0, 0,
/*stack_drop:*/ 0, 0, 0, 0, Instr::end));
}

TEST(parser_expr, block_br)
Expand All @@ -162,12 +158,11 @@ TEST(parser_expr, block_br)

const auto code_bin = "010240410a21010c00410b21010b20011a0b"_bytes;
const auto [code, pos] = parse_expr(code_bin, 0, {{2, ValType::i32}});
EXPECT_THAT(
code.instructions, ElementsAre(Instr::nop, Instr::block, Instr::i32_const, 0x0a, 0, 0, 0,
Instr::local_set, 1, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 36, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
Instr::i32_const, 0x0b, 0, 0, 0, Instr::local_set, 1, 0, 0, 0,
Instr::end, Instr::local_get, 1, 0, 0, 0, Instr::drop, Instr::end));
EXPECT_THAT(code.instructions,
ElementsAre(Instr::i32_const, 0x0a, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::br,
/*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 33, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
Instr::i32_const, 0x0b, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::local_get, 1, 0,
0, 0, Instr::drop, Instr::end));
EXPECT_EQ(code.max_stack_height, 1);

/* wat2wasm
Expand All @@ -182,9 +177,8 @@ TEST(parser_expr, block_br)
const auto module_parent_stack = parse(wasm_parent_stack);

EXPECT_THAT(module_parent_stack->codesec[0].instructions,
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::block, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 20, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop,
Instr::end));
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 18, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::drop, Instr::end));

/* wat2wasm
(func
Expand All @@ -200,9 +194,8 @@ TEST(parser_expr, block_br)
const auto module_arity = parse(wasm_arity);

EXPECT_THAT(module_arity->codesec[0].instructions,
ElementsAre(Instr::block, Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 20, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop,
Instr::end));
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 18, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::drop, Instr::end));
}

TEST(parser_expr, block_return)
Expand All @@ -214,8 +207,8 @@ TEST(parser_expr, block_return)
const auto module = parse(wasm);

EXPECT_THAT(module->codesec[0].instructions,
ElementsAre(Instr::block, Instr::return_, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 15, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end));
ElementsAre(Instr::return_, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 13, 0, 0, 0,
/*stack_drop:*/ 0, 0, 0, 0, Instr::end));
}

TEST(parser_expr, if_br)
Expand All @@ -230,10 +223,9 @@ TEST(parser_expr, if_br)
const auto module = parse(wasm);

EXPECT_THAT(module->codesec[0].instructions,
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::if_, /*else_offset:*/ 24, 0, 0, 0,
Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 24, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end,
/*24:*/ Instr::end));
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::if_, /*else_offset:*/ 23, 0, 0, 0,
Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 23, 0, 0, 0,
/*stack_drop:*/ 0, 0, 0, 0, /*23:*/ Instr::end));

/* wat2wasm
(func
Expand All @@ -249,9 +241,9 @@ TEST(parser_expr, if_br)

EXPECT_THAT(module_parent_stack->codesec[0].instructions,
ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::i32_const, 0, 0, 0, 0, Instr::if_,
/*else_offset:*/ 29, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 29, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*29:*/ Instr::end, Instr::drop, Instr::end));
/*else_offset:*/ 28, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 28, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*28:*/ Instr::drop, Instr::end));
}

TEST(parser_expr, instr_br_table)
Expand Down Expand Up @@ -286,27 +278,26 @@ TEST(parser_expr, instr_br_table)
const auto& code = module->codesec[0];

EXPECT_THAT(code.instructions,
ElementsAre(Instr::block, Instr::block, Instr::block, Instr::block, Instr::block,
Instr::local_get, 0, 0, 0, 0, Instr::br_table,
ElementsAre(Instr::local_get, 0, 0, 0, 0, Instr::br_table,
/*label_count:*/ 4, 0, 0, 0, /*arity:*/ 0, 0, 0, 0,
/*code_offset:*/ 135, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*code_offset:*/ 116, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*code_offset:*/ 97, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*code_offset:*/ 78, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,

/*59:*/ Instr::i32_const, 0x41, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end,
/*78:*/ Instr::i32_const, 0x42, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end,
/*97:*/ Instr::i32_const, 0x43, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end,
/*116:*/ Instr::i32_const, 0x44, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end,
/*135:*/ Instr::i32_const, 0x45, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end,
/*154:*/ Instr::i32_const, 0x46, 0, 0, 0,
/*159:*/ Instr::end));
/*code_offset:*/ 126, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*code_offset:*/ 108, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*code_offset:*/ 90, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*code_offset:*/ 72, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*code_offset:*/ 144, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,

/*54:*/ Instr::i32_const, 0x41, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*72:*/ Instr::i32_const, 0x42, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*90:*/ Instr::i32_const, 0x43, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*108:*/ Instr::i32_const, 0x44, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*126:*/ Instr::i32_const, 0x45, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0,
/*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
/*144:*/ Instr::i32_const, 0x46, 0, 0, 0,
/*149:*/ Instr::end));

EXPECT_EQ(code.max_stack_height, 1);
}
Expand All @@ -330,11 +321,11 @@ TEST(parser_expr, instr_br_table_empty_vector)
const auto& code = module->codesec[0];

EXPECT_THAT(code.instructions,
ElementsAre(Instr::block, Instr::local_get, 0, 0, 0, 0, Instr::br_table,
/*label_count:*/ 0, 0, 0, 0, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 42, 0, 0, 0,
ElementsAre(Instr::local_get, 0, 0, 0, 0, Instr::br_table,
/*label_count:*/ 0, 0, 0, 0, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 40, 0, 0, 0,
/*stack_drop:*/ 0, 0, 0, 0, Instr::i32_const, 0x63, 0, 0, 0, Instr::return_,
/*arity:*/ 1, 0, 0, 0, /*code_offset:*/ 47, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
Instr::end, Instr::i32_const, 0x64, 0, 0, 0, Instr::end));
/*arity:*/ 1, 0, 0, 0, /*code_offset:*/ 45, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0,
Instr::i32_const, 0x64, 0, 0, 0, Instr::end));

EXPECT_EQ(code.max_stack_height, 1);
}
Expand Down Expand Up @@ -517,6 +508,7 @@ TEST(parser_expr, call_1arg_1result)
EXPECT_EQ(module->codesec[0].max_stack_height, 1);
EXPECT_EQ(module->codesec[1].max_stack_height, 1);
}

TEST(parser_expr, call_nonexisting_typeidx)
{
// This creates a wasm module where code[0] has a call instruction calling function[1] which
Expand All @@ -528,3 +520,28 @@ TEST(parser_expr, call_nonexisting_typeidx)

EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "invalid function type index");
}

TEST(parser_expr, nop_like_instructions_are_skipped)
{
/* wat2wasm
(func
nop
(block
nop
(loop
nop
(block nop)
nop
)
nop
)
nop
)
*/
const auto wasm = from_hex(
"0061736d01000000010401600000030201000a14011200010240010340010240010b010b010b010b");

const auto module = parse(wasm);
ASSERT_EQ(module->codesec.size(), 1);
EXPECT_THAT(module->codesec[0].instructions, ElementsAre(Instr::end));
}
2 changes: 1 addition & 1 deletion test/unittests/parser_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,7 @@ TEST(parser, code_section_with_basic_instructions)
EXPECT_EQ(module->codesec[0].local_count, 4);
EXPECT_THAT(module->codesec[0].instructions,
ElementsAre(Instr::local_get, 1, 0, 0, 0, Instr::i32_const, 2, 0, 0, 0, Instr::i32_add,
Instr::local_set, 3, 0, 0, 0, Instr::nop, Instr::unreachable, Instr::end));
Instr::local_set, 3, 0, 0, 0, Instr::unreachable, Instr::end));
}

TEST(parser, code_section_with_memory_size)
Expand Down