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

Implement f{32,64}.neg instructions #477

Merged
merged 3 commits into from
Aug 12, 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
16 changes: 8 additions & 8 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,9 @@ jobs:
- benchmark:
min_time: "0.01"
- spectest:
expected_passed: 10598
expected_passed: 10639
expected_failed: 3
expected_skipped: 1212
expected_skipped: 1171

sanitizers-macos:
executor: macos
Expand All @@ -289,9 +289,9 @@ jobs:
- benchmark:
min_time: "0.01"
- spectest:
expected_passed: 10598
expected_passed: 10639
expected_failed: 3
expected_skipped: 1212
expected_skipped: 1171

benchmark:
machine:
Expand Down Expand Up @@ -397,13 +397,13 @@ jobs:
at: ~/build
- spectest:
skip_validation: true
expected_passed: 9656
expected_passed: 9697
expected_failed: 3
expected_skipped: 2154
expected_skipped: 2113
- spectest:
expected_passed: 10598
expected_passed: 10639
expected_failed: 3
expected_skipped: 1212
expected_skipped: 1171
- collect_coverage_data

workflows:
Expand Down
14 changes: 12 additions & 2 deletions lib/fizzy/execute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,12 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, span<const Value>
break;
}

case Instr::f32_neg:
{
unary_op(stack, std::negate<float>{});
break;
}

case Instr::f32_add:
{
binary_op(stack, std::plus<float>{});
Expand All @@ -1553,6 +1559,12 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, span<const Value>
break;
}

case Instr::f64_neg:
{
unary_op(stack, std::negate<double>{});
break;
}

case Instr::f64_add:
{
binary_op(stack, std::plus<double>{});
Expand Down Expand Up @@ -1698,7 +1710,6 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, span<const Value>
break;
}

case Instr::f32_neg:
case Instr::f32_ceil:
case Instr::f32_floor:
case Instr::f32_trunc:
Expand All @@ -1709,7 +1720,6 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, span<const Value>
case Instr::f32_min:
case Instr::f32_max:
case Instr::f32_copysign:
case Instr::f64_neg:
case Instr::f64_ceil:
case Instr::f64_floor:
case Instr::f64_trunc:
Expand Down
22 changes: 22 additions & 0 deletions test/unittests/execute_floating_point_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,28 @@ TYPED_TEST(execute_floating_point_types, abs)
}
}

TYPED_TEST(execute_floating_point_types, neg)
{
using FP = FP<TypeParam>;
using Limits = typename FP::Limits;

auto instance = instantiate(parse(this->get_unop_code(Instr::f32_neg)));
const auto exec = [&](auto arg) { return execute(*instance, 0, {arg}); };

std::vector p_values(
std::begin(this->positive_special_values), std::end(this->positive_special_values));
for (const auto x :
{TypeParam{0}, Limits::infinity(), FP::nan(FP::canon), FP::nan(FP::canon + 1), FP::nan(1)})
p_values.push_back(x);

for (const auto p : p_values)
{
// fneg(+-p) = -+p
EXPECT_THAT(exec(p), Result(-p));
EXPECT_THAT(exec(-p), Result(p));
Copy link
Member

Choose a reason for hiding this comment

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

Aren't in these tests practically trusting the compiler compared to the others where we have explicit constants?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We trust that negation work everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

If we wanted to be pedantic we'd always test against the encoded representation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Possible in the future... i.e. we would need to extend FP to do some software floating-point operations.

Copy link
Member

Choose a reason for hiding this comment

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

Something like wast2wasm4cpp script could work, which evaluates the C++ code and puts in the hard constants. We could manually verify that when committed and at every change.

}
}

TYPED_TEST(execute_floating_point_types, add)
{
using FP = FP<TypeParam>;
Expand Down