Skip to content

Update testsuite; fix conversion errors #1502

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

Merged
merged 2 commits into from
Jul 30, 2020
Merged

Update testsuite; fix conversion errors #1502

merged 2 commits into from
Jul 30, 2020

Conversation

binji
Copy link
Member

@binji binji commented Jul 29, 2020

The interpreter and wasm2c were incorrectly handling some float-to-int
conversions. For clarity, the wasm2c implementations of these
conversions now matches the implementation in interp-math.h more closely
(e.g. the numeric ranges are written as x >= min && x <= max in both
cases).

Quite a few wasm2c tests were previously being skipped, since wasm2c
doesn't currently support multi-value; it's better instead to duplicate
the tests here and disable the parts that are not supported so we don't
lose test coverage.

The interpreter and wasm2c were incorrectly handling some float-to-int
conversions. For clarity, the wasm2c implementations of these
conversions now matches the implementation in interp-math.h more closely
(e.g. the numeric ranges are written as `x >= min && x <= max` in both
cases).

Quite a few wasm2c tests were previously being skipped, since wasm2c
doesn't currently support multi-value; it's better instead to duplicate
the tests here and disable the parts that are not supported so we don't
lose test coverage.
@binji binji requested a review from sbc100 July 29, 2020 21:16
@binji
Copy link
Member Author

binji commented Jul 30, 2020

Thanks for double-checking the bounds! I had to think about them a bit to convince myself these are the correct values, but I think they are. For the 8 cases, (s32/u32/s64/u64 <- f32/f64), if we consider which values should be bounds if we had infinite floating-point precision, we would have the following (where _MAX values are the maximum value that is representable type):

to from min (excl.) max (excl.)
s32 f32 or f64 INT32_MIN - 1 INT32_MAX + 1
u32 f32 or f64 -1 UINT32_MAX + 1
s64 f32 or f64 INT64_MIN - 1 INT64_MAX + 1
u64 f32 or f64 -1 UINT64_MAX + 1

Now we have to consider which of these values are representable as f32 or f64. The values that had to be truncated are in bold:

value int f32 f64
INT32_MIN - 1 -2147483649 -2147483648.0 -2147483649.0
INT32_MAX + 1 2147483648 2147483648 2147483648
UINT32_MAX + 1 4294967296 4294967296 4294967296
INT64_MIN - 1 -9223372036854775809 -9223372036854775808.0 -9223372036854775808.0
INT64_MAX + 1 9223372036854775808 9223372036854775808 9223372036854775808
UINT64_MAX + 1 18446744073709551616 18446744073709551616.0 18446744073709551616.0

And finally, we can determine the bounds, where the ranges that had to be modified are in bold:

instr to from valid bounds
i32.trunc_f32_s s32 f32 -2147483648.f <= x < 2147483648.f
i32.trunc_f64_s s32 f64 -2147483649.0 < x < 2147483648.0
i32.trunc_f32_u u32 f32 -1.f < x < 4294967296.f
i32.trunc_f64_u u32 f64 -1.0 < x < 4294967296.0
i64.trunc_f32_s s64 f32 -9223372036854775808.f <= x < 9223372036854775808.f
i64.trunc_f64_s s64 f64 -9223372036854775808.f <= x < 9223372036854775808.f
i64.trunc_f32_u u64 f32 -1.f < x < 18446744073709551616.f
i64.trunc_f64_u u64 f64 -1.0 < x < 18446744073709551616.0

@binji binji merged commit 5390bb0 into master Jul 30, 2020
@binji binji deleted the update-testsuite branch July 30, 2020 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants