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

Add rint and rintf #272

Merged
merged 4 commits into from
Nov 9, 2022
Merged

Add rint and rintf #272

merged 4 commits into from
Nov 9, 2022

Conversation

Jules-Bertholet
Copy link
Contributor

Necessary to support rust-lang/rust#95317

@Jules-Bertholet
Copy link
Contributor Author

Actually, I think it might make more sense to just use rint/rintf...

@Jules-Bertholet Jules-Bertholet changed the title Add roundeven and roundevenf Add rint and rintf Nov 7, 2022
@Amanieu
Copy link
Member

Amanieu commented Nov 9, 2022

The differences between rint and roundeven are:

  • rint uses the rounding mode from the FP environment (which defaults to round-to-nearest) while roundeven always rounds to nearest even.
  • rint raises FE_INEXACT if rounding occurs, roundeven doesn't.

As you said though, Rust already assumes the default rounding mode, so it doesn't matter too much. I think roundeven is the "more correct" choice to use but it's not well supported so it's OK to just use rint for now. Ideally we should support both in libm.

@Amanieu Amanieu merged commit a2420eb into rust-lang:master Nov 9, 2022
@Jules-Bertholet Jules-Bertholet deleted the roundeven branch November 9, 2022 00:47
@Jules-Bertholet
Copy link
Contributor Author

This change needs to make its way to a stable release before rust-lang/rust#95317 can be unblocked.

@Amanieu
Copy link
Member

Amanieu commented Nov 9, 2022

I published a new version. However you will need to update compiler-builtins which includes libm as a submodule. You'll need to add a re-export here: https://github.com/rust-lang/compiler-builtins/blob/master/src/math.rs

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