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

impl std::simd::StdFloat #219

Merged
merged 2 commits into from
Dec 31, 2021
Merged

impl std::simd::StdFloat #219

merged 2 commits into from
Dec 31, 2021

Conversation

workingjubilee
Copy link
Member

This introduces an extension trait to allow use of floating point methods that need runtime support. It is excessively documented because its mere existence is quite vexing, as the entire thing constitutes a leakage of implementation details into user observable space. Eventually the entire thing will ideally be folded into core and restructured to match the rest of the library, whatever that structure might look like at the time. This is preferred in lieu of the "lang item" path because any energy the lang items require (and it will be not-insignificant, by Simulacrum's estimation) is better spent on implementing our libmvec.

I have already tested a sync of this branch into std and it works fine.

For further (and purely optional) reading on "why this", see:

@@ -2,5 +2,6 @@

members = [
"crates/core_simd",
"crates/std_float",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this crate maybe be called std_simd? It only has the extra float stuff for now, but it could end up with something unexpected.

Copy link
Member Author

@workingjubilee workingjubilee Dec 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessarily obvious we would want to keep anything else we include in std in the same crate (since, notionally, this one ought to be Doomed) and we'd likely have to adjust the paths for inclusion anyways (to use the more complicated scheme). It's fine to rename it if something does come up.

@@ -0,0 +1,165 @@
#![cfg_attr(feature = "as_crate", no_std)] // We are std!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this different than the lib.rs/mod.rs shenanigans in core_simd?

Copy link
Member Author

@workingjubilee workingjubilee Dec 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's just a single file we can just name it easily, and it doesn't cause any "dancing".

But when it's not "actually a crate", it's not allowed to set #![no_std]. I could probably adjust things more to be less complicated in this case (truly build as a crate and just reimport from std?) but I followed the same model as with core_simd so as to minimize any possible dependency issues in the Rust build system, as I would rather explore that as cleanup afterwards. I guess the question here is whether this will ever depend on something else inside std. If so: yes, we definitely need it to be included this way. If not, then it can just be its own entire thing.

@calebzulawski
Copy link
Member

I think we'll probably revisit this once we figure out what we're doing with traits at a larger scale (this may end up getting renamed to FloatExt or something, if get a core::simd::Float?)

@workingjubilee
Copy link
Member Author

Hm. StdFloat, I think, would be most "appropriate", in that event.

While consulting with Simulacrum on how to make available the float
functions that currently require runtime support for `Simd<f32, N>` and
`Simd<f64, N>`, we realized breaking coherence with the classic approach
of lang items was, since `{core,std}::simd::Simd` is a `ty::Adt`, likely
to be quite a bit nasty. The project group has a long-term plan for how
to get around this kind of issue and move the associated functions into
libcore, but that will likely take time as well. Since all routes
forward are temporally costly, we probably will skip the lang item
approach entirely and go the "proper" route, but in the interests of
having something this year for people to play around with, this
extension trait was whipped up.

For now, while it involves a lot of fairly internal details most users
shouldn't have to care about, I went ahead and fully documented the
situation for any passerby to read on the trait, as the situation is
quite unusual and puzzling to begin with.
@workingjubilee workingjubilee changed the title impl std::simd::Float impl std::simd::StdFloat Dec 31, 2021
@workingjubilee
Copy link
Member Author

workingjubilee commented Dec 31, 2021

Renamed it in anticipation of that possibility, to minimize churn for users.

Unresolved questions:

  • Should ./crates/std_float depend on details internal to std, or should it be a self-contained reexport?
  • Will StdFloat coexist with some other trait?
  • Will we want to add more to std specifically and use this as a std_simd crate?
  • How fast can we make this crate obsolete, rendering these questions irrelevant? 😈

@workingjubilee workingjubilee merged commit 7e4bdca into master Dec 31, 2021
@calebzulawski calebzulawski deleted the std-float branch October 17, 2022 00:17
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