Skip to content
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

Add copysignf16, copysignf128, fabsf16, and fabsf128 #320

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Oct 26, 2024

Add generic versions of abs and copysign, which is used to implement f16 and f128 versions of these functions. These currently aren't tested because the builtin musl tests don't provide a reference, but the implementations are straightforward and we will be able to test against #311 once it lands. (Technically musl on aarch64 has fabsl and copysignl as f128, but I don't think it is worth updating the serialization tests).

The second commit ("Add float and integer traits...") just copies the exact trait implementations that we use in compiler-builtins, with the assumption that we will use it more in the future and for testing.

The third commit ("Add f16 and f128 configuration...") copies the logic from compiler-builtins for setting enable_f16 and enable_f128 with the exception that here, the unstable feature must also be enabled. This will probably need to be tweaked a bit in the future so it isn't tied to other behavior enabled by unstable, but this is enough to get us started.

@tgross35 tgross35 force-pushed the generic-algorithms branch 2 times, most recently from eb24e93 to 965cf5a Compare October 26, 2024 06:01
@burrbull
Copy link
Contributor

Using traits blocks us from using functions in const context, is not it?

@tgross35
Copy link
Contributor Author

It would indeed prevent us from making any of the public functions const (none of them are now), but providing const implementations is out of scope for this crate. Instead, there is work that can be done so that the equivalent functions in rust-lang/rust can become const, e.g. const { 100.0f32.cos() } will work.

I have some more details here if you are interested #310

@burrbull
Copy link
Contributor

is out of scope for this crate

Who has decided this?

@tgross35
Copy link
Contributor Author

It has been a goal for a while to make the float math functions available in core, which require various changes in this crate (improving accuracy, testing across all targets, performance, supporting the new float types). Conveniently, figuring out those same things will allow us to make the functions in std/core const. If that gets done then there won't be much use for this crate in its current form - so making that happen seems like a much better time investment.

The existing implementations aren't going away either. Generic methods just make it easier to support f16 and f128, as well as f32 algorithms that don't rely on f64 (for cases where there is no hardfloat f64).

@tgross35 tgross35 changed the title [wip] prepare for generic algorithms Add copysignf16, copysignf128, fabsf16, and fabsf128 Oct 26, 2024
@tgross35 tgross35 marked this pull request as ready for review October 26, 2024 08:44
@tgross35 tgross35 force-pushed the generic-algorithms branch 15 times, most recently from 1351a39 to 5c3a925 Compare October 29, 2024 07:54
@tgross35 tgross35 marked this pull request as draft October 29, 2024 08:20
In preparation of adding routines from these two types, duplicate the
`compiler-builtins` configuration here.
Add generic versions of `abs` and `copysign`. Make use of it for the
`f32` and `f64` versions of these functions.
Use the generic algorithm to make these two functions available whenever
`f16_enabled` or `f128_enabled` are true. These require the `unstable`
feature.
wip

Fixup features

Fix things that should be cfg disabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants