Skip to content

Commit 2a707ee

Browse files
committed
race: Relax compare_exchange success ordering from AcqRel to Release.
See the analogous change in rust-lang/rust#131746 and the discussion in matklad#220. What is the effect of this change? Not much, because before we ever execute the `compare_exchange`, we do a load with `Ordering::Acquire`; the `compare_exchange` is in the `#[cold]` path already. Thus, this just mostly clarifies our expectations. See the non-doc comment added under the module's doc comment for the reasoning. How does this change the code gen? Consider this analogous example: ```diff #[no_mangle] fn foo1(y: &mut i32) -> bool { - let r = X.compare_exchange(0, 1, Ordering::AcqRel, Ordering::Acquire).is_ok(); + let r = X.compare_exchange(0, 1, Ordering::Release, Ordering::Acquire).is_ok(); r } ``` On x86_64, there is no change. Here is the generated code before and after: ``` foo1: mov rcx, qword ptr [rip + example::X::h9e1b81da80078af7@GOTPCREL] mov edx, 1 xor eax, eax lock cmpxchg dword ptr [rcx], edx sete al ret example::X::h9e1b81da80078af7: .zero 4 ``` On AArch64, regardless of whether atomics are outlined or not, there is no change. Here is the generated code with inlined atomics: ``` foo1: adrp x8, :got:example::X::h40b04fb69d714de3 ldr x8, [x8, :got_lo12:example::X::h40b04fb69d714de3] .LBB0_1: ldaxr w9, [x8] cbnz w9, .LBB0_4 mov w0, matklad#1 stlxr w9, w0, [x8] cbnz w9, .LBB0_1 ret .LBB0_4: mov w0, wzr clrex ret example::X::h40b04fb69d714de3: .zero 4 ``` For 32-bit ARMv7, with inlined atomics, the resulting diff in the object code is: ```diff @@ -10,14 +10,13 @@ mov r0, matklad#1 strex r2, r0, [r1] cmp r2, #0 - beq .LBB0_5 + bxeq lr ldrex r0, [r1] cmp r0, #0 beq .LBB0_2 .LBB0_4: - mov r0, #0 clrex -.LBB0_5: + mov r0, #0 dmb ish bx lr .LCPI0_0: @@ -54,4 +53,3 @@ example::X::h47e2038445e1c648: .zero 4 ```
1 parent a70d907 commit 2a707ee

File tree

4 files changed

+19
-8
lines changed

4 files changed

+19
-8
lines changed

CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# Changelog
22

3+
## 1.21.2
4+
- Relax success ordering from AcqRel to Release in `race`: [#278](https://github.com/matklad/once_cell/pull/278).
5+
36
## 1.21.1
47
- Reduce MSRV to 1.65: [#277](https://github.com/matklad/once_cell/pull/277).
58

Cargo.lock.msrv

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "once_cell"
3-
version = "1.21.1"
3+
version = "1.21.2"
44
authors = ["Aleksey Kladov <[email protected]>"]
55
license = "MIT OR Apache-2.0"
66
edition = "2021"

src/race.rs

+14-6
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@
1919
//! `Acquire` and `Release` have very little performance overhead on most
2020
//! architectures versus `Relaxed`.
2121
22+
// The "atomic orderings" section of the documentation above promises
23+
// "happens-before" semantics. This drives the choice of orderings in the uses
24+
// of `compare_exchange` below. On success, the value was zero/null, so there
25+
// was nothing to acquire (there is never any `Ordering::Release` store of 0).
26+
// On failure, the value was nonzero, so it was initialized previously (perhaps
27+
// on another thread) using `Ordering::Release`, so we must use
28+
// `Ordering::Acquire` to ensure that store "happens-before" this load.
29+
2230
#[cfg(not(feature = "portable-atomic"))]
2331
use core::sync::atomic;
2432
#[cfg(feature = "portable-atomic")]
@@ -98,7 +106,7 @@ impl OnceNonZeroUsize {
98106
#[inline]
99107
pub fn set(&self, value: NonZeroUsize) -> Result<(), ()> {
100108
let exchange =
101-
self.inner.compare_exchange(0, value.get(), Ordering::AcqRel, Ordering::Acquire);
109+
self.inner.compare_exchange(0, value.get(), Ordering::Release, Ordering::Acquire);
102110
match exchange {
103111
Ok(_) => Ok(()),
104112
Err(_) => Err(()),
@@ -144,7 +152,7 @@ impl OnceNonZeroUsize {
144152
#[inline(never)]
145153
fn init<E>(&self, f: impl FnOnce() -> Result<NonZeroUsize, E>) -> Result<NonZeroUsize, E> {
146154
let mut val = f()?.get();
147-
let exchange = self.inner.compare_exchange(0, val, Ordering::AcqRel, Ordering::Acquire);
155+
let exchange = self.inner.compare_exchange(0, val, Ordering::Release, Ordering::Acquire);
148156
if let Err(old) = exchange {
149157
val = old;
150158
}
@@ -258,7 +266,7 @@ impl<'a, T> OnceRef<'a, T> {
258266
pub fn set(&self, value: &'a T) -> Result<(), ()> {
259267
let ptr = value as *const T as *mut T;
260268
let exchange =
261-
self.inner.compare_exchange(ptr::null_mut(), ptr, Ordering::AcqRel, Ordering::Acquire);
269+
self.inner.compare_exchange(ptr::null_mut(), ptr, Ordering::Release, Ordering::Acquire);
262270
match exchange {
263271
Ok(_) => Ok(()),
264272
Err(_) => Err(()),
@@ -301,7 +309,7 @@ impl<'a, T> OnceRef<'a, T> {
301309
let exchange = self.inner.compare_exchange(
302310
ptr::null_mut(),
303311
ptr,
304-
Ordering::AcqRel,
312+
Ordering::Release,
305313
Ordering::Acquire,
306314
);
307315
if let Err(old) = exchange {
@@ -396,7 +404,7 @@ mod once_box {
396404
let exchange = self.inner.compare_exchange(
397405
ptr::null_mut(),
398406
ptr,
399-
Ordering::AcqRel,
407+
Ordering::Release,
400408
Ordering::Acquire,
401409
);
402410
if exchange.is_err() {
@@ -442,7 +450,7 @@ mod once_box {
442450
let exchange = self.inner.compare_exchange(
443451
ptr::null_mut(),
444452
ptr,
445-
Ordering::AcqRel,
453+
Ordering::Release,
446454
Ordering::Acquire,
447455
);
448456
if let Err(old) = exchange {

0 commit comments

Comments
 (0)