[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.

Thu Jul 14 13:56:34 PDT 2022

jrtc27 added a comment.

I can't obviously see a description of what the additional barriers implied by the sync variants is (which should be in an update to LangRef at the very least, if not also in the summary itself). Inferring it from the AArch64 assembly is also difficult, and the RISC-V lowering being identical to the non-sync forms confuses me further. I don't particularly want to trawl through the web of mailing list posts to try and find out which emails have the right information and which aren't relevant, either.

================
Comment at: llvm/include/llvm-c/Core.h:360
+                                                  operations. */
+  LLVMAtomicOrderingSyncAcquire = 8, /**< Acquire with additional barrier. */
+  LLVMAtomicOrderingSyncRelease = 9, /**< Release with additional barrier. */
----------------
These comments are not particularly insightful

================
Comment at: llvm/include/llvm/IR/Instructions.h:660
+  /// Release requests from the AtomicOrdering. A SequentiallyConsistent and
+  /// StrongSequetiallyConsistent operations would remain
+  /// SequentiallyConsistent.
----------------
You mean SyncSequentiallyConsistent?

================
Comment at: llvm/include/llvm/IR/Instructions.h:670
     case AtomicOrdering::Monotonic:
       return AtomicOrdering::Monotonic;
     case AtomicOrdering::AcquireRelease:
----------------
Are these correct? Without a clear description of what SyncFoo add over and above Foo it's hard to know.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129802