Description
RFC #888 introduced single-threaded (i.e., compiler-only) memory fence intrinsics, and was implemented in #24833.
The RFC explicitly does not add safe wrappers for these new barriers, and states
The existing fence intrinsics are exported in libstd with safe wrappers, but this design does not export safe wrappers for the new intrinsics. The existing fence functions will still perform correctly if used where a single-threaded fence is called for, but with a slight reduction in efficiency. Not exposing these new intrinsics through a safe wrapper reduces the possibility for confusion on which fences are appropriate in a given situation, while still providing the capability for users to opt in to a single-threaded fence when appropriate.
While the argument is sound, authors of low-level concurrency libraries are often very concerned about the performance overhead of full-blown memory fences when a simple compiler barrier will do. The current design requires authors of such libraries to either require a nightly compiler (to use intrinsics), or to fall back to the old
asm!("" ::: "memory" : "volatile")
workaround (which for the time being also requires nightly due to feature(asm)
).
I propose adding in a safe wrapper around these compiler barrier intrinsics alongside fence
after all, with name and documentation that clearly indicates how that function differs from what fence
provides. PR incoming shortly.
Activity
Auto merge of #41092 - jonhoo:std-fence-intrinsics, r=alexcrichton
Mark-Simulacrum commentedon Jun 20, 2017
This is the tracking issue for https://doc.rust-lang.org/nightly/std/sync/atomic/fn.compiler_fence.html.
alexcrichton commentedon Aug 29, 2017
@rfcbot fcp merge
Hasn't changed in quite awhile and seems straightforward to stabilize!
rfcbot commentedon Aug 29, 2017
Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams:
No concerns currently listed.
Once these reviewers reach consensus, 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.
sfackler commentedon Aug 30, 2017
It seems mildly strange to me that these would live in
sync::atomic
since they don't deal with synchronization or atomicity in the traditional sense. I'm not sure where they should live though.dtolnay commentedon Aug 30, 2017
I am okay with the placement. As a data point, C++ has their equivalent std::atomic_signal_fence in
#include <atomic>
next to their equivalent of everything we have in std::sync::atomic.jonhoo commentedon Aug 30, 2017
@sfackler: I completely agree, and had the same reservation when I first implemented this. But as you say yourself, it's unclear where else this somewhat weird function should go.
sfackler commentedon Aug 30, 2017
Yeah, given that C++ sticks it in
atomic
std::sync::atomic
seems like the least bad place.rfcbot commentedon Sep 10, 2017
🔔 This is now entering its final comment period, as per the review above. 🔔
9 remaining items