-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Tracking issue for SIMD support #27731
Comments
Note that #26403 may well be a blocker issue for 100% safe + composable SIMD |
This issue now tracks the |
Status update: In the compiler:
In https://github.com/huonw/simd:
I'm intending to work on the simd crate first, starting with the rewrite of the autogenerator, but I've currently got a thesis to work on. |
@huonw How's the thesis going? :) Any progress on this issue, to relay to those interested in SIMD stabilization? |
@BurntSushi, @nikomatsakis, and I talked about this recently at the work week, and our thoughts are:
All of this was discussed hopefully with an eye to start the process of stabilization soon-ish, and then we can all get SIMD on stable Rust! cc @eddyb, you likely have many opinions as well! |
@alexcrichton Ahh, I ignored the multiple-definition option in my recent comment. I am still wary about stabilizing intrinsics, but Other than that, this seems like a good move forward, without the complexities I was worried about. |
@eddyb hm yeah I'm not sure if |
There are other useful intrinsics like |
Oh interesting! I'd be ok punting on those for now in favor of just dealing with the SIMD pieces, but we can relatively easily reevaluate to do something different though. |
So I had a really interesting conversation with @sunfishcode on the topic of SIMD, and in particular the design of SIMD in WASM. The high-level summary was two points:
Some other interesting points that he raised:
|
On the topic of intrinsics, I feel overall pretty good about some kind of attribute that can be applied to a However, I feel mildly less good about the generic versions, since these cannot be checked until trans time, which means we have to face two annoying choices:
However, it does seem that there is a third way out: we could remove all support for generic intrinsics, and instead have people define their own traits that map to these operations. For example, today the #[simd_intrinsic(...)]
fn simd_eq<T,U>(t: T, u: T) -> U;
unsafe trait Simd {
type EqType;
}
fn generic_eq<T:Simd>(t: T, u: T) -> T::EqType {
simd_eq(t, t)
}
unsafe impl Simd for u32x4 { ... } // etc It seems like we could instead do: trait Simd { // no longer an unsafe trait
type EqType;
// we now include a method for the various simd operations we might want to do:
fn eq(x: &Self, y: &Self) -> Self::EqType;
...
}
#[simd_intrinsic]
fn eq_u32x4(x: u32x4, y: u32x4) -> boolx4 {...}
impl Simd for u32x4 {
#[inline(always)]
fn eq(x: &Self, y: &Self) -> Self::EqType {
eq_u32x4(x, y)
}
} I'm probably getting some of the details wrong (have to consult the crate for the precise names involved) but hopefully you get the idea. Basically, the compiler only supports monotype intrinsics, and the wrapper crate adds (using normal trait methods) any generic dispatch needed. |
Is there a good reason for making the function recurse into itself? It seems like unnecessary repetition to me. Would a macro like
I agree. This is one of the papercuts of the current state: most of the platform-specific intrinsics are there with their usual names, except for a few basic arithmetic operations, which are |
Maybe it's contrived, but casting the function to a function pointer would naturally give you a pointer to a function which contains the intrinsic operation.
There's a very good reason for keeping those that way: they're basic LLVM operations (i.e. |
Can anyone provide an overview of the status of this? I was talking with someone whose GitHub name I don't know on IRC, and there was some indication that no one is handling further development of this feature. I have enough experience with X86 SIMD that I could probably help. I like @nikomatsakis approach, except that sometimes you need to be able to treat One other possibility that comes to mind now that we're close to it is to finish type-level integers, then make generic intrinsics with declarations like this:
This of course depends on how close we are to having type-level integers, but it should be checkable well before trans in any sane implementation of type-level integers I can think of. Just a thought. |
LLVM shuffles don't care what the element types are, and neither do the Rust intrinsics exposing them. |
@eddyb If you drop the cross-platform shuffles in favor of putting it all in a crate and also drop the weird semi-generic nature of the original RFC, this does indeed become a problem. |
@camlorn afaik, nobody is carrying this forward, but I would very much like to see progress! I still basically stand by my previous comment, though I think @eddyb suggested (perhaps on IRC) the idea of applying the special attribute directly to the method in the impl, and that seems even better (perhaps just making it a lang item -- it would mean though that this lang item can be applied multiple times). I have no objection to exposing the platform intrinsics explicitly, but it also doesn't seem like a required ingredient. It'd be great to make progress on the wrapper library, and adding in platform-specific names feels orthogonal to me. (Right? This is a bit out of cache.) |
I'm not exactly sure what's the best next step. Perhaps a new RFC is warranted, just to lay out the plan clearly? At minimum some kind of canonical write-up feels appropriate. Hopefully the changes vis-a-vis today are relatively minimal. |
@nikomatsakis It seems to me that you could provide only the platform specific intrinsics, get the optimizer doing a good job with eliminating temporary moves, get type-level integers, and then add a As I understand it, we almost have type-level integers. And @pcwalton is working on the needed optimizer stuff. But that said, I have no problem with the original RFC. I started at the bottom of this thread and read up, however, and it seems to me that people are no longer convinced that this is a good way. Perhaps this impression changes once I read the whole thing. |
@BurntSushi I knew I saw something somewhere! See #27731 (comment) above. |
=== stdout === === stderr === error[E0412]: cannot find type `U` in this scope --> /home/runner/work/glacier/glacier/ices/82926.rs:5:44 | 5 | fn simd_insert<T>(x: T, idx: u32, val: U) -> T; | - ^ help: a type parameter with a similar name exists: `T` | | | similarly named type parameter `T` defined here error[E0658]: inline-const is experimental --> /home/runner/work/glacier/glacier/ices/82926.rs:9:5 | 9 | const { simd_insert(U, 1_u32, 42_f32) } | ^^^^^ | = note: see issue #76001 <rust-lang/rust#76001> for more information = help: add `#![feature(inline_const)]` to the crate attributes to enable error[E0658]: platform intrinsics are experimental and possibly buggy --> /home/runner/work/glacier/glacier/ices/82926.rs:3:8 | 3 | extern "platform-intrinsic" { | ^^^^^^^^^^^^^^^^^^^^ | = note: see issue #27731 <rust-lang/rust#27731> for more information = help: add `#![feature(platform_intrinsics)]` to the crate attributes to enable warning: type `f32x3` should have an upper camel case name --> /home/runner/work/glacier/glacier/ices/82926.rs:2:8 | 2 | struct f32x3(f32, f32, f32); | ^^^^^ help: convert the identifier to upper camel case (notice the capitalization): `F32x3` | = note: `#[warn(non_camel_case_types)]` on by default error[E0094]: intrinsic has wrong number of type parameters: found 1, expected 2 --> /home/runner/work/glacier/glacier/ices/82926.rs:5:19 | 5 | fn simd_insert<T>(x: T, idx: u32, val: U) -> T; | ^^^ expected 2 type parameters error[E0308]: mismatched types --> /home/runner/work/glacier/glacier/ices/82926.rs:9:25 | 9 | const { simd_insert(U, 1_u32, 42_f32) } | ^ expected `()`, found struct `f32x3` error: aborting due to 5 previous errors; 1 warning emitted Some errors have detailed explanations: E0094, E0308, E0412, E0658. For more information about an error, try `rustc --explain E0094`. ==============
=== stdout === === stderr === error[E0412]: cannot find type `T` in this scope --> /home/runner/work/glacier/glacier/ices/83837.rs:5:23 | 5 | fn simd_insert(x: T, idx: u32, val: U); | ^ not found in this scope error[E0412]: cannot find type `U` in this scope --> /home/runner/work/glacier/glacier/ices/83837.rs:5:41 | 5 | fn simd_insert(x: T, idx: u32, val: U); | ^ not found in this scope error[E0658]: inline-const is experimental --> /home/runner/work/glacier/glacier/ices/83837.rs:9:5 | 9 | const { simd_insert(U, 0x1319_8a2e, 42_u16) } | ^^^^^ | = note: see issue #76001 <rust-lang/rust#76001> for more information = help: add `#![feature(inline_const)]` to the crate attributes to enable error[E0658]: platform intrinsics are experimental and possibly buggy --> /home/runner/work/glacier/glacier/ices/83837.rs:3:8 | 3 | extern "platform-intrinsic" { | ^^^^^^^^^^^^^^^^^^^^ | = note: see issue #27731 <rust-lang/rust#27731> for more information = help: add `#![feature(platform_intrinsics)]` to the crate attributes to enable warning: type `u16x2` should have an upper camel case name --> /home/runner/work/glacier/glacier/ices/83837.rs:2:8 | 2 | struct u16x2(u16, u16); | ^^^^^ help: convert the identifier to upper camel case (notice the capitalization): `U16x2` | = note: `#[warn(non_camel_case_types)]` on by default error[E0094]: intrinsic has wrong number of type parameters: found 0, expected 2 --> /home/runner/work/glacier/glacier/ices/83837.rs:5:19 | 5 | fn simd_insert(x: T, idx: u32, val: U); | ^ expected 2 type parameters error: aborting due to 5 previous errors; 1 warning emitted Some errors have detailed explanations: E0094, E0412, E0658. For more information about an error, try `rustc --explain E0094`. ==============
Sorry to bump this issue, but after reading the discussions here, I am not 100% clear as to why the RISC-V intrinsics are tied to this issue. With the ARM, wasm32, x86 and x86_64 intrinsics being available on stable, is SIMD the blocker for RISC-V instrinsics? Since the vector extension (as well as other extensions) are ratified and available through a target-feature, is there a chance of merging those into a more stable release? If so, I am willing to pick up this issue of getting them ready for a more stable release. |
I'm in the process of splitting all std::arch intrinsics into their own separate target features. The main blocker for RISC-V vector intrinsics is support for scalable vectors, which is tracked in rust-lang/rfcs#3268. |
Would you be open to having me slowly including other extensions in the |
Zbb doesn't need to be exposed as intrinsics, all of the functionality is already available as methods on plain integer types. Crypto intrinsics are fine to add though, but please open a new tracking issue for them when you send a PR. |
…acrum Update stdarch submodule Splits up rust-lang#27731 into multiple tracking issues. Closes rust-lang#27731
…ulacrum Update stdarch submodule Splits up rust-lang#27731 into multiple tracking issues. Closes rust-lang#27731
…acrum Update stdarch submodule Splits up rust-lang#27731 into multiple tracking issues. Closes rust-lang#27731
…acrum Update stdarch submodule Splits up rust-lang#27731 into multiple tracking issues. Closes rust-lang#27731
@workingjubilee bors has run rampant with power, this issue isn't done. |
The commit explicitly says that this issue got split into various smaller issues. The question is rather where are they? |
Hashbrown depends on ahash which used to use feature(stdsimd) which as been removed in rust-lang/rust#27731. Latest hashbrown bumps ahash which doesn't use the removed feature anymore.
|
This comment was marked as outdated.
This comment was marked as outdated.
…acrum Update stdarch submodule Splits up rust-lang#27731 into multiple tracking issues. Closes rust-lang#27731
This is a tracking issue for the unstable
core_simd
feature in the standard library. SIMD support is quite a thorny topic, but it's largely covered in rust-lang/rfcs#1199, being implemented in #27169, and @huonw will be creating an external crate for full-fledged SIMD support.cc @huonw
The text was updated successfully, but these errors were encountered: