Skip to content

Commit

Permalink
Add uint32_t i32 field to Value union
Browse files Browse the repository at this point in the history
This allows using i32 values directly what is less confusing than
previous emulation with storing these as uint64_t.
  • Loading branch information
chfast committed Jan 13, 2021
1 parent 26bb712 commit 52786d4
Show file tree
Hide file tree
Showing 18 changed files with 171 additions and 105 deletions.
3 changes: 1 addition & 2 deletions include/fizzy/fizzy.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ typedef struct FizzyModule FizzyModule;
typedef struct FizzyInstance FizzyInstance;

/// The data type representing numeric values.
///
/// i64 member is used to represent values of both i32 and i64 type.
typedef union FizzyValue
{
uint32_t i32;
uint64_t i64;
float f32;
double f64;
Expand Down
6 changes: 3 additions & 3 deletions lib/fizzy/execute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
goto trap;
const auto lhs = stack.top().as<int32_t>();
if (lhs == std::numeric_limits<int32_t>::min() && rhs == -1)
stack.top() = 0;
stack.top() = int32_t{0};
else
stack.top() = rem(lhs, rhs);
break;
Expand Down Expand Up @@ -1199,7 +1199,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
goto trap;
const auto lhs = stack.top().as<int64_t>();
if (lhs == std::numeric_limits<int64_t>::min() && rhs == -1)
stack.top() = 0;
stack.top() = int64_t{0};
else
stack.top() = rem(lhs, rhs);
break;
Expand Down Expand Up @@ -1434,7 +1434,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
}
case Instr::i64_extend_i32_u:
{
// effectively no-op
stack.top() = uint64_t{stack.top().i32};
break;
}
case Instr::i64_trunc_f32_s:
Expand Down
4 changes: 2 additions & 2 deletions lib/fizzy/instantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ std::unique_ptr<Instance> instantiate(std::unique_ptr<const Module> module,
{
// Offset is validated to be i32, but it's used in 64-bit calculation below.
const uint64_t offset =
eval_constant_expression(data.offset, imported_globals, globals).i64;
eval_constant_expression(data.offset, imported_globals, globals).i32;

if (offset + data.init.size() > memory->size())
throw instantiate_error{"data segment is out of memory bounds"};
Expand All @@ -304,7 +304,7 @@ std::unique_ptr<Instance> instantiate(std::unique_ptr<const Module> module,
{
// Offset is validated to be i32, but it's used in 64-bit calculation below.
const uint64_t offset =
eval_constant_expression(element.offset, imported_globals, globals).i64;
eval_constant_expression(element.offset, imported_globals, globals).i32;

if (offset + element.init.size() > table->size())
throw instantiate_error{"element segment is out of table bounds"};
Expand Down
2 changes: 1 addition & 1 deletion lib/fizzy/stack.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class OperandStack
m_top = m_bottom - 1;

const auto local_variables = std::copy_n(args, num_args, m_locals);
std::fill_n(local_variables, num_local_variables, 0);
std::fill_n(local_variables, num_local_variables, Value{});
}

OperandStack(const OperandStack&) = delete;
Expand Down
12 changes: 5 additions & 7 deletions lib/fizzy/value.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@

#pragma once

#include <cassert>
#include <cstdint>
#include <limits>

namespace fizzy
{
union Value
{
uint32_t i32;
uint64_t i64;
float f32;
double f64;
Expand All @@ -26,11 +26,11 @@ union Value
/// We need to support {signed,unsigned} x {32,64} integers. However, due to uint64_t being
/// defined differently in different implementations we need to avoid the alias and provide
/// constructors for unsigned long and unsigned long long independently.
constexpr Value(unsigned int v) noexcept : i64{v} {}
constexpr Value(unsigned int v) noexcept : i32{v} {}
constexpr Value(unsigned long v) noexcept : i64{v} {}
constexpr Value(unsigned long long v) noexcept : i64{v} {}
constexpr Value(int64_t v) noexcept : i64{static_cast<uint64_t>(v)} {}
constexpr Value(int32_t v) noexcept : i64{static_cast<uint32_t>(v)} {}
constexpr Value(int32_t v) noexcept : i32{static_cast<uint32_t>(v)} {}

constexpr Value(float v) noexcept : f32{v} {}
constexpr Value(double v) noexcept : f64{v} {}
Expand All @@ -54,8 +54,7 @@ constexpr uint64_t Value::as<uint64_t>() const noexcept
template <>
constexpr uint32_t Value::as<uint32_t>() const noexcept
{
assert((i64 & 0xffffffff00000000) == 0);
return static_cast<uint32_t>(i64);
return i32;
}

template <>
Expand All @@ -67,8 +66,7 @@ constexpr int64_t Value::as<int64_t>() const noexcept
template <>
constexpr int32_t Value::as<int32_t>() const noexcept
{
assert((i64 & 0xffffffff00000000) == 0);
return static_cast<int32_t>(i64);
return static_cast<int32_t>(i32);
}

template <>
Expand Down
1 change: 1 addition & 0 deletions test/testfloat/testfloat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ bool eq(TypedValue v, uint64_t expected_bits, bool ignore_nans)
switch (v.type)
{
case ValType::i32:
return v.value.i32 == expected_bits;
case ValType::i64:
return v.value.i64 == expected_bits;
case ValType::f32:
Expand Down
4 changes: 2 additions & 2 deletions test/unittests/api_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ TEST(api, execution_result_value)
const ExecutionResult result = Value{1234_u32};
EXPECT_FALSE(result.trapped);
EXPECT_TRUE(result.has_value);
EXPECT_EQ(result.value.i64, 1234_u32);
EXPECT_EQ(result.value.i32, 1234_u32);
}

TEST(api, execution_result_bool_constructor)
Expand All @@ -65,7 +65,7 @@ TEST(api, execution_result_value_constructor)
const ExecutionResult result{value};
EXPECT_FALSE(result.trapped);
EXPECT_TRUE(result.has_value);
EXPECT_EQ(result.value.i64, 1234_u32);
EXPECT_EQ(result.value.i32, 1234_u32);
}

TEST(api, resolve_imported_functions)
Expand Down
14 changes: 8 additions & 6 deletions test/unittests/capi_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ TEST(capi, find_exported_global)
ASSERT_TRUE(fizzy_find_exported_global(instance, "g1", &global));
EXPECT_EQ(global.type.value_type, FizzyValueTypeI32);
EXPECT_FALSE(global.type.is_mutable);
EXPECT_EQ(global.value->i64, 42);
EXPECT_EQ(global.value->i32, 42);

EXPECT_FALSE(fizzy_find_exported_global(instance, "g2", &global));
EXPECT_FALSE(fizzy_find_exported_global(instance, "foo", &global));
Expand Down Expand Up @@ -729,7 +729,7 @@ TEST(capi, memory_access)
memory[0] = 0xaa;
memory[1] = 0xbb;

EXPECT_EQ(fizzy_execute(instance, 0, nullptr, 0).value.i64, 0x22bbaa);
EXPECT_THAT(fizzy_execute(instance, 0, nullptr, 0), CResult(0x22bbaa_u32));

fizzy_free_instance(instance);
}
Expand Down Expand Up @@ -771,7 +771,7 @@ TEST(capi, imported_memory_access)
auto* instance = fizzy_instantiate(module, nullptr, 0, nullptr, &memory, nullptr, 0);
ASSERT_NE(instance, nullptr);

EXPECT_EQ(fizzy_execute(instance, 0, nullptr, 0).value.i64, 0x221100);
EXPECT_EQ(fizzy_execute(instance, 0, nullptr, 0).value.i32, 0x221100);

EXPECT_EQ(fizzy_get_instance_memory_size(instance), 65536);

Expand All @@ -781,8 +781,8 @@ TEST(capi, imported_memory_access)
memory_data[0] = 0xaa;
memory_data[1] = 0xbb;

EXPECT_EQ(fizzy_execute(instance_memory, 0, nullptr, 0).value.i64, 0x22bbaa);
EXPECT_EQ(fizzy_execute(instance, 0, nullptr, 0).value.i64, 0x22bbaa);
EXPECT_EQ(fizzy_execute(instance_memory, 0, nullptr, 0).value.i32, 0x22bbaa);
EXPECT_EQ(fizzy_execute(instance, 0, nullptr, 0).value.i32, 0x22bbaa);

fizzy_free_instance(instance);
fizzy_free_instance(instance_memory);
Expand Down Expand Up @@ -838,7 +838,9 @@ TEST(capi, execute_with_host_function)
nullptr},
{{FizzyValueTypeI32, &inputs[0], 2},
[](void*, FizzyInstance*, const FizzyValue* args, int) {
return FizzyExecutionResult{false, true, {args[0].i64 / args[1].i64}};
FizzyValue v;
v.i32 = args[0].i32 / args[1].i32;
return FizzyExecutionResult{false, true, {v}};
},
nullptr}};

Expand Down
6 changes: 3 additions & 3 deletions test/unittests/cxx20_span_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ TEST(cxx20_span, stack)
span<const Value> s(stack.rend() - num_items, num_items);
EXPECT_FALSE(s.empty());
EXPECT_EQ(s.size(), 2);
EXPECT_EQ(s[0].i64, 12);
EXPECT_EQ(s[1].i64, 13);
EXPECT_EQ(s[0].i32, 12);
EXPECT_EQ(s[1].i32, 13);

stack[0] = 0;
EXPECT_EQ(s[1].i64, 0);
EXPECT_EQ(s[1].i32, 0);
}

TEST(cxx20_span, initializer_list)
Expand Down
33 changes: 33 additions & 0 deletions test/unittests/execute_numeric_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,16 @@ TEST(execute_numeric, i64_extend_i32_s)
EXPECT_THAT(i64_extend_i32_s(0x80000001_u32), Result(0xffffffff80000001_u64));
EXPECT_THAT(i64_extend_i32_s(0xfffffffe_u32), Result(0xfffffffffffffffe_u64));
EXPECT_THAT(i64_extend_i32_s(0xffffffff_u32), Result(0xffffffffffffffff_u64));

// Put some garbage in the Value's high bits.
TypedValue v{ValType::i32, {}};
v.value.i64 = 0xdeaddeaddeaddead;
v.value.i32 = 0x80000000;
EXPECT_THAT(i64_extend_i32_s(v), Result(0xffffffff80000000_u64));

v.value.i64 = 0xdeaddeaddeaddead;
v.value.i32 = 0x40000000;
EXPECT_THAT(i64_extend_i32_s(v), Result(0x0000000040000000_u64));
}

TEST(execute_numeric, i64_extend_i32_u)
Expand All @@ -496,6 +506,12 @@ TEST(execute_numeric, i64_extend_i32_u)
EXPECT_THAT(i64_extend_i32_u(0x80000001_u32), Result(0x0000000080000001_u64));
EXPECT_THAT(i64_extend_i32_u(0xfffffffe_u32), Result(0x00000000fffffffe_u64));
EXPECT_THAT(i64_extend_i32_u(0xffffffff_u32), Result(0x00000000ffffffff_u64));

// Put some garbage in the Value's high bits.
TypedValue v{ValType::i32, {}};
v.value.i64 = 0xdeaddeaddeaddead;
v.value.i32 = 0x80000000;
EXPECT_THAT(i64_extend_i32_u(v), Result(0x0000000080000000_u64));
}

TEST(execute_numeric, i64_extend_i32_u_2)
Expand All @@ -515,6 +531,23 @@ TEST(execute_numeric, i64_extend_i32_u_2)
EXPECT_THAT(execute(*instance, 0, {0xff000000}), Result(0x00000000ff000000_u64));
}

TEST(execute_numeric, i64_extend_i32_u_3)
{
/* wat2wasm
(func (param i32) (result i64)
i64.const 0xdeadbeefdeadbeef
drop
local.get 0
i64.extend_i32_u
)
*/
const auto wasm = from_hex(
"0061736d0100000001060160017f017e030201000a1201100042effdb6f5fdddefd65e1a2000ad0b");

auto instance = instantiate(parse(wasm));
EXPECT_THAT(execute(*instance, 0, {0xff000000}), Result(0x00000000ff000000_u64));
}

TEST(execute_numeric, i64_clz)
{
const auto i64_clz = create_unary_operation_executor(Instr::i64_clz);
Expand Down
6 changes: 3 additions & 3 deletions test/unittests/execute_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,16 @@ TEST(execute, global_get_imported)
const auto wasm = from_hex(
"0061736d010000000105016000017e020d01036d6f6404676c6f62037e00030201000a0601040023000b");

Value global_value = 42;
Value global_value = 42_u64;
auto instance = instantiate(
parse(wasm), {}, {}, {}, {ExternalGlobal{&global_value, {ValType::i64, false}}});

EXPECT_THAT(execute(*instance, 0, {}), Result(42));

global_value = 0;
global_value = 0_u64;
EXPECT_THAT(execute(*instance, 0, {}), Result(0));

global_value = 43;
global_value = 43_u64;
EXPECT_THAT(execute(*instance, 0, {}), Result(43));
}

Expand Down
Loading

0 comments on commit 52786d4

Please sign in to comment.