Make floating point intrinsics generic #29792
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Jeffrharr
force-pushed
the
FloatingIntrinsics
branch
from
a9e48e7 to
ffc0932
Compare
Jeffrharr
force-pushed
the
FloatingIntrinsics
branch
from
ffc0932 to
e4d7cd7
Compare
| @@ -1 +1 @@ | |||
| Subproject commit cde1ed3196ba9b39bcf028e06e08a8722113a5cb | |||
| Subproject commit 62ad301a2407a7aca50c1d5120c63597d676d29f | |||
There was a problem hiding this comment.
Does this patch need to change the llvm submodule, or is this an accident?
|
What is the motivation for doing this? I see a lot of snapshot churn without clear benefit. |
|
Looks like it makes the implementation simpler perhaps? I'll just let r? @alexcrichton review. |
| pub fn log10f64(x: f64) -> f64; | ||
| /// Returns the base 10 logarithm of a foating point of type `T`. | ||
| #[cfg(not(stage0))] | ||
| pub fn log10<T>(x: T) -> T; |
There was a problem hiding this comment.
To cut down on the annotation burden here, could you extract all the intrinsics to their own extern "rust-intrinsic" blocks? That way the cfg annotations should only be needed once.
|
cc @rust-lang/compiler about changes to intrinsics |
|
seems like a @rust-lang/lang question too |
|
I guess given that this is what we do for integer intrinsics, it seems ok. I'm not wild about it but these aren't stable or anything like that, so... |
|
...to be clear, what I'm not wild about is not this patch, per se, but the kind of ad-hoc monomorphization rules that are not captured by a trait. But we have this a fair amount in intrisics (e.g., |
|
Any feedback on this PR so it can move forward? |
| pub fn expf64(x: f64) -> f64; | ||
| /// Returns the exponential of a foating point of type `T`. |
|
I'm fine with it, seems better to align these intrinsics with the integer ones. |
|
☔ The latest upstream changes (presumably #30719) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Apologies on disappearing for a while. It seems that most people are fine with this, so I'm going to go ahead and make the requested changes. |
|
Closing due to inactivity, but feel free to resubmit with a rebase! |
Bring floating point intrinsics in line with other generic intrinsics such as integer generics in #29316