Migrate away from nonfunctional fenv stubs
#510
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tgross35
force-pushed
the
replace-fenv
branch
5 times, most recently
from
d87dc07 to
1266321
Compare
|
Changes on the directly changed functions: The wins on the rounding functions are pretty significant. Looks like LLVM is having a slightly harder time optimizing sqrt and fma. |
Many routines have some form of handling for rounding mode and floating point exceptions, which are implemented via a combination of stubs and `force_eval!` use. This is suboptimal, however, because: 1. Rust does not interact with the floating point environment, so most of this code does nothing. 2. The parts of the code that are not dead are not testable. 3. `force_eval!` blocks optimizations, which is unnecessary because we do not rely on its side effects. We cannot ensure correct rounding and exception handling in all cases without some form of arithmetic operations that are aware of this behavior. However, the cases where rounding mode is explicitly handled or exceptions are explicitly raised are testable. Make this possible here for functions that depend on `math::fenv` by moving the implementation to a nonpublic function that takes a `Round` and returns a `Status`. Link: rust-lang#480
tgross35
force-pushed
the
replace-fenv
branch
2 times, most recently
from
0d878df to
e13e944
Compare
tgross35
force-pushed
the
replace-fenv
branch
from
e13e944 to
e8fbb05
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Many routines have some form of handling for rounding mode and floating point exceptions, which are implemented via a combination of stubs and
force_eval!use. This is suboptimal, however, because:force_eval!blocks optimizations, which is unnecessary because we do not rely on its side effects.We cannot ensure correct rounding and exception handling in all cases without some form of arithmetic operations that are aware of this behavior. However, the cases where rounding mode is explicitly handled or exceptions are explicitly raised are testable. Make this possible here for functions that depend on
math::fenvby moving the implementation to a nonpublic function that takes aRoundand returns aStatus.Cc: #480