4

Stabilise `is_aarch64_feature_detected!` under `simd_aarch64` feature by adamge...

 2 years ago
source link: https://github.com/rust-lang/rust/pull/90271
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Conversation

Copy link

Contributor

adamgemmell commented on Oct 25, 2021

Initial implementation, looking for feedback on the approach here. #86941

One point I noticed was that I haven't seen different "since" versions for the same feature - does this mean that other features can't be added to to the simd_aarch64 feature once this is in stable? If so it might need a more specific name.

r? @Amanieu

Copy link

Contributor

Author

adamgemmell commented on Oct 25, 2021

CI should fail until the stdarch PR is committed and updated here

Copy link

Contributor

Amanieu commented on Oct 25, 2021

We can have different since versions for the same feature. So adding new target features in the future is not a problem.

Copy link

Contributor

bors commented on Jan 5

umbrella The latest upstream changes (presumably #92587) made this pull request unmergeable. Please resolve the merge conflicts.

@@ -6,7 +6,6 @@

all(target_arch = "aarch64", any(target_os = "linux", target_os = "android")),

all(target_arch = "powerpc", target_os = "linux"),

all(target_arch = "powerpc64", target_os = "linux"),

any(target_arch = "x86", target_arch = "x86_64"),

Copy link

Contributor

@Amanieu Amanieu 8 days ago

Should this be removed for aarch64 as well? You might need to put this behind cfg(bootstrap) so the stage0 compiler still accepts it.

Copy link

Contributor

Author

@adamgemmell adamgemmell 7 days ago

True - I mistakenly removed it when making pauth temporarily unstable again.

Copy link

Contributor

Author

@adamgemmell adamgemmell 7 days ago

edited

Actually, it does need to remain there while pauth is unstable. I'll fix it up when rust-lang/stdarch#1259 is merged, and update the submodule at the same time.

adamgemmell

marked this pull request as ready for review

7 days ago

Copy link

Contributor

Amanieu commented 7 days ago

@bors r+ rollup=never

Copy link

Contributor

bors commented 7 days ago

pushpin Commit 93b5bfb has been approved by Amanieu

Copy link

Contributor

bors commented 6 days ago

hourglass Testing commit 93b5bfb with merge e789f3a...

Copy link

Contributor

bors commented 5 days ago

sunny Test successful - checks-actions
Approved by: Amanieu
Pushing e789f3a to master...

bors

merged commit e789f3a into

rust-lang:master 5 days ago

11 checks passed

rustbot

added this to the 1.60.0 milestone

5 days ago

Copy link

Collaborator

rust-timer commented 5 days ago

Finished benchmarking commit (e789f3a): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Copy link

Contributor

ehuss commented 4 days ago

Just a heads up, the stabilization machinery may cause confusion here. The docs say this is still unstable: https://doc.rust-lang.org/nightly/std/arch/macro.is_aarch64_feature_detected.html

This is because the https://doc.rust-lang.org/ docs are built for x86_64. For some reason, using the macro from an incompatible target raises an unstable error. For example, using is_aarch64_feature_detected on x86_64 will display:

error[E0658]: use of unstable library feature 'stdsimd'
 --> src/main.rs:2:8
  |
2 |     if std::arch::is_aarch64_feature_detected!("paca") {
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #27731 <https://github.com/rust-lang/rust/issues/27731> for more information
  = help: add `#![feature(stdsimd)]` to the crate attributes to enable

error:
               is_aarch64_feature_detected can only be used on AArch64 targets.
               You can prevent it from being used in other architectures by
               guarding it behind a cfg(target_arch) as follows:

                   #[cfg(target_arch = "aarch64")] {
                       if is_aarch64_feature_detected(...) { ... }
                   }

 --> src/main.rs:2:8
  |
2 |     if std::arch::is_aarch64_feature_detected!("paca") {
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `std::arch::is_aarch64_feature_detected` (in Nightly builds, run with -Z macro-backtrace for more info)

Is that intentional?

adamgemmell

deleted the dev/feat-detect-stabilise branch

3 days ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Amanieu

Assignees

Amanieu

Projects

None yet

Milestone

1.60.0

Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK