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

port updated version of llvm float div #531

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

knickish
Copy link
Contributor

@knickish knickish commented Jun 25, 2023

This pr updates the float division code to match more recent versions of LLVM's implementations from compiler-rt, adding proper handling of subnormal values. I tested this by fuzzing all f32 values and an equivalent number of f64s against hard float operations on my computer (AMD zen3 cpu) in addition to updating the existing tests in the repo.

There seem to be very slight differences against the arm target, not sure if there's a better approach than just ignoring subnormals on that arch.

@knickish knickish force-pushed the float_div_subnormal_rounding branch 2 times, most recently from 96bf09e to 5563410 Compare June 25, 2023 13:25
@knickish knickish marked this pull request as ready for review June 25, 2023 13:33
@Amanieu
Copy link
Member

Amanieu commented Jun 26, 2023

Thanks!

Regarding ARM, I know the the ARM SIMD instructions always flush subnormals to zero, so that might be what is happening here. The normal scalar FP instructions properly support subnormals though. I think it's fine to leave it under a #[cfg] with this explanation as a comment.

@knickish knickish force-pushed the float_div_subnormal_rounding branch from 5563410 to 71b5f58 Compare June 27, 2023 01:17
@knickish
Copy link
Contributor Author

Okay, added a comment next the cfg to explain.

@Amanieu Amanieu merged commit 049f73e into rust-lang:master Jun 28, 2023
25 checks passed
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