Add integer atomic types

[Amanieu]

Tracking issue: #32976
RFC: rust-lang/rfcs#1543

The changes to AtomicBool in the RFC are not included, they are in a separate PR (#32365).

[rust-highfive]

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

[Amanieu]

r? @alexcrichton

[alexcrichton]

Thanks @Amanieu! Some thoughts of mine:

• Can you make sure the #[cfg(target_has_atomic)] starts out as unstable as well? The gating here should be similar to things like target_vendor.
• Would it be possible to define Atomic{I,U}size as part of the same atomic_int! macro?

[alexcrichton]

Just confirming, but this is inherent to aarch64 (e.g. 128-bit atomics are available everywhere?)

[Amanieu]

Yes

[alexcrichton]

Gah, I hit enter too quickly...

• Would it be possible to detect cmpxchg16b for on x86_64 for more CPUs? Something like querying LLVM? It seems like that'd be a major use case for this, but it's only supported on OSX right now.
• To confirm, platforms like MIPS only have 32-bit atomics but we're saying they have 8-bit atomics, right? Are we really sure that these don't lower to calls in compiler-rt?

I wonder if this can also try to have a more comprehensive test? We don't run tests for platforms like ARM/MIPS/etc, but it'd be good to exercise those platforms as they're quite relevant here.

Also, cc @rust-lang/libs, @brson

[Amanieu]

Would it be possible to define Atomic{I,U}size as part of the same atomic_int! macro?

The reason I left them out is because they use different stability attributes (rust1, extended_compare_and_swap and atomic_debug), whereas all the new atomic types are under the integer_atomics feature.

Would it be possible to detect cmpxchg16b for on x86_64 for more CPUs? Something like querying LLVM? It seems like that'd be a major use case for this, but it's only supported on OSX right now.

This is a bit complicated because some early AMD x86_64 CPUs don't support cmpxchg16b. Support for this instruction can be enabled in LLVM by targeting a CPU that supports it (which the OSX target does with core2), or by enabling the cx16 LLVM feature.

I don't think there is a way to extract the maximum atomic size from LLVM's API. I've mostly copied what Clang does, which is to determine it from the target architecture and any compilation flags passed in (-march, -mcx16, etc).

To confirm, platforms like MIPS only have 32-bit atomics but we're saying they have 8-bit atomics, right? Are we really sure that these don't lower to calls in compiler-rt?

Yes, these will never be lowered to compiler-rt calls. The test should be able to catch any cases that are lowered to libcalls.

I wonder if this can also try to have a more comprehensive test? We don't run tests for platforms like ARM/MIPS/etc, but it'd be good to exercise those platforms as they're quite relevant here.

How would you suggest doing this?

[Amanieu]

Can you make sure the #[cfg(target_has_atomic)] starts out as unstable as well? The gating here should be similar to things like target_vendor.

Should I reuse the integer_atomics feature gate for this or add a new cfg_target_has_atomic feature gate?

[alexcrichton]

The reason I left them out is because they use different stability attributes

Right yeah, but perhaps that could be an input to the macro? (e.g. the macro takes a stability attribute as input)

I've mostly copied what Clang does

Only statically, though, right? If I pass -C target-cpu=native this won't enable cmpxchg16b?

The test should be able to catch any cases that are lowered to libcalls.

Unfortunately this test won't catch anything because it's only run on x86_64 and x86 for now.

How would you suggest doing this?

A "minimal cross-compiled libcore" could probably suffice. Something with a few lang items enough to just generate an object file should be enough. You could then call the intrinsics directly to ensure they're not lowered to libcalls.

[Amanieu]

Only statically, though, right? If I pass -C target-cpu=native this won't enable cmpxchg16b?

No, for that we would need to be able to recognize all the CPU variant and set the appropriate LLVM flags for them. See Clang: http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130930/090167.html

[Amanieu]

Should we expose compare_and_swap on the new atomic types? The plan is to deprecate it in favor of compare_exchange.

[Amanieu]

Also, should I add constants like ATOMIC_U8_INIT for all the new integer types?

[Amanieu]

Right yeah, but perhaps that could be an input to the macro? (e.g. the macro takes a stability attribute as input)

Done

A "minimal cross-compiled libcore" could probably suffice. Something with a few lang items enough to just generate an object file should be enough. You could then call the intrinsics directly to ensure they're not lowered to libcalls.

Done

[alexcrichton]

No, for that we would need to be able to recognize all the CPU variant and set the appropriate LLVM flags for them. See Clang: http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130930/090167.html

Yeah that's kinda unfortunate for now, although we're not even exposing the 128-bit types just yet, so it's ok to punt on this for now.

Should we expose compare_and_swap on the new atomic types?

Nah I wouldn't bother. If it's easier to write the macro where they all have compare_and_swap (as isize/usize must have it) then that's also fine (no skin off our back).

Also, should I add constants like ATOMIC_U8_INIT for all the new integer types?

Indeed!

---

Awesome, thanks for the updates!

[Amanieu]

I've added the constants, is there anything else to do?

[alexcrichton]

Looks good to me, thanks @Amanieu! I want to double check with @rust-lang/libs about the #[cfg(target_has_atomic = "ptr")] for bool/isize/usize/ptr, but I suspect we'll be ok with it for now.

[ollie27]

Are the constants not going to be deprecated once const fn is stabilized?

[alexcrichton]

They likely will be, yes, but const fn isn't currently stable so some stable alternative is needed

[arielb1]

../src/libsyntax/feature_gate.rs:272:5: 272:6 error: no rules expected the token `(`
../src/libsyntax/feature_gate.rs:272 (active, cfg_target_has_atomic, "1.10.0", Some(32976)),
^
make: *** [x86_64-apple-darwin/stage0/lib/rustlib/x86_64-apple-darwin/lib/stamp.syntax] Error 101
make: *** Waiting for unfinished jobs....

missing comma

[Amanieu]

Fixed

[alexcrichton]

@bors: r+ 1a6530803e1c38a105274d0554306fede183d9ce

[bors]

⌛ Testing commit 1a65308 with merge 4a133c2...

[bors]

💔 Test failed - auto-linux-64-cross-armsf

[Amanieu]

Sorry about that, fixed again.

[alexcrichton]

@bors: r+ 033699ca561a7e8d2f33eee392a3a8a7373e891b

[bors]

⌛ Testing commit 033699c with merge 780897b...

[bors]

💔 Test failed - auto-win-gnu-32-opt-rustbuild

[bors]

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

[Amanieu]

Rebased and changed the test to only run on Linux.

[alexcrichton]

@bors: r+ 03dd9b8

[bors]

⌛ Testing commit 03dd9b8 with merge af0a433...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-32cross-opt, auto-linux-64-cargotest, auto-linux-64-cross-armhf, auto-linux-64-cross-armsf, auto-linux-64-cross-freebsd, auto-linux-64-cross-netbsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild