Skip to content

Commit 32c2734

Browse files
committed
race: Reduce 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? Consider this 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 0d6bc31 commit 32c2734

File tree

1 file changed

+42
-6
lines changed

1 file changed

+42
-6
lines changed

src/race.rs

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,14 @@ impl OnceNonZeroUsize {
8181
/// full.
8282
#[inline]
8383
pub fn set(&self, value: NonZeroUsize) -> Result<(), ()> {
84+
// The module documentation promises "happens-before" semantics. On
85+
// success, the value was zero, so there is nothing to acquire (there
86+
// is never any `Ordering::Release` store of 0). On failure, the value
87+
// was nonzero, so it was initialized previously (perhaps on another
88+
// thread) using `Ordering::Release`; use `Ordering::Acquire` to
89+
// ensure that store "happens-before" this load.
8490
let exchange =
85-
self.inner.compare_exchange(0, value.get(), Ordering::AcqRel, Ordering::Acquire);
91+
self.inner.compare_exchange(0, value.get(), Ordering::Release, Ordering::Acquire);
8692
match exchange {
8793
Ok(_) => Ok(()),
8894
Err(_) => Err(()),
@@ -128,7 +134,13 @@ impl OnceNonZeroUsize {
128134
#[inline(never)]
129135
fn init<E>(&self, f: impl FnOnce() -> Result<NonZeroUsize, E>) -> Result<NonZeroUsize, E> {
130136
let mut val = f()?.get();
131-
let exchange = self.inner.compare_exchange(0, val, Ordering::AcqRel, Ordering::Acquire);
137+
// The module documentation promises "happens-before" semantics. On
138+
// success, the value was zero, so there is nothing to acquire (there
139+
// is never any `Ordering::Release` store of 0). On failure, the value
140+
// was nonzero, so it was initialized previously (perhaps on another
141+
// thread) using `Ordering::Release`; use `Ordering::Acquire` to
142+
// ensure that store "happens-before" this load.
143+
let exchange = self.inner.compare_exchange(0, val, Ordering::Release, Ordering::Acquire);
132144
if let Err(old) = exchange {
133145
val = old;
134146
}
@@ -241,8 +253,14 @@ impl<'a, T> OnceRef<'a, T> {
241253
/// full.
242254
pub fn set(&self, value: &'a T) -> Result<(), ()> {
243255
let ptr = value as *const T as *mut T;
256+
// The module documentation promises "happens-before" semantics. On
257+
// success, the value was null, so there is nothing to acquire (there
258+
// is never any `Ordering::Release` store of null). On failure, the
259+
// value was non-null, so it was initialized previously (perhaps on
260+
// another thread) using `Ordering::Release`; use `Ordering::Acquire`
261+
// to ensure that store "happens-before" this load.
244262
let exchange =
245-
self.inner.compare_exchange(ptr::null_mut(), ptr, Ordering::AcqRel, Ordering::Acquire);
263+
self.inner.compare_exchange(ptr::null_mut(), ptr, Ordering::Release, Ordering::Acquire);
246264
match exchange {
247265
Ok(_) => Ok(()),
248266
Err(_) => Err(()),
@@ -282,10 +300,16 @@ impl<'a, T> OnceRef<'a, T> {
282300
if ptr.is_null() {
283301
// TODO replace with `cast_mut` when MSRV reaches 1.65.0 (also in `set`)
284302
ptr = f()? as *const T as *mut T;
303+
// The module documentation promises "happens-before" semantics. On
304+
// success, the value was null, so there is nothing to acquire (there
305+
// is never any `Ordering::Release` store of null). On failure, the
306+
// value was non-null, so it was initialized previously (perhaps on
307+
// another thread) using `Ordering::Release`; use `Ordering::Acquire`
308+
// to ensure that store "happens-before" this load.
285309
let exchange = self.inner.compare_exchange(
286310
ptr::null_mut(),
287311
ptr,
288-
Ordering::AcqRel,
312+
Ordering::Release,
289313
Ordering::Acquire,
290314
);
291315
if let Err(old) = exchange {
@@ -377,10 +401,16 @@ mod once_box {
377401
/// full.
378402
pub fn set(&self, value: Box<T>) -> Result<(), Box<T>> {
379403
let ptr = Box::into_raw(value);
404+
// The module documentation promises "happens-before" semantics. On
405+
// success, the value was null, so there is nothing to acquire (there
406+
// is never any `Ordering::Release` store of null). On failure, the
407+
// value was non-null, so it was initialized previously (perhaps on
408+
// another thread) using `Ordering::Release` so use `Ordering::Acquire`
409+
// to ensure that store "happens-before" this load.
380410
let exchange = self.inner.compare_exchange(
381411
ptr::null_mut(),
382412
ptr,
383-
Ordering::AcqRel,
413+
Ordering::Release,
384414
Ordering::Acquire,
385415
);
386416
if exchange.is_err() {
@@ -423,10 +453,16 @@ mod once_box {
423453
if ptr.is_null() {
424454
let val = f()?;
425455
ptr = Box::into_raw(val);
456+
// The module documentation promises "happens-before" semantics. On
457+
// success, the value was null, so there is nothing to acquire (there
458+
// is never any `Ordering::Release` store of null). On failure, the
459+
// value was non-null, so it was initialized previously (perhaps on
460+
// another thread) using `Ordering::Release` so use `Ordering::Acquire`
461+
// to ensure that store "happens-before" this load.
426462
let exchange = self.inner.compare_exchange(
427463
ptr::null_mut(),
428464
ptr,
429-
Ordering::AcqRel,
465+
Ordering::Release,
430466
Ordering::Acquire,
431467
);
432468
if let Err(old) = exchange {

0 commit comments

Comments
 (0)