-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Stabilize x86/x86_64 SIMD #49664
Stabilize x86/x86_64 SIMD #49664
Conversation
Note it's not intended that this should merge before the FCP lapses in #48556, but rather this is submitted early to get some more time for review |
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
01ec2d0
to
6de696a
Compare
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Doesn't this stabilise all the (whitelisted) feature names for all the architectures, or am I mistaken? The RFC mentioned this specific set https://github.com/rust-lang/rfcs/blob/master/text/2325-stable-simd.md#the-target_feature-attribute. |
☔ The latest upstream changes (presumably #48851) made this pull request unmergeable. Please resolve the merge conflicts. |
.gitmodules
Outdated
@@ -49,7 +49,7 @@ | |||
url = https://github.com/rust-lang/llvm | |||
[submodule "src/stdsimd"] | |||
path = src/stdsimd | |||
url = https://github.com/rust-lang-nursery/stdsimd | |||
url = https://github.com/alexcrichton/stdsimd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why doesn't this still use rust-lang-nursery?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this isn't ready to land yet, it's waiting on FCP to finish.
What is the difference between the
Why do we have a macro specific to x86? Does that also apply to the |
From IRC last night:
|
There is a module per
As explained by @steveklabnik the macro is x86-specific in its feature detection and list of accepted features. For convenience there is no |
ffcd619
to
8a02174
Compare
So are there modules for i286, i386, i486, i586, i786 too? It doesn't seem terribly useful to have this many variants of x86. What is the justification for this?
That doesn't answer the question. Why do we have platform specific macros instead of a single macro? and why does the justification for that not apply to |
@parched ah yes, thanks! I've pushed a commit which should allow us to tweak the stability of each target feature.
No, there are not all those modules. I'd recommend reading the RFC and the issue tracker for stdsimd for rationale here. There are only modules for each
The answer to your question was indicated in the IRC logs, a platform-independent macro in no way makes SIMD easier to use. Previous attempts at using the same name and same namespace for attributes didn't provide any ergonomics so a platform-specific macro is the conservative option. |
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@alexcrichton Did you have a typo in this sentence?
I would have expected either of:
|
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Er oops sorry yeah, @rkruppe you are correct! (both of those alternatives are true) |
I'd expect to have
How does having a platform-independent macro make SIMD harder to use?
By that logic we should also have platform-specific versions of |
edf2362
to
ca8a7ca
Compare
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
ca8a7ca
to
a97e381
Compare
This does not match
You've misread that, I said that it wasn't easier to use, not harder to use. |
9d51780
to
a97e381
Compare
📌 Commit b7a4532 has been approved by |
⌛ Testing commit b7a453287867cfc10d6d03bfccad00aad09e711f with merge 2d76bee9ca65c77b4f42a13ced55d2f43b5e03ef... |
💔 Test failed - status-appveyor |
Use an explicit whitelist for what features are actually stable and can be enabled.
b7a4532
to
1217d70
Compare
@bors: r=BurntSushi |
📌 Commit 1217d70 has been approved by |
Stabilize x86/x86_64 SIMD This commit stabilizes the SIMD in Rust for the x86/x86_64 platforms. Notably this commit is stabilizing: * The `std::arch::{x86, x86_64}` modules and the intrinsics contained inside. * The `is_x86_feature_detected!` macro in the standard library * The `#[target_feature(enable = "...")]` attribute * The `#[cfg(target_feature = "...")]` matcher Stabilization of the module and intrinsics were primarily done in rust-lang/stdarch#414 and the two attribute stabilizations are done in this commit. The standard library is also tweaked a bit with the new way that stdsimd is integrated. Note that other architectures like `std::arch::arm` are not stabilized as part of this commit, they will likely stabilize in the future after they've been implemented and fleshed out. Similarly the `std::simd` module is also not being stabilized in this commit, only `std::arch`. Finally, nothing related to `__m64` is stabilized in this commit either (MMX), only SSE and up types and intrinsics are stabilized. Closes #29717 Closes #44839 Closes #48556
☀️ Test successful - status-appveyor, status-travis |
This commit stabilizes the SIMD in Rust for the x86/x86_64 platforms. Notably
this commit is stabilizing:
std::arch::{x86, x86_64}
modules and the intrinsics contained inside.is_x86_feature_detected!
macro in the standard library#[target_feature(enable = "...")]
attribute#[cfg(target_feature = "...")]
matcherStabilization of the module and intrinsics were primarily done in
rust-lang/stdarch#414 and the two attribute stabilizations are done in
this commit. The standard library is also tweaked a bit with the new way that
stdsimd is integrated.
Note that other architectures like
std::arch::arm
are not stabilized as partof this commit, they will likely stabilize in the future after they've been
implemented and fleshed out. Similarly the
std::simd
module is also not beingstabilized in this commit, only
std::arch
. Finally, nothing related to__m64
is stabilized in this commit either (MMX), only SSE and up types and intrinsics
are stabilized.
Closes #29717
Closes #44839
Closes #48556