Skip to content

Provide safe wrappers for single-threaded memory fence intrinsics #41091

Closed
@jonhoo

Description

@jonhoo
Contributor

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

added a commit that references this issue on Apr 8, 2017

Auto merge of #41092 - jonhoo:std-fence-intrinsics, r=alexcrichton

added
B-unstableBlocker: Implemented in the nightly compiler and unstable.
T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.
on Jun 20, 2017
Mark-Simulacrum

Mark-Simulacrum commented on Jun 20, 2017

@Mark-Simulacrum
Member
added
C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
on Jul 22, 2017
alexcrichton

alexcrichton commented on Aug 29, 2017

@alexcrichton
Member

@rfcbot fcp merge

Hasn't changed in quite awhile and seems straightforward to stabilize!

rfcbot

rfcbot commented on Aug 29, 2017

@rfcbot
Collaborator

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.

added
proposed-final-comment-periodProposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off.
on Aug 29, 2017
sfackler

sfackler commented on Aug 30, 2017

@sfackler
Member

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

dtolnay commented on Aug 30, 2017

@dtolnay
Member

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

jonhoo commented on Aug 30, 2017

@jonhoo
ContributorAuthor

@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

sfackler commented on Aug 30, 2017

@sfackler
Member

Yeah, given that C++ sticks it in atomic std::sync::atomic seems like the least bad place.

rfcbot

rfcbot commented on Sep 10, 2017

@rfcbot
Collaborator

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

9 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    B-unstableBlocker: Implemented in the nightly compiler and unstable.C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCT-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.final-comment-periodIn the final comment period and will be merged soon unless new substantive objections are raised.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @alexcrichton@jonhoo@sfackler@dtolnay@Mark-Simulacrum

        Issue actions

          Provide safe wrappers for single-threaded memory fence intrinsics · Issue #41091 · rust-lang/rust