Repository navigation
add scalbn and ilogb methods on floats - #163187
folkertdev wants to merge 1 commit into
Conversation
rustbot
added
A-compiler-builtins
| } | ||
|
|
||
| intrinsic_dispatch_on_type! { | ||
| /// Returns the order of magnitude of `x` (in base 2). |
There was a problem hiding this comment.
Should probably clarify that this means it ignores the sign, even though it's technically correct.
There was a problem hiding this comment.
Do you have a concrete suggestion for how to rephrase this? When I try it just gets more confusing, leaning on the examples here seems fine to me.
| /// Placeholder. | ||
| #[inline] | ||
| #[unstable(feature = "float_exponent_methods", issue = "163184")] | ||
| #[rustc_const_unstable(feature = "float_exponent_methods", issue = "163184")] | ||
| #[must_use = "method returns a new number and does not mutate the original value"] | ||
| pub const fn scalbn(self, exponent: i32) -> f128 { | ||
| intrinsics::scalbn(self, exponent) | ||
| } | ||
|
|
||
| /// Placeholder. |
There was a problem hiding this comment.
Placeholder docs. (I hate that we can't just use macros to define the float types.)
There was a problem hiding this comment.
What could be nice nowadays is putting the docs into one of the macro_rules! attribute macros so you could do something like #[float_docs(ilogb, f128, constant_a = 1234, constant_b = 3.141)]. So the docs can get reused but "go to source" works correctly and the implementations can deviate where necessary.
| /// Placeholder. | ||
| #[inline] | ||
| #[unstable(feature = "float_exponent_methods", issue = "163184")] | ||
| #[rustc_const_unstable(feature = "float_exponent_methods", issue = "163184")] | ||
| #[must_use = "method returns a new number and does not mutate the original value"] | ||
| pub const fn scalbn(self, exponent: i32) -> f16 { | ||
| intrinsics::scalbn(self, exponent) | ||
| } | ||
|
|
||
| /// Placeholder. |
There was a problem hiding this comment.
Placeholder docs. (I hate that we can't just use macros to define the float types.)
| intrinsics::scalbn(self, exponent) | ||
| } | ||
|
|
||
| /// Returns the exponent of `self`. |
There was a problem hiding this comment.
Should probably clarify that it's the absolute value of self, and ignores sign.
| /// Placeholder. | ||
| #[inline] | ||
| #[unstable(feature = "float_exponent_methods", issue = "163184")] | ||
| #[rustc_const_unstable(feature = "float_exponent_methods", issue = "163184")] | ||
| #[must_use = "method returns a new number and does not mutate the original value"] | ||
| pub const fn scalbn(self, exponent: i32) -> f64 { | ||
| intrinsics::scalbn(self, exponent) | ||
| } | ||
|
|
||
| /// Placeholder. |
There was a problem hiding this comment.
Placeholder docs. (I hate that we can't just use macros to define the float types.)
| // bitwise, no NaN adjustments | ||
| sym::fabs => interp_ok(x.abs().into()), | ||
| sym::ilogb => interp_ok(Scalar::from_i32(x.ilogb())), |
There was a problem hiding this comment.
The comment seems wrong for the new intrinsic -- I think ilogb needs NaN and precision non-determinism.
There was a problem hiding this comment.
It should be exact: it just extracts the exponent, accounting for subnormals, and returns an int.
The weird thing about it is that the ilogb libm function returns either INT_MIN or -INT_MAX for an input of 0, INT_MAX or INT_MIN for NaN. I guess that nondeterminism wouldn't hurt to add.
There was a problem hiding this comment.
| // bitwise, no NaN adjustments | |
| sym::fabs => interp_ok(x.abs().into()), | |
| sym::ilogb => interp_ok(Scalar::from_i32(x.ilogb())), | |
| // fabs and ilogb are both bitwise, no NaN adjustments | |
| sym::fabs => interp_ok(x.abs().into()), | |
| sym::ilogb => interp_ok(Scalar::from_i32(x.ilogb())), |
| let res = arg.scalbn(exp); | ||
| let res = self.adjust_nan(res, &[arg]); | ||
| interp_ok(res.into()) |
There was a problem hiding this comment.
This isn't doing precision non-determinism. That seems wrong?
There was a problem hiding this comment.
This is also a bitwise operation, so the result is exact.
There was a problem hiding this comment.
There are three kinds of float operations:
- bitwise ops like fabs, fully deterministic
- core arithmetic operations like add, mul, div, sqrt; non-deterministic NaNs but otherwise deterministic, precise results
- imprecise arithmetic operations like sin, exp
What does scalbn guarantee about NaNs? If it's bitwise, it should preserve the exact NaN payload and sign. Your implementation calls adjust_nan indicating that it is not bitwise.
What's the best authoritative spec for scalbn (and ilogb)?
There was a problem hiding this comment.
I haven’t read what exactly the C spec does but most implementations prepare a scale based on n then multiply x by it. So possible results are the same as multiplication between the input and a real, at least in practice.
There was a problem hiding this comment.
Not exactly authoritative but a data point
https://en.cppreference.com/c/numeric/math/scalbn
Return value
If no errors occur, [ ... ]
If a range error due to overflow occurs, ±HUGE_VAL, ±HUGE_VALF, or ±HUGE_VALL is returned.
If a range error due to underflow occurs, the correct result (after rounding) is returned.
My interpretation of that is that it's in this category
core arithmetic operations like add, mul, div, sqrt; non-deterministic NaNs but otherwise deterministic, precise results
Nothing suggests it's in the sin etc category. And modulo underflow it is an exact operation, except that it's not guaranteed that a NaN is returned unchanged, hence the adjust_nan.
| ( | ||
| $(#[$attr:meta])* | ||
| $vis:vis fn $name:ident<$generic:ident: $bound:path>( | ||
| $($arg:ident: $arg_ty:ty),* $(,)? | ||
| ) -> $ret_ty:ty; | ||
| $($concrete:ty => $body:block)* | ||
| ) => { | ||
| intrinsic_dispatch_on_type! { | ||
| @declare [] $(#[$attr])* | ||
| $vis fn $name<$generic: $bound>($($arg: $arg_ty),*) -> $ret_ty; | ||
| $($concrete => $body)* | ||
| } | ||
| }; | ||
|
|
||
| ( | ||
| $(#[$attr:meta])* | ||
| $vis:vis const fn $name:ident<$generic:ident: $bound:path>( | ||
| $($arg:ident: $arg_ty:ty),* $(,)? | ||
| ) -> $ret_ty:ty; | ||
| $($concrete:ty => $body:block)* | ||
| ) => { | ||
| intrinsic_dispatch_on_type! { | ||
| @declare [const] $(#[$attr])* | ||
| $vis fn $name<$generic: $bound>($($arg: $arg_ty),*) -> $ret_ty; | ||
| $($concrete => $body)* | ||
| } | ||
| }; |
There was a problem hiding this comment.
You can avoid the duplicate with $(const $($const_dummy:literal)?)? and emitting $(const $($const_dummy)?)?. That's annoying to write so idk if it's worth it, but it works better if you might need to stack const/unsafe/async/etc.
There was a problem hiding this comment.
We can change that when it's needed.
| // bitwise, no NaN adjustments | ||
| sym::fabs => interp_ok(x.abs().into()), | ||
| sym::ilogb => interp_ok(Scalar::from_i32(x.ilogb())), |
There was a problem hiding this comment.
It should be exact: it just extracts the exponent, accounting for subnormals, and returns an int.
The weird thing about it is that the ilogb libm function returns either INT_MIN or -INT_MAX for an input of 0, INT_MAX or INT_MIN for NaN. I guess that nondeterminism wouldn't hurt to add.
| /// Placeholder. | ||
| #[inline] | ||
| #[unstable(feature = "float_exponent_methods", issue = "163184")] | ||
| #[rustc_const_unstable(feature = "float_exponent_methods", issue = "163184")] | ||
| #[must_use = "method returns a new number and does not mutate the original value"] | ||
| pub const fn scalbn(self, exponent: i32) -> f128 { | ||
| intrinsics::scalbn(self, exponent) | ||
| } | ||
|
|
||
| /// Placeholder. |
There was a problem hiding this comment.
What could be nice nowadays is putting the docs into one of the macro_rules! attribute macros so you could do something like #[float_docs(ilogb, f128, constant_a = 1234, constant_b = 3.141)]. So the docs can get reused but "go to source" works correctly and the implementations can deviate where necessary.
|
☔ The latest upstream changes (presumably #164788) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
tracking issue: #163184