[PATCH] [libcxx] Add __atomic header that handles needed atomic operations.

Eric Fiselier eric at efcs.ca
Thu Jun 11 21:11:13 PDT 2015


Respond to @jroelofs comments.


================
Comment at: include/__atomic:85
@@ +84,3 @@
+{
+    *__dest = __val;
+}
----------------
jroelofs wrote:
> Should these fall back on the __sync_* ones instead of just dropping the atomicness? It seems a little precarious to silently default to non-atomic implementations for these (unless this is for a single-threaded build of the library).
Yes and no. The only atomic operations needed in the headers are relaxed atomic loads. AFAIK all loads are relaxed atomic loads on x86 so not using atomic builtins isn't a big deal (its what we already do).

The rest of the atomic operations happen within the dylib. I agree we don't want to drop the atomicness on these unless it is a single threaded build.

What we really should do is limit the libc++ dylib build to compilers that provide the required atomic intrinsics (single-threaded builds not withstanding). If the compiler is not supported then the build should fail. For GCC and Clang the required versions would be 4.7 and 3.3 (possibly lower) respectively.

Since most of the stuff in `__atomic` is only used in the dylib build perhaps it should be moved out of the shipped headers. I'll figure out how to handle that tomorrow.

http://reviews.llvm.org/D10406

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list