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

Tracking issue for integer_atomics #99069

Open
2 tasks
tamird opened this issue Jul 8, 2022 · 20 comments
Open
2 tasks

Tracking issue for integer_atomics #99069

tamird opened this issue Jul 8, 2022 · 20 comments
Labels
A-atomic Area: Atomics, barriers, and sync primitives A-concurrency Area: Concurrency C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@tamird
Copy link
Contributor

tamird commented Jul 8, 2022

Tracking rust-lang/rfcs#1543.

  • std::sync::atomic::AtomicI128
  • std::sync::atomic::AtomicU128

Seems related to #56071. See also #57425 which stabilized other atomic integer widths.

@tamird tamird added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 8, 2022
Dylan-DPC pushed a commit to Dylan-DPC/rust that referenced this issue Jul 9, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 9, 2022
…fJung

Update integer_atomics tracking issue

Updates rust-lang#32976.
Updates rust-lang#99069.

r? `@RalfJung`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 9, 2022
…fJung

Update integer_atomics tracking issue

Updates rust-lang#32976.
Updates rust-lang#99069.

r? ``@RalfJung``
@RalfJung
Copy link
Member

RalfJung commented Jul 9, 2022

It'd be good to list the APIs that are covered by this tracking issue, and copy any open questions that have been noted in the RFC of the previous tracking issue.

JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jul 26, 2022
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jul 26, 2022
…fJung

Update integer_atomics tracking issue

Updates rust-lang#32976.
Updates rust-lang#99069.

r? ``@RalfJung``
@kadiwa4
Copy link
Contributor

kadiwa4 commented Jul 28, 2022

The unstable ATOMIC_*_INIT constants can be removed, right? Or should they be stabilized for consistency with the stable ATOMIC_*_INIT constants? (All of them are deprecated.)

@workingjubilee workingjubilee added A-concurrency Area: Concurrency A-atomic Area: Atomics, barriers, and sync primitives labels Aug 17, 2022
@eric-seppanen
Copy link

The standard library docs for AtomicU128, etc. still point to #32976 instead of here.

@kadiwa4
Copy link
Contributor

kadiwa4 commented Jan 17, 2023

What do you mean?
image
(rust 1.66.1)

Interestingly, in the online documentation the types don't show up at all, whereas they do show up in my local copy (rust-docs from rustup).

@eric-seppanen
Copy link

Apologies, I must have accidentally followed a link to somebody else's stale docs.

@gitmalong
Copy link

the trait bound std::sync::atomic::AtomicU128: mymod::_::_serde::Deserialize<'_> is not satisfied. Where to fill an issue for that?

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 21, 2023
…=Amanieu

remove the unstable `core::sync::atomic::ATOMIC_*_INIT` constants

Tracking issue: rust-lang#99069

It would be weird to ever stabilise these as they are already deprecated.
@HayleyDeckers
Copy link

Now that #120820 has been merged, there are multiple T1 targets that support 128-bit atomics: x86_64-apple-darwin, x86_64-pc-windows-msvc and, x86_64-pc-windows-gnu.

as well as a few lower-tier targets such as aarch64-unknown-linux-musl, arm64e-apple-darwin and aarch64-unknown-uefi .

@HayleyDeckers
Copy link

@m-ou-se @Amanieu Are there any remaining blockers for this that need to be addressed?

@m-ou-se m-ou-se added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label May 21, 2024
@Amanieu Amanieu added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels May 21, 2024
@Amanieu
Copy link
Member

Amanieu commented May 21, 2024

This will stabilize AtomicI128 and AtomicU128 for AArch64 and x86_64 Windows/Apple targets. Notably, AtomicI128 is not available on x86_64 Linux targets because they target a version of x86_64 without cmpxchg16b. On those targets, you can check for support at runtime and use the cmpxchg16b intrinsic.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented May 21, 2024

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 21, 2024
@joshtriplett
Copy link
Member

@Amanieu Is this serving as a precedent for APIs that only work on a subset of targets?

To be clear, I'm all for exposing target-specific APIs wherever appropriate. I want to make sure we apply the policy this FCP is implying consistently. :)

@Amanieu
Copy link
Member

Amanieu commented May 21, 2024

Atomic types are already target-specific: AtomicU64 isn't available on some 32-bit targets for example. This isn't making things worse except that this difference is now noticeable on more common platforms.

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label May 21, 2024
@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label May 21, 2024
@rfcbot
Copy link

rfcbot commented May 21, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@eric-seppanen
Copy link

AtomicI128 is not available on x86_64 Linux targets because they target a version of x86_64 without cmpxchg16b.

That's disappointing. Where can I learn more about this constraint and/or ideas for resolving it?

@ChrisDenton
Copy link
Member

IIRC, the issue is that the Linux kernel, unlike Mac and Windows, does not require cmpxchg16b. It was suggested that we could split the x86_64 Linux targets in two, one targeting a higher baseline than the other. But that's a compiler issue, not a libs issue.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 31, 2024
@rfcbot
Copy link

rfcbot commented May 31, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@taiki-e
Copy link
Member

taiki-e commented Jun 3, 2024

As for the correctness of the current implementations:

As for future 128-bit atomic support on other targets (not stabilization blockers):

Footnotes

  1. However, if future LLVM changes code generation to address performance issues with some chips (like https://github.com/taiki-e/portable-atomic/pull/89), it may be easily reproduced using Rust alone.

@programmerjake
Copy link
Member

Interestingly, in the online documentation the types don't show up at all

Reported as #130474

@cuviper
Copy link
Member

cuviper commented Sep 18, 2024

I'm updating the minimum to LLVM 18 in #130487.

We can try updating the s390x target spec separately though, after we make sure it's not also broken. :)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 19, 2024
Support 128-bit atomics on s390x

Since LLVM 18 (llvm/llvm-project@c568927), 128-bit atomics are fully supported on s390x. And the current minimum external LLVM version is now 18 (rust-lang#130487).

s390x 128-bit atomic instructions (lpq,stpq,cdsg) has been present since [the First Edition of the Principles of Operation](https://publibfp.dhe.ibm.com/epubs/pdf/dz9zr000.pdf). (LLVM's minimal supported architecture level [is z10 (the Eighth Edition of the PoP)](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZProcessors.td#L16-L17).)

cc rust-lang#99069

r? `@cuviper`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 19, 2024
Rollup merge of rust-lang#130558 - taiki-e:s390x-atomic-128, r=cuviper

Support 128-bit atomics on s390x

Since LLVM 18 (llvm/llvm-project@c568927), 128-bit atomics are fully supported on s390x. And the current minimum external LLVM version is now 18 (rust-lang#130487).

s390x 128-bit atomic instructions (lpq,stpq,cdsg) has been present since [the First Edition of the Principles of Operation](https://publibfp.dhe.ibm.com/epubs/pdf/dz9zr000.pdf). (LLVM's minimal supported architecture level [is z10 (the Eighth Edition of the PoP)](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZProcessors.td#L16-L17).)

cc rust-lang#99069

r? `@cuviper`
@beetrees
Copy link
Contributor

Since generic Atomic<T> is in the process of being added (tracking issue), would it make sense to stabilise this as just Atomic<u128> and Atomic<i128>, rather than stabilising AtomicU128 and AtomicI128 only to have them made obsolete by Atomic<T> shortly afterwards?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-atomic Area: Atomics, barriers, and sync primitives A-concurrency Area: Concurrency C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests