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

Refactor stdsimd #640

Merged
merged 1 commit into from
Jan 22, 2019
Merged

Refactor stdsimd #640

merged 1 commit into from
Jan 22, 2019

Conversation

gnzlbg
Copy link
Contributor

@gnzlbg gnzlbg commented Jan 21, 2019

This commit:

  • renames coresimd to core_arch and stdsimd to std_detect

  • std_detect does no longer depend on core_arch - it is a freestanding
    no_std (on some archs) library that only depends on core

  • moves the top-level coresimd and stdsimd directories into the appropriate
    crates/... directories - this simplifies creating crate.io releases of these crates

  • moves the top-level coresimd and stdsimd sub-directories into their
    corresponding crates in crates/{core_arch, std_detect}.

  • moves the examples into its own crate at examples/ to reduce the dev-dependencies of std_detect to a minimum

After this PR lands rust-lang/rust#57808 could land. The main benefit is that we no longer build core::arch twice in rust-lang/rust, once in libstd, and once in libcore. Instead, these two PR change that to only build it once in libcore, and then libstd just re-export the core::arch module. The run-time feature detection is provided in libstd by just adding std_detet as a dependency, hopefully, without much hackery (since it is a #[no_std] library it's only a #[no_std] library on some archs, so we'll see how this goes). This also allows those #[no_std] users that want to have the exact same run-time feature detection system as libstd to be able to do so by just adding std_detect as a dependency.

@@ -78,7 +78,8 @@ struct Cache(AtomicU64);
impl Cache {
/// Creates an uninitialized cache.
const fn uninitialized() -> Self {
Cache(AtomicU64::new(u64::max_value()))
const X: AtomicU64 = AtomicU64::new(u64::max_value());
Self(X)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Centril @oli-obk without this change std_detect fails to compile, which is weird. The error is "can only call other min_const_fn within a min_const_fn" but the crate has feature(const_fn) enabled =/

Copy link

Choose a reason for hiding this comment

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

(repeating from Discord: I wasn't able to reproduce either... I'll see if I have time to test this locally; for the time being, this hack is fine, and AtomicU64 is stable in 1.34 so hopefully that makes the hack go away... However, when using the hack in the future, please cc myself and @oli-obk...)

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jan 21, 2019

So @alexcrichton this should be all green now. In some targets std_detect needs to depend on std, to use auxv/cpuinfo/ etc. Is that possible at all when adding std_detect as a normal dependency to libstd ? In the future we might want to add it a use_std feature that only exposes no_std functionality.

This commit:

* renames `coresimd` to `core_arch` and `stdsimd` to `std_detect`

* `std_detect` does no longer depend on `core_arch` - it is a freestanding
  `no_std` library that only depends on `core` - it is renamed to `std_detect`

* moves the top-level coresimd and stdsimd directories into the appropriate
  crates/... directories - this simplifies creating crate.io releases of these crates

* moves the top-level `coresimd` and `stdsimd` sub-directories into their
  corresponding crates in `crates/{core_arch, std_detect}`.
@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jan 22, 2019

I am merging this. We can iterate on this in tree.

@gnzlbg gnzlbg merged commit 4c66d96 into rust-lang:master Jan 22, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 29, 2019
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