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

[RFC]: Portable SIMD Libs Project Group #2977

Merged
merged 3 commits into from
Sep 18, 2020

Conversation

KodrAus
Copy link
Contributor

@KodrAus KodrAus commented Aug 28, 2020

Establishes a new project group under libs for the production and stabilization of a core::simd and std::simd module. Based on the work of @hsivonen.

Rendered

cc @BurntSushi @hsivonen @Lokathor

@KodrAus KodrAus added A-simd SIMD related proposals & ideas T-libs-api Relevant to the library API team, which will review and decide on the RFC. labels Aug 28, 2020
@KodrAus
Copy link
Contributor Author

KodrAus commented Aug 28, 2020

cc @Licenser based on your work in simd-json you may also be interested in portable SIMD in the standard library?

@Licenser
Copy link

@KodrAus Very much, portable SIMD in the standard library would be awesome!

@BurntSushi
Copy link
Member

LGTM. Thanks @KodrAus for putting this together!

@Lokathor Lokathor mentioned this pull request Aug 28, 2020
@scottmcm
Copy link
Member

Sounds great! I have no desire to tie myself to particular intrinsics, but would love to be able to write my semantic meaning with a nice rusty library and have it compile to something good for the target.

To repeat my personal opinion from the lang issue linked in the RFC: I expect that this will be semantically indistinguishable from an implementation that lowers to just using scalar operations for everything (and would probably do that on targets without simd instructions) so am expecting there to be minimal-if-any lang changes here. It'd be up to libs to bless an interface and libs-impl/compiler to agree on the best way to implement it. So having it be a libs project sounds like the correct home.

@programmerjake
Copy link
Member

To repeat my personal opinion from the lang issue linked in the RFC: I expect that this will be semantically indistinguishable from an implementation that lowers to just using scalar operations for everything (and would probably do that on targets without simd instructions) so am expecting there to be minimal-if-any lang changes here.

That works except in a few edge-cases: denormal floating-point numbers work differently on some architectures (ARM) when moving between SIMD/scalar, so requiring the exact same semantics would be detrimental.

@Lokathor
Copy link
Contributor

Also: bitwise ops on f32 don't work properly on x86/x64 if sse isn't enabled, because signaling nans are converted to quiet nans.

@workingjubilee
Copy link
Member

Gonna have to slap a "if you compile this to a computer not made this century it may not work right" label on this stuff.
image

@Lokathor
Copy link
Contributor

Rust has official i586 targets (tier2), which are i686 with sse/sse2 turned off (they are on by default in the i686 targets).

@workingjubilee
Copy link
Member

Indeed! I actually currently live with someone who uses a 32 bit computer, so I am being quite serious when I say that the project group will likely have to discuss including some sort of caution about how the behavior will be normalized across architectures as much as possible but architectural variation may yield alterations, or guidelines about programming around such, or something.

@KodrAus
Copy link
Contributor Author

KodrAus commented Aug 31, 2020

@Licenser @workingjubilee @programmerjake @scottmcm would any of you be interested in supporting the group and be added to the list? On that note...

@rfcbot fcp merge

Let's get the ball rolling on this so we don't lose any of this momentum!

@rfcbot
Copy link
Collaborator

rfcbot commented Aug 31, 2020

Team member @KodrAus has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Aug 31, 2020
@pickfire
Copy link
Contributor

pickfire commented Aug 31, 2020

I think @sebcrozet may be interested in this too since he worked on zapier, simba and such mathematical libraries.

@KodrAus
Copy link
Contributor Author

KodrAus commented Sep 4, 2020

Based on the current state of the Libs FCP list, I'll go ahead and check SimonSapin's box.

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Sep 7, 2020
@rfcbot
Copy link
Collaborator

rfcbot commented Sep 7, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@hsivonen
Copy link
Member

hsivonen commented Sep 8, 2020

Establishes a new project group under libs for the production and stabilization of a core::simd and std::simd module.

Thank you for moving this forward! LGTM.

Sorry about the slow response this month.

Based on the work of @hsivonen.

@gnzlbg should get the credit for packed_simd.

@pachi
Copy link

pachi commented Sep 8, 2020

There's a new simd crate, generic-simd, https://crates.io/crates/generic-simd and its author has shown interest in participating in this effort on Reddit (and states that they don't know how to participate in this groups).

@calebzulawski
Copy link
Member

That's me, I'd like to participate wherever I can be helpful!

@KodrAus
Copy link
Contributor Author

KodrAus commented Sep 9, 2020

@calebzulawski That's great! I'll add you to the list 😃

We're going to reach the end of the FCP soon so will be able to set up a dedicated workspace and figure out what our first steps will be.

## Goals

- Determine the shape of the portable SIMD API.
- Get an unstable `std::simd` and `core::simd` API in the standard library. This may mean renaming `packed_simd` to `stdsimd` and working directly on it, or creating a new repository and pulling in chunks of code as needed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we evaluate options other than packed_simd and stdsimd too or try experimenting more? Or are these already good enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is definitely something to figure out when we kick off, and at the very least we should survey the various approaches out there. I’d love to take some time to dig into generic-simd properly.

In general I think we’d settled on a starting point based off packed_simd, without exposing a generic Simd<T> (so a public structure similar to wide), with a sprinkling of const generics for shuffles.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we evaluate options other than packed_simd and stdsimd too or try experimenting more? Or are these already good enough?

I think more experimentation carries a significant risk of not reaching the goal of getting to stable.

Notably, simd already existed before packed_simd, the initial design of packed_simd omitted something that simd already had (boolean/mask vectors) and ended up adding them back, so packed_simd ended up being like a more comprehensive version of simd with the internals rewritten.

That is, portable SIMD in Rust has already gone through the cycle of a change in the person working on it resulting in a rewrite that, while making things more comprehensive, on the high level ended up close to the old thing. While each such cycle might improved the design marginally, starting over means the distance to getting the stable gets reset to zero.

Copy link
Contributor

@pickfire pickfire Sep 9, 2020

Choose a reason for hiding this comment

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

Ah, so the goal here is more of stabilizing it rather than having more experimentation to improve the existing ones and then only decide to go stable.

Copy link
Member

@calebzulawski calebzulawski Sep 9, 2020

Choose a reason for hiding this comment

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

I would also like to mention that even though I started from scratch, generic-simd ended up particularly similar to packed_simd, and most of the features would be applicable to an implementation based on packed_simd.

The major differences of generic-simd are:

  • works on stable (likely irrelevant here)
  • a little bit more comprehensive traits (like num-traits, but for SIMD)
  • a different model for runtime feature selection (not sure if this is relevant)

Copy link
Member

Choose a reason for hiding this comment

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

Actually looking at the LLVM code some more, it seems you can bypass this check by using #[inline(always)]. Please ignore my previous comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just #[inline] goes on the core::simd function, and then the caller can choose to target_feature(enable) in a function that uses core::simd.

According to the docs for is_x86_feature_detected!, that alone should do it, without core::simd needing to be target feature aware at all.

Copy link
Member

Choose a reason for hiding this comment

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

You need to use #[inline(always)] on the core::simd function otherwise LLVM will refuse to inline functions with mismatching target features. But otherwise you are correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about runtime selection? Is rust going to provide runtime selection for SIMD which may not be zero-cost? Or maybe users can opt-in for it?

Copy link
Contributor

Choose a reason for hiding this comment

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

That is what the multiversion crate (or similar) would do.

Actually putting that sort of mechanic in the standard library is probably a whole separate project.

@fu5ha
Copy link

fu5ha commented Sep 13, 2020

I'm also interested in this in terms of using it in ultraviolet, which is currently using @Lokathor 's wide crate...

For a fair while I had a branch which was able to function with either wide or packed_simd as its underlying primitive type based on crate feature selection and I only had to minimally shim the two apis together to make that work... and the process of writing ultraviolet using these similar interfaces was quite pleasant, which as some others have mentioned is I think a good sign that at least the outward-facing APIs have converged to a good place. Hence, I'd say moving towards something stabilized in core/std is a reasonable goal...

A note (though perhaps this should be discussed more once this is kicked off properly and not here) is that packed-simd is currently using SLEEF as the backbone for a lot of the specifically SIMD-codegen'd versions of non-builtin math functions. I'm not sure what the precedent is here but it seems weird to have something in core/std depending on a C lib?

@workingjubilee
Copy link
Member

workingjubilee commented Sep 14, 2020

Lots of Rust has depended on C libs now or in the past. Obviously there's bindings to libc, and there's been various other bindings like to libbacktrace. The general precedent in the Rust project(s) has been "use opportunistically where necessary, then increasingly Rust away the C when it causes problems." Obviously this project group is going to have one of its tasks be "so, what DO we do about SLEEF?"

@bjorn3
Copy link
Member

bjorn3 commented Sep 14, 2020

While Rust depends on C libs for the libstd, libcore doesn't. libcore and liballoc can be built completely without a C compiler for as long as the c feature of compiler-builtins is disabled. Currently building for wasm doesn't use any C code, as there is no wasm capable C compiler that we can depend on to be available. Only when compiling for wasi do we use some C code in the form of a pre compiled libc. If SLEEF were to be added, you would need a C compiler to compile for wasm.

@SimonSapin
Copy link
Contributor

Right, if SLEEF is involved then APIs that depend on it should be in libstd but not libcore.

@hsivonen
Copy link
Member

SLEEF is only for x86_64 and for specialized functionality. It's also disabled by default. It would make sense to start without the operations that call into SLEEF.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Sep 17, 2020
@rfcbot
Copy link
Collaborator

rfcbot commented Sep 17, 2020

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@KodrAus
Copy link
Contributor Author

KodrAus commented Sep 18, 2020

It looks like we've got the green light to make a start here! 🥳 As with the Error handling group, we might need some help from our friendly neighborhood @Mark-Simulacrum to set up a project-portable-simd stream in Zulip and a rust-lang/project-portable-simd repository in GitHub.

I'm happy to write up an announcement post for the Insiders Blog (unless somebody else would like to do it!).

I can also start up a HackMD doc that we can use to put together a list of first steps, then figure out a time for a kickoff meeting via Zulip?

What do you all think?

@KodrAus
Copy link
Contributor Author

KodrAus commented Sep 18, 2020

Just so that everyone subscribed to this thread can find it I've created a HackMD that we can all drop some first steps and things that need to be figured out into:

https://hackmd.io/_2MnKA4FTEaiac7RR1MG4A

There's a link to a poll for a regular meeting time too.

@KodrAus
Copy link
Contributor Author

KodrAus commented Sep 18, 2020

Alrighty, I've created a tracking issue for the project group in the Libs team repo that can be used to post status updates: rust-lang/libs-team#4

Some of the links won't work just yet 🙂

@KodrAus
Copy link
Contributor Author

KodrAus commented Sep 18, 2020

I've updated the filename and links so with a ✔️ we should be able to merge this one too.

@Mark-Simulacrum Mark-Simulacrum merged commit 1aa4590 into rust-lang:master Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-simd SIMD related proposals & ideas disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.