-
Notifications
You must be signed in to change notification settings - Fork 726
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
SIMD Integer to floating point conversion implementation. #795
SIMD Integer to floating point conversion implementation. #795
Conversation
break; | ||
|
||
case Opcode::F64X2ConvertUI64X2: | ||
CHECK_TRAP(SimdUnop<v128, uint64_t>(SimdConvert<double, uint64_t>)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case uses wabt_convert_uint64_to_double
to handle a floating point rounding issue on windows. Can you do the same here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
dcdad44
to
468d6b0
Compare
Including: f32x4.convert_s/i32x4 f32x4.convert_u/i32x4 f64x2.convert_s/i64x2 f64x2.convert_u/i64x2
src/interp.cc
Outdated
break; | ||
|
||
case Opcode::F64X2ConvertUI64X2: | ||
#if COMPILER_IS_MSVC && defined(_M_X64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'd prefer to keep the #if in config.h.in
. I forgot that wabt_convert_*
is not a function otherwise. Can you instead convert the macro in config.h.in
to an inline function and always use that here? Sorry for the extra effort!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, looking at the appveyor build, it looks like it breaks on Windows. Not quite sure why though.
468d6b0
to
982a974
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thank you!
Including:
f32x4.convert_s/i32x4
f32x4.convert_u/i32x4
f64x2.convert_s/i64x2
f64x2.convert_u/i64x2