Use f128 internally for Duration-float methods#154107
Use f128 internally for Duration-float methods#154107cuviper wants to merge 4 commits intorust-lang:mainfrom
f128 internally for Duration-float methods#154107Conversation
rustbot
added
S-waiting-on-review
|
r? @scottmcm rustbot has assigned @scottmcm. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
I think this is a good solution but we can't actually do this unconditionally now, |
|
More specific list of that: rust/compiler/rustc_codegen_llvm/src/llvm_util.rs Lines 386 to 405 in 1f7f8ea |
|
Hmm, conditional implementation would be straightforward, but conditional testing would be quite annoying. I'll wait for feedback whether we actually want this change before I attempt that. |
|
Since a software float implementation would effectively be identical to the other PR, it does feel like this might be a better long-term change, but perhaps in the short-term the other PR is better? As in, leave a |
|
@tgross35 is it possible to use explicit software mul/div from e.g. compiler-builtins on the non-reliable targets? |
|
We need LLVM to not crash when it sees a Could we stomach some platform differences for now and use f128 where it is well-supported? |
|
To match #150933, |
|
|
|
I added conditions for |
If we want to keep as much `Duration` precision as possible when multiplying or dividing by a float, then we shouldn't convert it to `f32` nor `f64` to match the operand, because their mantissas aren't large enough to hold a full `Duration`. However, `f128` is large enough with `MANTISSA_DIGITS = 113`, since `Duration::MAX` only needs 94 bits.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Sorry if you're waiting on me here, did you maybe want to nominate for libs? |
rustbot
added
the
I-libs-nominated
A `Duration` is essentially a 94-bit value (64-bit sec and ~30-bit ns), so there's some inherent loss when converting to floating-point for `mul_f64` and `div_f64`. We could go to greater lengths to compute these with more accuracy, like rust-lang#150933 or rust-lang#154107, but it's not clear that it's worth the effort. The least we can do is document that some rounding is to be expected, which this commit does with simple examples that only multiply or divide by `1.0`. This also changes the `f32` methods to just forward to `f64`, so we keep more of that duration precision, as the range is otherwise much more limited there.
Document precision considerations of `Duration`-float methods A `Duration` is essentially a 94-bit value (64-bit sec and ~30-bit ns), so there's some inherent loss when converting to floating-point for `mul_f64` and `div_f64`. We could go to greater lengths to compute these with more accuracy, like rust-lang#150933 or rust-lang#154107, but it's not clear that it's worth the effort. The least we can do is document that some rounding is to be expected, which this commit does with simple examples that only multiply or divide by `1.0`. This also changes the `f32` methods to just forward to `f64`, so we keep more of that duration precision, as the range is otherwise much more limited there.
Rollup merge of #155133 - cuviper:duration_fp_docs, r=the8472 Document precision considerations of `Duration`-float methods A `Duration` is essentially a 94-bit value (64-bit sec and ~30-bit ns), so there's some inherent loss when converting to floating-point for `mul_f64` and `div_f64`. We could go to greater lengths to compute these with more accuracy, like #150933 or #154107, but it's not clear that it's worth the effort. The least we can do is document that some rounding is to be expected, which this commit does with simple examples that only multiply or divide by `1.0`. This also changes the `f32` methods to just forward to `f64`, so we keep more of that duration precision, as the range is otherwise much more limited there.
|
Closed in favor of #155133, which just merged. The code is here if we ever want to revisit this idea. |
rustbot
removed
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #155655) made this pull request unmergeable. Please resolve the merge conflicts. |
Document precision considerations of `Duration`-float methods A `Duration` is essentially a 94-bit value (64-bit sec and ~30-bit ns), so there's some inherent loss when converting to floating-point for `mul_f64` and `div_f64`. We could go to greater lengths to compute these with more accuracy, like rust-lang/rust#150933 or rust-lang/rust#154107, but it's not clear that it's worth the effort. The least we can do is document that some rounding is to be expected, which this commit does with simple examples that only multiply or divide by `1.0`. This also changes the `f32` methods to just forward to `f64`, so we keep more of that duration precision, as the range is otherwise much more limited there.
6 participants
If we want to keep as much
Durationprecision as possible whenmultiplying or dividing by a float, then we shouldn't convert it to
f32norf64to match the operand, because their mantissas aren'tlarge enough to hold a full
Duration. However,f128is large enoughwith
MANTISSA_DIGITS = 113, sinceDuration::MAXonly needs 94 bits.Fixes #149794, as an alternative to #150933.