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.

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

Pass function args by reference using span<> #359

Merged
merged 4 commits into from
Jul 14, 2020
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
34 changes: 15 additions & 19 deletions lib/fizzy/execute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,10 @@ bool invoke_function(
{
const auto num_args = func_type.inputs.size();
assert(stack.size() >= num_args);
std::vector<uint64_t> call_args{stack.rend() - num_args, stack.rend()};
span<const uint64_t> call_args{stack.rend() - num_args, num_args};
stack.drop(num_args);

const auto ret = func(instance, std::move(call_args), depth + 1);
const auto ret = func(instance, call_args, depth + 1);
// Bubble up traps
if (ret.trapped)
return false;
Expand All @@ -293,8 +293,8 @@ bool invoke_function(
inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instance& instance,
OperandStack& stack, int depth)
{
const auto func = [func_idx](Instance& _instance, std::vector<uint64_t> args, int _depth) {
return execute(_instance, func_idx, std::move(args), _depth);
const auto func = [func_idx](Instance& _instance, span<const uint64_t> args, int _depth) {
return execute(_instance, func_idx, args, _depth);
};
return invoke_function(func_type, func, instance, stack, depth);
}
Expand Down Expand Up @@ -549,10 +549,9 @@ std::unique_ptr<Instance> instantiate(Module module,
auto it_table = instance->table->begin() + elementsec_offsets[i];
for (const auto idx : instance->module.elementsec[i].init)
{
auto func = [idx, &instance_ref = *instance](
fizzy::Instance&, std::vector<uint64_t> args, int depth) {
return execute(instance_ref, idx, std::move(args), depth);
};
auto func = [idx, &instance_ref = *instance](fizzy::Instance&,
span<const uint64_t> args,
int depth) { return execute(instance_ref, idx, args, depth); };

*it_table++ =
ExternalFunction{std::move(func), instance->module.get_function_type(idx)};
Expand Down Expand Up @@ -584,10 +583,8 @@ std::unique_ptr<Instance> instantiate(Module module,
// Wrap the function with the lambda capturing shared instance
auto& table_function = (*it_table)->function;
table_function = [shared_instance, func = std::move(table_function)](
fizzy::Instance& _instance, std::vector<uint64_t> args,
int depth) {
return func(_instance, std::move(args), depth);
};
fizzy::Instance& _instance, span<const uint64_t> args,
int depth) { return func(_instance, args, depth); };
++it_table;
}
}
Expand All @@ -599,24 +596,23 @@ std::unique_ptr<Instance> instantiate(Module module,
return instance;
}

execution_result execute(
Instance& instance, FuncIdx func_idx, std::vector<uint64_t> args, int depth)
execution_result execute(Instance& instance, FuncIdx func_idx, span<const uint64_t> args, int depth)
{
assert(depth >= 0);
if (depth > CallStackLimit)
return {true, {}};

if (func_idx < instance.imported_functions.size())
return instance.imported_functions[func_idx].function(instance, std::move(args), depth);
return instance.imported_functions[func_idx].function(instance, args, depth);

const auto code_idx = func_idx - instance.imported_functions.size();
assert(code_idx < instance.module.codesec.size());

const auto& code = instance.module.codesec[code_idx];
auto* const memory = instance.memory.get();

std::vector<uint64_t> locals = std::move(args);
locals.resize(locals.size() + code.local_count);
std::vector<uint64_t> locals(args.size() + code.local_count, 0);
Copy link
Member

Choose a reason for hiding this comment

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

Is this zeroing it out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. First we allocate and zero for args + locals. Then we overwrite args with proper values.

std::copy_n(args.begin(), args.size(), locals.begin());

OperandStack stack(static_cast<size_t>(code.max_stack_height));

Expand Down Expand Up @@ -1523,8 +1519,8 @@ std::optional<ExternalFunction> find_exported_function(Instance& instance, std::
return std::nullopt;

const auto idx = *opt_index;
auto func = [idx, &instance](fizzy::Instance&, std::vector<uint64_t> args, int depth) {
return execute(instance, idx, std::move(args), depth);
auto func = [idx, &instance](fizzy::Instance&, span<const uint64_t> args, int depth) {
return execute(instance, idx, args, depth);
};

return ExternalFunction{std::move(func), instance.module.get_function_type(idx)};
Expand Down
13 changes: 10 additions & 3 deletions lib/fizzy/execute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "exceptions.hpp"
#include "module.hpp"
#include "span.hpp"
#include "types.hpp"
#include <cstdint>
#include <functional>
Expand All @@ -27,7 +28,7 @@ struct Instance;

struct ExternalFunction
{
std::function<execution_result(Instance&, std::vector<uint64_t>, int depth)> function;
std::function<execution_result(Instance&, span<const uint64_t>, int depth)> function;
FuncType type;
};

Expand Down Expand Up @@ -95,7 +96,13 @@ std::unique_ptr<Instance> instantiate(Module module,

// Execute a function on an instance.
execution_result execute(
Instance& instance, FuncIdx func_idx, std::vector<uint64_t> args, int depth = 0);
Instance& instance, FuncIdx func_idx, span<const uint64_t> args, int depth = 0);

inline execution_result execute(
Instance& instance, FuncIdx func_idx, std::initializer_list<uint64_t> args)
{
return execute(instance, func_idx, span<const uint64_t>{args});
}


// Function that should be used by instantiate as imports, identified by module and function name.
Expand All @@ -105,7 +112,7 @@ struct ImportedFunction
std::string name;
std::vector<ValType> inputs;
std::optional<ValType> output;
std::function<execution_result(Instance&, std::vector<uint64_t>, int depth)> function;
std::function<execution_result(Instance&, span<const uint64_t>, int depth)> function;
};

// Create vector of ExternalFunctions ready to be passed to instantiate.
Expand Down
1 change: 1 addition & 0 deletions lib/fizzy/span.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class span

constexpr T& operator[](std::size_t index) const noexcept { return m_begin[index]; }

constexpr T* data() const noexcept { return m_begin; }
[[nodiscard]] constexpr std::size_t size() const noexcept { return m_size; }

constexpr iterator begin() const noexcept { return m_begin; }
Expand Down
1 change: 1 addition & 0 deletions test/bench_internal/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ add_executable(fizzy-bench-internal)

target_sources(fizzy-bench-internal PRIVATE
bench_internal.cpp
execute_benchmarks.cpp
experimental.cpp
parser_benchmarks.cpp
parser_noinline.cpp
Expand Down
80 changes: 80 additions & 0 deletions test/bench_internal/execute_benchmarks.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Fizzy: A fast WebAssembly interpreter
// Copyright 2020 The Fizzy Authors.
// SPDX-License-Identifier: Apache-2.0

#include "span.hpp"
#include <benchmark/benchmark.h>
#include <memory>
#include <vector>

namespace
{
[[gnu::noinline]] auto init_locals_1(fizzy::span<const uint64_t> args, uint32_t local_count)
{
std::vector<uint64_t> locals;
locals.reserve(args.size() + local_count);
std::copy_n(args.begin(), args.size(), std::back_inserter(locals));
locals.resize(locals.size() + local_count);
return locals;
}

[[gnu::noinline]] auto init_locals_2(fizzy::span<const uint64_t> args, uint32_t local_count)
{
std::vector<uint64_t> locals(args.size() + local_count);
Copy link
Member

Choose a reason for hiding this comment

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

Is this the same as locals(args.size() + local_count, 0); e.g. the one used in execute.cpp?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. The 0 value is default.

std::copy_n(args.begin(), args.size(), locals.begin());
return locals;
}

[[gnu::noinline]] auto init_locals_3(fizzy::span<const uint64_t> args, uint32_t local_count)
{
std::vector<uint64_t> locals(args.size() + local_count);
__builtin_memcpy(locals.data(), args.data(), args.size());
return locals;
}

[[gnu::noinline]] auto init_locals_4(fizzy::span<const uint64_t> args, uint32_t local_count)
{
auto locals = std::make_unique<uint64_t[]>(args.size() + local_count);
std::copy_n(args.begin(), args.size(), &locals[0]);
std::fill_n(&locals[args.size()], local_count, 0);
return locals;
}

[[gnu::noinline]] auto init_locals_5(fizzy::span<const uint64_t> args, uint32_t local_count)
{
auto locals = std::make_unique<uint64_t[]>(args.size() + local_count);
__builtin_memcpy(locals.get(), args.data(), args.size());
__builtin_memset(locals.get() + args.size(), 0, local_count);
return locals;
}
} // namespace

template <typename T, T Fn(fizzy::span<const uint64_t>, uint32_t)>
static void init_locals(benchmark::State& state)
{
const auto num_args = static_cast<size_t>(state.range(0));
const auto num_locals = static_cast<uint32_t>(state.range(1));

const std::vector<uint64_t> args(num_args, 0xa49);
benchmark::ClobberMemory();

for ([[maybe_unused]] auto _ : state)
{
const auto locals = Fn(args, num_locals);
benchmark::DoNotOptimize(locals);
}
}
#define ARGS \
Args({0, 0}) \
->Args({2, 4}) \
->Args({2, 38}) \
->Args({3, 4}) \
->Args({3, 8}) \
->Args({3, 13}) \
->Args({5, 30}) \
->Args({10, 100})
BENCHMARK_TEMPLATE(init_locals, std::vector<uint64_t>, init_locals_1)->ARGS;
BENCHMARK_TEMPLATE(init_locals, std::vector<uint64_t>, init_locals_2)->ARGS;
BENCHMARK_TEMPLATE(init_locals, std::vector<uint64_t>, init_locals_3)->ARGS;
BENCHMARK_TEMPLATE(init_locals, std::unique_ptr<uint64_t[]>, init_locals_4)->ARGS;
BENCHMARK_TEMPLATE(init_locals, std::unique_ptr<uint64_t[]>, init_locals_5)->ARGS;
2 changes: 1 addition & 1 deletion test/spectests/spectests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ class test_runner

try
{
return fizzy::execute(*instance, *func_idx, std::move(args));
return fizzy::execute(*instance, *func_idx, args);
}
catch (fizzy::unsupported_feature const& ex)
{
Expand Down
6 changes: 3 additions & 3 deletions test/unittests/api_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ namespace
{
auto function_returning_value(uint64_t value) noexcept
{
return [value](Instance&, std::vector<uint64_t>, int) {
return [value](Instance&, span<const uint64_t>, int) {
return execution_result{false, {value}};
};
}

execution_result function_returning_void(Instance&, std::vector<uint64_t>, int) noexcept
execution_result function_returning_void(Instance&, span<const uint64_t>, int) noexcept
{
return {};
}
Expand Down Expand Up @@ -222,7 +222,7 @@ TEST(api, find_exported_function)
"0061736d010000000105016000017f021001087370656374657374036261720000040401700000050401010102"
"0606017f0041000b07170403666f6f000001670300037461620100036d656d0200");

auto bar = [](Instance&, std::vector<uint64_t>, int) { return execution_result{false, {42}}; };
auto bar = [](Instance&, span<const uint64_t>, int) { return execution_result{false, {42}}; };
const auto bar_type = FuncType{{}, {ValType::i32}};

auto instance_reexported_function =
Expand Down
26 changes: 12 additions & 14 deletions test/unittests/execute_call_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ TEST(execute_call, call_indirect_imported_table)

const Module module = parse(bin);

auto f1 = [](Instance&, std::vector<uint64_t>, int) { return execution_result{false, {1}}; };
auto f2 = [](Instance&, std::vector<uint64_t>, int) { return execution_result{false, {2}}; };
auto f3 = [](Instance&, std::vector<uint64_t>, int) { return execution_result{false, {3}}; };
auto f4 = [](Instance&, std::vector<uint64_t>, int) { return execution_result{false, {4}}; };
auto f5 = [](Instance&, std::vector<uint64_t>, int) { return execution_result{true, {}}; };
auto f1 = [](Instance&, span<const uint64_t>, int) { return execution_result{false, {1}}; };
auto f2 = [](Instance&, span<const uint64_t>, int) { return execution_result{false, {2}}; };
auto f3 = [](Instance&, span<const uint64_t>, int) { return execution_result{false, {3}}; };
auto f4 = [](Instance&, span<const uint64_t>, int) { return execution_result{false, {4}}; };
auto f5 = [](Instance&, span<const uint64_t>, int) { return execution_result{true, {}}; };

auto out_i32 = FuncType{{}, {ValType::i32}};
auto out_i64 = FuncType{{}, {ValType::i64}};
Expand Down Expand Up @@ -218,7 +218,7 @@ TEST(execute_call, imported_function_call)

const auto module = parse(wasm);

constexpr auto host_foo = [](Instance&, std::vector<uint64_t>, int) -> execution_result {
constexpr auto host_foo = [](Instance&, span<const uint64_t>, int) -> execution_result {
return {false, {42}};
};
const auto host_foo_type = module.typesec[0];
Expand All @@ -245,7 +245,7 @@ TEST(execute_call, imported_function_call_with_arguments)

const auto module = parse(wasm);

auto host_foo = [](Instance&, std::vector<uint64_t> args, int) -> execution_result {
auto host_foo = [](Instance&, span<const uint64_t> args, int) -> execution_result {
return {false, {args[0] * 2}};
};
const auto host_foo_type = module.typesec[0];
Expand Down Expand Up @@ -289,10 +289,10 @@ TEST(execute_call, imported_functions_call_indirect)
ASSERT_EQ(module.importsec.size(), 2);
ASSERT_EQ(module.codesec.size(), 2);

constexpr auto sqr = [](Instance&, std::vector<uint64_t> args, int) -> execution_result {
constexpr auto sqr = [](Instance&, span<const uint64_t> args, int) -> execution_result {
return {false, {args[0] * args[0]}};
};
constexpr auto isqrt = [](Instance&, std::vector<uint64_t> args, int) -> execution_result {
constexpr auto isqrt = [](Instance&, span<const uint64_t> args, int) -> execution_result {
return {false, {(11 + args[0] / 11) / 2}};
};

Expand Down Expand Up @@ -337,10 +337,8 @@ TEST(execute_call, imported_function_from_another_module)
const auto func_idx = fizzy::find_exported_function(module1, "sub");
ASSERT_TRUE(func_idx.has_value());

auto sub = [&instance1, func_idx](
Instance&, std::vector<uint64_t> args, int) -> execution_result {
return fizzy::execute(*instance1, *func_idx, std::move(args));
};
auto sub = [&instance1, func_idx](Instance&, span<const uint64_t> args,
int) -> execution_result { return fizzy::execute(*instance1, *func_idx, args); };

auto instance2 = instantiate(module2, {{sub, module1.typesec[0]}});

Expand Down Expand Up @@ -516,7 +514,7 @@ TEST(execute_call, call_imported_infinite_recursion)
"0061736d010000000105016000017f020b01036d6f6403666f6f0000030201000a0601040010000b");

const auto module = parse(wasm);
auto host_foo = [](Instance& instance, std::vector<uint64_t>, int depth) -> execution_result {
auto host_foo = [](Instance& instance, span<const uint64_t>, int depth) -> execution_result {
return execute(instance, 0, {}, depth + 1);
};
const auto host_foo_type = module.typesec[0];
Expand Down
2 changes: 1 addition & 1 deletion test/unittests/execute_control_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ TEST(execute_control, br_1_out_of_function_and_imported_function)
"0061736d010000000108026000006000017f02150108696d706f727465640866756e6374696f6e000003020101"
"0a0d010b00034041010c010b41000b");

constexpr auto fake_imported_function = [](Instance&, std::vector<uint64_t>,
constexpr auto fake_imported_function = [](Instance&, span<const uint64_t>,
int) noexcept -> execution_result { return {}; };

const auto module = parse(bin);
Expand Down
Loading