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

rustc_data_structures: Explicitly check for 64-bit atomics support #127075

Merged
merged 1 commit into from
Jun 29, 2024

Conversation

glaubitz
Copy link
Contributor

@glaubitz glaubitz commented Jun 28, 2024

Instead of keeping a list of architectures which have native support
for 64-bit atomics, just use #[cfg(target_has_atomic = "64")] and its
inverted counterpart to determine whether we need to use portable
AtomicU64 on the target architecture.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 28, 2024
@taiki-e
Copy link
Member

taiki-e commented Jun 28, 2024

FWIW, I think we can use #[cfg(target_has_atomic = "64")]/#[cfg(not(target_has_atomic = "64"))] instead of listing concrete architecture names.

@glaubitz
Copy link
Contributor Author

FWIW, I think we can use #[cfg(target_has_atomic = "64")]/#[cfg(not(target_has_atomic = "64"))] instead of listing concrete architecture names.

Good idea. Can we use this in the Cargo.toml as well?

@taiki-e
Copy link
Member

taiki-e commented Jun 28, 2024

Can we use this in the Cargo.toml as well?

Yes. (For example, cargo and pyo3 uses it in Cargo.toml.)

@glaubitz
Copy link
Contributor Author

Can we use this in the Cargo.toml as well?

Yes. (For example, cargo and pyo3 uses it in Cargo.toml.)

OK, testing this now. Thanks a lot for the suggestion!

@SparrowLii
Copy link
Member

Thanks! If #[cfg(target_has_atomic = "64")] works it's the best
r? @SparrowLii

@rustbot rustbot assigned SparrowLii and unassigned nagisa Jun 28, 2024
@SparrowLii
Copy link
Member

@bors delegate+
r=me after your testing

@bors
Copy link
Contributor

bors commented Jun 28, 2024

✌️ @glaubitz, you can now approve this pull request!

If @SparrowLii told you to "r=me" after making some further change, please make that change, then do @bors r=@SparrowLii

Instead of keeping a list of architectures which have native support
for 64-bit atomics, just use #[cfg(target_has_atomic = "64")] and its
inverted counterpart to determine whether we need to use portable
AtomicU64 on the target architecture.
@glaubitz glaubitz changed the title rustc_data_structures: Fix copy-and-paste error from previous change rustc_data_structures: Explicitly check for 64-bit atomics support Jun 28, 2024
@glaubitz
Copy link
Contributor Author

@bors r=@SparrowLii

@bors
Copy link
Contributor

bors commented Jun 28, 2024

📌 Commit ab1b48e has been approved by SparrowLii

It is now in the queue for this repository.

@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 Jun 28, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 28, 2024
…rrowLii

rustc_data_structures: Explicitly check for 64-bit atomics support

Instead of keeping a list of architectures which have native support
for 64-bit atomics, just use #[cfg(target_has_atomic = "64")] and its
inverted counterpart to determine whether we need to use portable
AtomicU64 on the target architecture.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2024
…llaumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#123714 (Add test for fn pointer duplication.)
 - rust-lang#124091 (Update AST validation module docs)
 - rust-lang#126963 (Add basic Serde serialization capabilities to Stable MIR)
 - rust-lang#127015 (Switch back `non_local_definitions` lint to allow-by-default)
 - rust-lang#127016 (docs: check if the disambiguator matches its suffix)
 - rust-lang#127029 (Fix Markdown tables in platform-support.md)
 - rust-lang#127032 (Enable const casting for `f16` and `f128`)
 - rust-lang#127041 (Migrate `run-make/override-aliased-flags` to `rmake.rs`)
 - rust-lang#127045 (Rename `super_predicates_of` and similar queries to `explicit_*` to note that they're not elaborated)
 - rust-lang#127075 (rustc_data_structures: Explicitly check for 64-bit atomics support)

r? `@ghost`
`@rustbot` modify labels: rollup
@glaubitz
Copy link
Contributor Author

@SparrowLii This seems to be fallen out of the merge queue. Any ideas?

@SparrowLii
Copy link
Member

SparrowLii commented Jun 29, 2024

@glaubitz Seems not related to this PR
https://github.com/rust-lang-ci/rust/actions/runs/9711788661/job/26805261710
or
plain

 thread 'main' panicked at src/lib.rs:1712:17:
  failed to copy `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe` to `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools-bin\miri.exe`: The process cannot access the file because it is being used by another process. (os error 32)
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Build completed unsuccessfully in 0:00:09
  local time: Fri, Jun 28, 2024 12:32:23 PM
  network time: Fri, 28 Jun 2024 12:32:23 GMT
Error: Process completed with exit code 1.

Just wait for #127111 to merge

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 29, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126822 (Bootstrap command refactoring: port more `Command` usages to `BootstrapCmd` (step 2))
 - rust-lang#126835 (Simplifications in match lowering)
 - rust-lang#126953 (std: separate TLS key creation from TLS access)
 - rust-lang#127045 (Rename `super_predicates_of` and similar queries to `explicit_*` to note that they're not elaborated)
 - rust-lang#127075 (rustc_data_structures: Explicitly check for 64-bit atomics support)
 - rust-lang#127101 (remove redundant match statement from dataflow const prop)
 - rust-lang#127102 (Rename fuchsia builder and bump Fuchsia)
 - rust-lang#127103 (Move binder and polarity parsing into `parse_generic_ty_bound`)
 - rust-lang#127108 (unify `dylib` and `bin_helpers` and create `shared_helpers::parse_value_from_args`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3369e83 into rust-lang:master Jun 29, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 29, 2024
Rollup merge of rust-lang#127075 - glaubitz:copy-and-paste-fix, r=SparrowLii

rustc_data_structures: Explicitly check for 64-bit atomics support

Instead of keeping a list of architectures which have native support
for 64-bit atomics, just use #[cfg(target_has_atomic = "64")] and its
inverted counterpart to determine whether we need to use portable
AtomicU64 on the target architecture.
// MIPS, PowerPC and SPARC platforms with 32-bit pointers do not
// have AtomicU64 type.
#[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc", target_arch = "sparc")))]
// Use portable AtomicU64 for targets without native 64-bit atomics

Choose a reason for hiding this comment

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

Wrong comment position, should be placed before line 156.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment applies to both the following statements, see the comment from the previous version.

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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants