Skip to content

cranelift: Use round_ties_even for nearest in interpreter#4413

Merged
cfallin merged 1 commit intobytecodealliance:mainfrom
afonso360:interp-fix-nearest
Jul 7, 2022
Merged

cranelift: Use round_ties_even for nearest in interpreter#4413
cfallin merged 1 commit intobytecodealliance:mainfrom
afonso360:interp-fix-nearest

Conversation

@afonso360
Copy link
Contributor

@afonso360 afonso360 commented Jul 7, 2022

As @MaxGraey pointed out (thanks!) in #4397, round has different behavior from nearest. And it looks like the native rust implementation is still pending stabilization.

Right now we duplicate the wasmtime implementation, merged in #2171 since its quite short.

This also adds the missing tests that would have caught this issue.

Note: I'm happy to replace this with a version that tries to reuse the wasmtime implementation, but it wasn't immediately obvious to me how to do that in a clean manner.

 As @MaxGraey pointed out (thanks!) in bytecodealliance#4397, `round` has different
 behavior from `nearest`. And it looks like the native rust
 implementation is still pending stabilization.

 Right now we duplicate the wasmtime implementation, merged in bytecodealliance#2171.

 However, we definitely should switch to the rust native version
 when it is available.
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks!

@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Jul 7, 2022
@cfallin cfallin merged commit 16cb287 into bytecodealliance:main Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants