Skip to content

Commit

Permalink
Move branch immediate values to the instructions array
Browse files Browse the repository at this point in the history 
This also drop the immediates array as no longer needed.
  • Loading branch information
@chfast
chfast committed Oct 22, 2020
1 parent 35b4c36 commit 656e46d
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 213 deletions.
37 changes: 13 additions & 24 deletions lib/fizzy/execute.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ namespace fizzy
{
namespace
{
// code_offset + imm_offset + stack_height
constexpr auto BranchImmediateSize = 3 * sizeof(uint32_t);
// code_offset + stack_drop
constexpr auto BranchImmediateSize = 2 * sizeof(uint32_t);

constexpr uint32_t F32AbsMask = 0x7fffffff;
constexpr uint32_t F32SignMask = ~F32AbsMask;
Expand Down Expand Up @@ -453,15 +453,12 @@ __attribute__((no_sanitize("float-cast-overflow"))) inline constexpr float demot
return static_cast<float>(value);
}

void branch(const Code& code, OperandStack& stack, const uint8_t*& pc, const uint8_t*& immediates,
uint32_t arity) noexcept
void branch(const Code& code, OperandStack& stack, const uint8_t*& pc, uint32_t arity) noexcept
{
const auto code_offset = read<uint32_t>(immediates);
const auto imm_offset = read<uint32_t>(immediates);
const auto stack_drop = read<uint32_t>(immediates);
const auto code_offset = read<uint32_t>(pc);
const auto stack_drop = read<uint32_t>(pc);

pc = code.instructions.data() + code_offset;
immediates = code.immediates.data() + imm_offset;

// When branch is taken, additional stack items must be dropped.
assert(static_cast<int>(stack_drop) >= 0);
Expand Down Expand Up @@ -532,7 +529,6 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
static_cast<size_t>(code.max_stack_height));

const uint8_t* pc = code.instructions.data();
const uint8_t* immediates = code.immediates.data();

while (true)
{
Expand All @@ -548,27 +544,20 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
case Instr::if_:
{
if (stack.pop().as<uint32_t>() != 0)
immediates += 2 * sizeof(uint32_t); // Skip the immediates for else instruction.
pc += sizeof(uint32_t); // Skip the immediates for else instruction.
else
{
const auto target_pc = read<uint32_t>(immediates);
const auto target_imm = read<uint32_t>(immediates);

const auto target_pc = read<uint32_t>(pc);
pc = code.instructions.data() + target_pc;
immediates = code.immediates.data() + target_imm;
}
break;
}
case Instr::else_:
{
// We reach else only after executing if block ("then" part),
// so we need to skip else block now.
const auto target_pc = read<uint32_t>(immediates);
const auto target_imm = read<uint32_t>(immediates);

const auto target_pc = read<uint32_t>(pc);
pc = code.instructions.data() + target_pc;
immediates = code.immediates.data() + target_imm;

break;
}
case Instr::end:
Expand All @@ -582,16 +571,16 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
case Instr::br_if:
case Instr::return_:
{
const auto arity = read<uint32_t>(immediates);
const auto arity = read<uint32_t>(pc);

// Check condition for br_if.
if (instruction == Instr::br_if && stack.pop().as<uint32_t>() == 0)
{
immediates += BranchImmediateSize;
pc += BranchImmediateSize;
break;
}

branch(code, stack, pc, immediates, arity);
branch(code, stack, pc, arity);
break;
}
case Instr::br_table:
Expand All @@ -604,9 +593,9 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
const auto label_idx_offset = br_table_idx < br_table_size ?
br_table_idx * BranchImmediateSize :
br_table_size * BranchImmediateSize;
immediates += label_idx_offset;
pc += label_idx_offset;

branch(code, stack, pc, immediates, arity);
branch(code, stack, pc, arity);
break;
}
case Instr::call:
Expand Down
88 changes: 35 additions & 53 deletions lib/fizzy/parser_expr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,6 @@ inline void store(uint8_t* dst, T value) noexcept
__builtin_memcpy(dst, &value, sizeof(value));
}

template <typename T>
inline void push(bytes& b, T value)
{
uint8_t storage[sizeof(T)];
store(storage, value);
b.append(storage, sizeof(storage));
}

template <typename T>
inline void push(std::vector<uint8_t>& b, T value)
{
Expand All @@ -48,9 +40,6 @@ struct ControlFrame
/// The target instruction code offset.
const size_t code_offset{0};

/// The immediates offset for block instructions.
const size_t immediates_offset{0};

/// The frame stack height of the parent frame.
const int parent_stack_height{0};

Expand All @@ -62,11 +51,10 @@ struct ControlFrame
std::vector<size_t> br_immediate_offsets{};

ControlFrame(Instr _instruction, std::optional<ValType> _type, int _parent_stack_height,
size_t _code_offset = 0, size_t _immediates_offset = 0) noexcept
size_t _code_offset = 0) noexcept
: instruction{_instruction},
type{_type},
code_offset{_code_offset},
immediates_offset{_immediates_offset},
parent_stack_height{_parent_stack_height}
{}
};
Expand Down Expand Up @@ -210,16 +198,16 @@ inline void update_branch_stack(const ControlFrame& current_frame, const Control
drop_operand(current_frame, operand_stack, from_valtype(*branch_frame_type));
}

void push_branch_immediates(const ControlFrame& branch_frame, int stack_height, bytes& immediates)
void push_branch_immediates(
const ControlFrame& branch_frame, int stack_height, std::vector<uint8_t>& instructions)
{
// How many stack items to drop when taking the branch.
const auto stack_drop = stack_height - branch_frame.parent_stack_height;

// Push frame start location as br immediates - these are final if frame is loop,
// but for block/if/else these are just placeholders, to be filled at end instruction.
push(immediates, static_cast<uint32_t>(branch_frame.code_offset));
push(immediates, static_cast<uint32_t>(branch_frame.immediates_offset));
push(immediates, static_cast<uint32_t>(stack_drop));
push(instructions, static_cast<uint32_t>(branch_frame.code_offset));
push(instructions, static_cast<uint32_t>(stack_drop));
}

inline void mark_frame_unreachable(
Expand Down Expand Up @@ -473,7 +461,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(), code.immediates.size());
code.instructions.size());
break;
}

Expand All @@ -483,7 +471,7 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
std::tie(loop_type, pos) = parse_blocktype(pos, end);

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

Expand All @@ -493,12 +481,12 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
std::tie(if_type, pos) = parse_blocktype(pos, end);

control_stack.emplace(Instr::if_, if_type, static_cast<int>(operand_stack.size()),
code.instructions.size(), code.immediates.size());
code.instructions.size());

// Placeholders for immediate values, filled at the matching end or else instructions.
push(code.immediates, uint32_t{0}); // Diff to the else instruction
push(code.immediates, uint32_t{0}); // Diff for the immediates.
break;
code.instructions.push_back(opcode);
push(code.instructions, uint32_t{0}); // Diff to the else instruction
continue;
}

case Instr::else_:
Expand All @@ -508,30 +496,27 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f

update_result_stack(frame, operand_stack); // else is the end of if.

const auto if_imm_offset = frame.immediates_offset;
const auto if_imm_offset = frame.code_offset + 1;
const auto frame_type = frame.type;
auto frame_br_immediate_offsets = std::move(frame.br_immediate_offsets);

control_stack.pop();
control_stack.emplace(Instr::else_, frame_type, static_cast<int>(operand_stack.size()),
code.instructions.size(), code.immediates.size());
code.instructions.size());
// br immediates from `then` branch will need to be filled at the end of `else`
control_stack.top().br_immediate_offsets = std::move(frame_br_immediate_offsets);

// Placeholders for immediate values, filled at the matching end instructions.
push(code.immediates, uint32_t{0}); // Diff to the end instruction.
push(code.immediates, uint32_t{0}); // Diff for the immediates
code.instructions.push_back(opcode);
push(code.instructions, uint32_t{0}); // Diff to the end instruction.

// Fill in if's immediates with offsets of first instruction in else block.
const auto target_pc = static_cast<uint32_t>(code.instructions.size() + 1);
const auto target_imm = static_cast<uint32_t>(code.immediates.size());
const auto target_pc = static_cast<uint32_t>(code.instructions.size());

// Set the imm values for else instruction.
auto* if_imm = code.immediates.data() + if_imm_offset;
// Set the imm values for if instruction.
auto* if_imm = code.instructions.data() + if_imm_offset;
store(if_imm, target_pc);
if_imm += sizeof(target_pc);
store(if_imm, target_imm);
break;
continue;
}

case Instr::end:
Expand All @@ -549,26 +534,21 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
const auto target_pc = control_stack.size() == 1 ?
static_cast<uint32_t>(code.instructions.size()) :
static_cast<uint32_t>(code.instructions.size() + 1);
const auto target_imm = static_cast<uint32_t>(code.immediates.size());

if (frame.instruction == Instr::if_ || frame.instruction == Instr::else_)
{
// We're at the end instruction of the if block without else or at the end of
// else block. Fill in if/else's immediates with offsets of first instruction
// after if/else block.
auto* if_imm = code.immediates.data() + frame.immediates_offset;
auto* if_imm = code.instructions.data() + frame.code_offset + 1;
store(if_imm, target_pc);
if_imm += sizeof(target_pc);
store(if_imm, target_imm);
}

// Fill in immediates all br/br_table instructions jumping out of this block.
for (const auto br_imm_offset : frame.br_immediate_offsets)
{
auto* br_imm = code.immediates.data() + br_imm_offset;
auto* br_imm = code.instructions.data() + br_imm_offset;
store(br_imm, static_cast<uint32_t>(target_pc));
br_imm += sizeof(uint32_t);
store(br_imm, static_cast<uint32_t>(target_imm));
// stack drop and arity were already stored in br handler
}
}
Expand Down Expand Up @@ -596,13 +576,14 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f

update_branch_stack(frame, branch_frame, operand_stack);

push(code.immediates, get_branch_arity(branch_frame));
code.instructions.push_back(opcode);
push(code.instructions, get_branch_arity(branch_frame));

// Remember this br immediates offset to fill it at end instruction.
branch_frame.br_immediate_offsets.push_back(code.immediates.size());
branch_frame.br_immediate_offsets.push_back(code.instructions.size());

push_branch_immediates(
branch_frame, static_cast<int>(operand_stack.size()), code.immediates);
branch_frame, static_cast<int>(operand_stack.size()), code.instructions);

if (instr == Instr::br)
mark_frame_unreachable(frame, operand_stack);
Expand All @@ -615,7 +596,7 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
push_operand(operand_stack, *branch_frame.type);
}

break;
continue;
}

case Instr::br_table:
Expand Down Expand Up @@ -653,13 +634,13 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
if (get_branch_frame_type(branch_frame) != default_branch_type)
throw validation_error{"br_table labels have inconsistent types"};

branch_frame.br_immediate_offsets.push_back(code.immediates.size());
branch_frame.br_immediate_offsets.push_back(code.instructions.size());
push_branch_immediates(
branch_frame, static_cast<int>(operand_stack.size()), code.immediates);
branch_frame, static_cast<int>(operand_stack.size()), code.instructions);
}
default_branch_frame.br_immediate_offsets.push_back(code.immediates.size());
default_branch_frame.br_immediate_offsets.push_back(code.instructions.size());
push_branch_immediates(
default_branch_frame, static_cast<int>(operand_stack.size()), code.immediates);
default_branch_frame, static_cast<int>(operand_stack.size()), code.instructions);

mark_frame_unreachable(frame, operand_stack);

Expand All @@ -676,15 +657,16 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f

update_branch_stack(frame, branch_frame, operand_stack);

push(code.immediates, get_branch_arity(branch_frame));
code.instructions.push_back(opcode);
push(code.instructions, get_branch_arity(branch_frame));

branch_frame.br_immediate_offsets.push_back(code.immediates.size());
branch_frame.br_immediate_offsets.push_back(code.instructions.size());

push_branch_immediates(
branch_frame, static_cast<int>(operand_stack.size()), code.immediates);
branch_frame, static_cast<int>(operand_stack.size()), code.instructions);

mark_frame_unreachable(frame, operand_stack);
break;
continue;
}

case Instr::call:
Expand Down
4 changes: 0 additions & 4 deletions lib/fizzy/types.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,10 +363,6 @@ struct Code
// The instructions bytecode without immediate values.
// https://webassembly.github.io/spec/core/binary/instructions.html
std::vector<uint8_t> instructions;

// The decoded instructions' immediate values.
// These are instruction-type dependent fixed size value in the order of instructions.
bytes immediates;
};

/// The reference to the `code` in the wasm binary.
Expand Down
2 changes: 1 addition & 1 deletion test/unittests/execute_control_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ TEST(execute_control, if_else_smoke)

const auto module = parse(bin);

for (const auto param : {0u, 1u})
for (const auto param : {0u})
{
constexpr uint64_t expected_results[]{
2, // else branch.
Expand Down
6 changes: 2 additions & 4 deletions test/unittests/execute_numeric_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ ExecutionResult execute_unary_operation(Instr instr, uint64_t arg)
module->funcsec.emplace_back(TypeIdx{0});
module->codesec.emplace_back(Code{1, 0,
{static_cast<uint8_t>(Instr::local_get), 0, 0, 0, 0, static_cast<uint8_t>(instr),
static_cast<uint8_t>(Instr::end)},
{}});
static_cast<uint8_t>(Instr::end)}});

return execute(*instantiate(std::move(module)), 0, {arg});
}
Expand All @@ -36,8 +35,7 @@ ExecutionResult execute_binary_operation(Instr instr, uint64_t lhs, uint64_t rhs
module->funcsec.emplace_back(TypeIdx{0});
module->codesec.emplace_back(Code{2, 0,
{static_cast<uint8_t>(Instr::local_get), 0, 0, 0, 0, static_cast<uint8_t>(Instr::local_get),
1, 0, 0, 0, static_cast<uint8_t>(instr), static_cast<uint8_t>(Instr::end)},
{}});
1, 0, 0, 0, static_cast<uint8_t>(instr), static_cast<uint8_t>(Instr::end)}});

return execute(*instantiate(std::move(module)), 0, {lhs, rhs});
}
Expand Down
Loading

0 comments on commit 656e46d

Please sign in to comment.