rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.16k stars 12.69k forks source link

Tracking issue for integer_atomics #99069

Open tamird opened 2 years ago

tamird commented 2 years ago

Tracking https://github.com/rust-lang/rfcs/pull/1543.

Seems related to https://github.com/rust-lang/rust/issues/56071. See also https://github.com/rust-lang/rust/pull/57425 which stabilized other atomic integer widths.

RalfJung commented 2 years ago

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.

kadiwa4 commented 2 years ago

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.)

eric-seppanen commented 1 year ago

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

kadiwa4 commented 1 year ago

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 commented 1 year ago

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

gitmalong commented 1 year ago

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

HayleyDeckers commented 5 months ago

Now that https://github.com/rust-lang/rust/pull/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 commented 5 months ago

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

Amanieu commented 5 months ago

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 commented 5 months ago

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.

joshtriplett commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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

eric-seppanen commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

As for the correctness of the current implementations:

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

[^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 commented 1 month ago

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

Reported as #130474

cuviper commented 1 month ago

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. :)

beetrees commented 1 month ago

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?