[PATCH] D129802: [DRAFT] Implementing new atomic orderings in LLVM and generate barriers for legacy __sync builtins. Support corresponding memory model in outline atomics as well.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

`atomicrmw add [...] sync_seq_cst` is supposed to be equivalent to `fence seq_cst; atomicrmw add seq_cst; fence seq_cst`, I think.

The extra fences aren't necessary for most uses of `__sync_*`, but the difference is theoretically visible for exotic synchronization primitives.  At least the gcc developers thought the difference might be visible.  They therefore decided to add the extra barriers to `__sync_*` on AArch64 (and maybe other targets?).

As far as I know, the issue is entirely theoretical; nobody has demonstrated any issue on actual hardware, even with a synthetic testcase.

If we do decide we need to do something here, I'd prefer to model this using explicit fences in the IR, as opposed to a new atomic ordering.  Adding extra atomic orderings just makes everything harder to reason about.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129802/new/

https://reviews.llvm.org/D129802