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

Update stdsimd to undo an accidental stabilization #52535

Merged
merged 1 commit into from
Jul 21, 2018

Conversation

alexcrichton
Copy link
Member

Closes #52403

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 19, 2018
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

carrying over approval from #52429

@bors
Copy link
Contributor

bors commented Jul 19, 2018

📌 Commit ec4aaf1 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 19, 2018
@alexcrichton
Copy link
Member Author

This error was suspected to be caused by this update, so I'm running a test locally

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Jul 20, 2018
…-Simulacrum

Update stdsimd to undo an accidental stabilization

Closes rust-lang#52403
@kennytm
Copy link
Member

kennytm commented Jul 20, 2018

The same powerpc error in #52570 (comment), but again it is a rollup 🤷.

@alexcrichton
Copy link
Member Author

@bors: r-

I'll take a look

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 20, 2018
alexcrichton added a commit to alexcrichton/stdarch that referenced this pull request Jul 20, 2018
We're running into issues updating with rust-lang/rust#52535, so we need to get
this working without `RUSTFLAGS` enabling the `altivec` feature
alexcrichton added a commit to rust-lang/stdarch that referenced this pull request Jul 20, 2018
We're running into issues updating with rust-lang/rust#52535, so we need to get
this working without `RUSTFLAGS` enabling the `altivec` feature
@alexcrichton
Copy link
Member Author

@gnzlbg as a heads up I had to update this to stdsimd master to pull in rust-lang/stdarch#531 which also pulls in the deletion of std::simd

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jul 20, 2018

📌 Commit a25dfa4dacb39d892ab7ff6ab2dda4d30f260ad9 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 20, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 20, 2018 via email

@bors
Copy link
Contributor

bors commented Jul 20, 2018

@gnzlbg: 🔑 Insufficient privileges: Not in reviewers

@alexcrichton
Copy link
Member Author

@gnzlbg oh I thought I recalled you saying you didn't want to delete std::simd just yet while you waited to publish your first version, but I may be misremembering! Other than that it sounds great! If you'd like I can move it to the nursery as well

@bors
Copy link
Contributor

bors commented Jul 21, 2018

⌛ Testing commit a25dfa4dacb39d892ab7ff6ab2dda4d30f260ad9 with merge ba65701428f1c1ae53aee962ac32bd0771d4e922...

@bors
Copy link
Contributor

bors commented Jul 21, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 21, 2018
@bors
Copy link
Contributor

bors commented Jul 21, 2018

⌛ Testing commit d77defc with merge 17eb392...

bors added a commit that referenced this pull request Jul 21, 2018
Update stdsimd to undo an accidental stabilization

Closes #52403
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 21, 2018

@alexcrichton

oh I thought I recalled you saying you didn't want to delete std::simd just yet while you waited to publish your first version, but I may be misremembering! Other than that it sounds great!

You are remembering it correctly, but I changed my mind about doing releases. Those who want to use this will need to add a git dependency on it, which ties them to nightly, and forbids them from releasing stuff.

That's unfortunate, but it will prevent the issue that we had with faster where the compiler moved on, and faster and all their dependencies stopped working because the released versions of coresimd became unsupported.

If you'd like I can move it to the nursery as well

I think that would be best. I guess one just needs to fork it right?

@bors
Copy link
Contributor

bors commented Jul 21, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: Mark-Simulacrum
Pushing 17eb392 to master...

@bors bors merged commit d77defc into rust-lang:master Jul 21, 2018
@alexcrichton alexcrichton deleted the update-stdsimd branch July 21, 2018 15:05
@alexcrichton
Copy link
Member Author

@gnzlbg ok, if you want to catch me on IRC I can help you transfer the repo to the nursery

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 21, 2018 via email

@@ -244,9 +244,6 @@ macro_rules! vector_impl { ($([$f:ident, $($args:tt)*]),*) => { $($f!($($args)*)
#[cfg(not(stage0))] // allow changes to how stdsimd works in stage0
mod coresimd;

#[unstable(feature = "stdsimd", issue = "48556")]
#[cfg(not(stage0))]
pub use coresimd::simd;
Copy link
Contributor

Choose a reason for hiding this comment

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

This removal broke the CI of rand, but I have trouble finding the reason. Is there an alternative to use simd with no_std, or is this no longer supported?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed! This has moved over to https://github.com/gnzlbg/packed_simd

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!

@nnethercote
Copy link
Contributor

perf.rust-lang.org indicates that this caused a significant compile-time win, which is (a) nice, and (b) surprising to me.

@alexcrichton, do you know why this might have improved rustc speed?

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 25, 2018

@nnethercote we removed a very big module from stdsimd: rust-lang/stdarch#528 but I don't know how could that make rustc compile things faster. AFAICT none of the benchmarks being compiled there were using that module (otherwise they would have stopped compiling when we removed it).

This might mean that adding stuff to the std library incurs a compile-time cost for users, even if they don't use the stuff, which would be very bad.

@alexcrichton
Copy link
Member Author

Yes this removed a large unstable module from libcore. I believe it had a ton of trait impls, so maybe metadata loading for libstd is faster now? It could mean perhaps that metadata loading isn't as efficient as it could otherwise be perhaps?

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 25, 2018

I believe it had a ton of trait impls,

This is correct (it really had a ton of trait impls). It also had a ton of types with tons of methods.

FYI what was removed now lives in the nursery: https://github.com/rust-lang-nursery/packed_simd

@MoSal
Copy link

MoSal commented Jul 25, 2018

FYI, while it's obviously removed from the nightly documentation, std::simd is still in the official documentation here.

This may confuse some people (e.g. game developers) who are googling "Rust SIMD".

@steveklabnik

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 25, 2018

That's weird, the APIs are unstable so they should only appear on the nightly docs, but those are not in nightly/... :/

@MoSal
Copy link

MoSal commented Jul 25, 2018

@gnzlbg

Unstable APIs appear in stable docs. Maybe they shouldn't, but they do. And maybe it's too late to change that behavior.

Edit: A possible improvement would be to not show unstable modules, but continue showing unstable functions/methods. But that's a topic that may deserve its own issue.

@nnethercote
Copy link
Contributor

so maybe metadata loading for libstd is faster now?

Seems plausible... especially given that the speedups were largest for short-running benchmarks (esp. "check" builds) which suggests a constant-ish time reduction.

I intended to do a Cachegrind diff but when I ran locally I saw a negligible difference caused by the change. I don't know why.

SimonSapin added a commit to servo/servo that referenced this pull request Jul 27, 2018
SimonSapin added a commit to servo/servo that referenced this pull request Jul 27, 2018
SimonSapin added a commit to servo/servo that referenced this pull request Jul 27, 2018
bors-servo pushed a commit to servo/servo that referenced this pull request Jul 28, 2018
Use the packed_simd crate instead of std::simd

`std::simd` was removed in rust-lang/rust#52535.
https://crates.io/crates/packed_simd is in the rust-lang-nursery org.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21272)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Jul 28, 2018
Use the packed_simd crate instead of std::simd

`std::simd` was removed in rust-lang/rust#52535.
https://crates.io/crates/packed_simd is in the rust-lang-nursery org.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21272)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Jul 28, 2018
Use the packed_simd crate instead of std::simd

`std::simd` was removed in rust-lang/rust#52535.
https://crates.io/crates/packed_simd is in the rust-lang-nursery org.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21272)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Jul 29, 2018
Use the packed_simd crate instead of std::simd

`std::simd` was removed in rust-lang/rust#52535.
https://crates.io/crates/packed_simd is in the rust-lang-nursery org.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21272)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants