-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
add a fallback for more f16 intrinsics
#159175
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1026,9 +1026,12 @@ pub unsafe fn unaligned_volatile_store<T>(dst: *mut T, val: T); | |
| /// | ||
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::sqrt`](../../std/primitive.f16.html#method.sqrt) | ||
| #[inline] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub fn sqrtf16(x: f16) -> f16; | ||
| pub fn sqrtf16(x: f16) -> f16 { | ||
| sqrtf32(x as f32) as f16 | ||
| } | ||
| /// Returns the square root of an `f32` | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
|
|
@@ -1055,9 +1058,12 @@ pub fn sqrtf128(x: f128) -> f128; | |
| /// | ||
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::powi`](../../std/primitive.f16.html#method.powi) | ||
| #[inline] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub fn powif16(a: f16, x: i32) -> f16; | ||
| pub fn powif16(a: f16, x: i32) -> f16 { | ||
| powif32(a as f32, x) as f16 | ||
| } | ||
| /// Raises an `f32` to an integer power. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
|
|
@@ -1437,9 +1443,14 @@ pub fn log2f128(x: f128) -> f128 { | |
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::mul_add`](../../std/primitive.f16.html#method.mul_add) | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[inline] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const fn fmaf16(a: f16, b: f16, c: f16) -> f16; | ||
| pub const fn fmaf16(a: f16, b: f16, c: f16) -> f16 { | ||
| // NOTE: f32 does not have sufficient precision, so use f64 instead. | ||
| // see also https://github.com/llvm/llvm-project/issues/128450#issuecomment-2727540179. | ||
| fmaf64(a as f64, b as f64, c as f64) as f16 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment that f32 isn't enough precision so we need f64? The difference isn't obvious if you don't know the context.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you verified that f64 gives enough precision? I am not aware of a theorem for this case.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dug it up, by @beetrees of course llvm/llvm-project#128450 (comment).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I can't really follow but good to know they looked into it. :D This might be something one can just exhaustively test though?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I did a quick estimate by changing https://github.com/rust-lang/compiler-builtins/blob/5af7a65c0342d3f5ac1d250c7902aaabbfc2bd49/libm/src/math/fmaf16.rs to: extern crate std;
(x as f64).mul_add(y as f64, z as f64) as f16And running
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm fair, 48 bits is too big. Oh well.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't llvm/llvm-project#131531 say that f64 is not enough?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is for
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I'm mixing up the various 16-bit floats.... |
||
| } | ||
| /// Returns `a * b + c` without rounding the intermediate result for `f32` values. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
|
|
@@ -1535,9 +1546,12 @@ pub const fn fmuladdf128(a: f128, b: f128, c: f128) -> f128 { | |
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::floor`](../../std/primitive.f16.html#method.floor) | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[inline] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const fn floorf16(x: f16) -> f16; | ||
| pub const fn floorf16(x: f16) -> f16 { | ||
| floorf32(x as f32) as f16 | ||
| } | ||
| /// Returns the largest integer less than or equal to an `f32`. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
|
|
@@ -1568,9 +1582,12 @@ pub const fn floorf128(x: f128) -> f128; | |
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::ceil`](../../std/primitive.f16.html#method.ceil) | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[inline] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const fn ceilf16(x: f16) -> f16; | ||
| pub const fn ceilf16(x: f16) -> f16 { | ||
| ceilf32(x as f32) as f16 | ||
| } | ||
| /// Returns the smallest integer greater than or equal to an `f32`. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
|
|
@@ -1601,9 +1618,12 @@ pub const fn ceilf128(x: f128) -> f128; | |
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::trunc`](../../std/primitive.f16.html#method.trunc) | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[inline] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const fn truncf16(x: f16) -> f16; | ||
| pub const fn truncf16(x: f16) -> f16 { | ||
| truncf32(x as f32) as f16 | ||
| } | ||
| /// Returns the integer part of an `f32`. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
|
|
@@ -1635,9 +1655,12 @@ pub const fn truncf128(x: f128) -> f128; | |
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::round_ties_even`](../../std/primitive.f16.html#method.round_ties_even) | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[inline] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const fn round_ties_even_f16(x: f16) -> f16; | ||
| pub const fn round_ties_even_f16(x: f16) -> f16 { | ||
| round_ties_even_f32(x as f32) as f16 | ||
| } | ||
|
|
||
| /// Returns the nearest integer to an `f32`. Rounds half-way cases to the number with an even | ||
| /// least significant digit. | ||
|
|
@@ -1674,9 +1697,12 @@ pub const fn round_ties_even_f128(x: f128) -> f128; | |
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::round`](../../std/primitive.f16.html#method.round) | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[inline] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const fn roundf16(x: f16) -> f16; | ||
| pub const fn roundf16(x: f16) -> f16 { | ||
| roundf32(x as f32) as f16 | ||
| } | ||
| /// Returns the nearest integer to an `f32`. Rounds half-way cases away from zero. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
|
|
@@ -3610,34 +3636,46 @@ pub const fn fabs<T: bounds::FloatPrimitive>(x: T) -> T; | |
| /// | ||
| /// The stabilized version of this intrinsic is | ||
| /// [`f16::copysign`](../../std/primitive.f16.html#method.copysign) | ||
| #[inline] | ||
| #[rustc_nounwind] | ||
| #[rustc_intrinsic] | ||
| pub const fn copysignf16(x: f16, y: f16) -> f16; | ||
| pub const fn copysignf16(x: f16, y: f16) -> f16 { | ||
| f16::from_bits((x.to_bits() & !f16::SIGN_MASK) | (y.to_bits() & f16::SIGN_MASK)) | ||
| } | ||
|
|
||
| /// Copies the sign from `y` to `x` for `f32` values. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
| /// [`f32::copysign`](../../std/primitive.f32.html#method.copysign) | ||
| #[inline] | ||
| #[rustc_nounwind] | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[rustc_intrinsic] | ||
| pub const fn copysignf32(x: f32, y: f32) -> f32; | ||
| pub const fn copysignf32(x: f32, y: f32) -> f32 { | ||
| f32::from_bits((x.to_bits() & !f32::SIGN_MASK) | (y.to_bits() & f32::SIGN_MASK)) | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @RalfJung should I remove the custom const-eval implementation (and add
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Merging makes the fallback bodies more tricky of course. For these bitwise operations maybe a simple trait is enough to implement them generically? Minimum has non-determinism and other complications, so that's not duplicated in const-eval, it's a genuinely different implementation. For these here... not sure, they seem sufficiently primitive to me that I think it's worth keeping the existing const-eval implementations.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes sorry i started doing it in #153934 and never got around to finishing it 🫠 ill pick it back up next week, @folkertdev once im done maybe rebasing this PR on that will make your life easier?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really, combining the intrinsics means it's impossible to add fallbacks for just For
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have edited the various minimum intrinsics' docs quite a bit, so those certainly do not qualify as "simple". So I feel very strongly that we should deduplicate these comments -- but we can do that without merging the functions, by writings the docs once on the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, fair. So then I think next steps are
@N1ark are you interested in any of that? I'll be doing some of the cranelift stuff to remove another
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To add a bit of context -- the reason why merging the intrinsics is nice is that every single tool that consumes MIR (such as the verification tool @N1ark is working on) needs to deal with all those intrinsics, and they can typically deal with them uniformly across all types, so merging them avoids a bunch of code duplication. Now, when in doubt we'll prioritize in-tree MIR consumers over out-of-tree MIR consumers. But it is a shame that we need to make a choice either way at all. So I wonder if we can somehow have our cake and eat it too. Is the problem with sqrt and pow and the others that they have a fallback for some types but not for others? Or could we have a fallback for them always, and what's annoying is just dispatching on the type? The latter could be solved... |
||
| /// Copies the sign from `y` to `x` for `f64` values. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
| /// [`f64::copysign`](../../std/primitive.f64.html#method.copysign) | ||
| #[inline] | ||
| #[rustc_nounwind] | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[rustc_intrinsic] | ||
| pub const fn copysignf64(x: f64, y: f64) -> f64; | ||
| pub const fn copysignf64(x: f64, y: f64) -> f64 { | ||
| f64::from_bits((x.to_bits() & !f64::SIGN_MASK) | (y.to_bits() & f64::SIGN_MASK)) | ||
| } | ||
|
|
||
| /// Copies the sign from `y` to `x` for `f128` values. | ||
| /// | ||
| /// The stabilized version of this intrinsic is | ||
| /// [`f128::copysign`](../../std/primitive.f128.html#method.copysign) | ||
| #[inline] | ||
| #[rustc_nounwind] | ||
| #[rustc_intrinsic] | ||
| pub const fn copysignf128(x: f128, y: f128) -> f128; | ||
| pub const fn copysignf128(x: f128, y: f128) -> f128 { | ||
| f128::from_bits((x.to_bits() & !f128::SIGN_MASK) | (y.to_bits() & f128::SIGN_MASK)) | ||
| } | ||
|
|
||
| /// Generates the LLVM body for the automatic differentiation of `f` using Enzyme, | ||
| /// with `df` as the derivative function and `args` as its arguments. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.