[PATCH] D24864: [libcxxabi] Refactor pthread usage into a separate API

Saleem Abdulrasool via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 26 11:20:25 PDT 2016


compnerd added inline comments.

================
Comment at: src/config.h:41
@@ +40,3 @@
+    defined(__CloudABI__) || \
+    defined(__sun__)
+#  define _LIBCXXABI_HAS_THREAD_API_PTHREAD
----------------
Can you change this to `defined(__sun__) && defined(_POSIX_THREADS)` at the very least?  There is no reason to assume pthreads on Solaris.  Solaris Threads are a valid threading model.

================
Comment at: src/config.h:42
@@ +41,3 @@
+    defined(__sun__)
+#  define _LIBCXXABI_HAS_THREAD_API_PTHREAD
+# else
----------------
Can we use `_LIBCXXABI_USE_THREAD_API_PTHREAD` instead?  You can have more than one threading API on a platform, and wish to use a specific one.

================
Comment at: src/config.h:46
@@ +45,3 @@
+# endif
+#endif
+
----------------
I really think that we should use `_POSIX_THREADS` macro rather than this enumeration approach.

================
Comment at: test/test_fallback_malloc.pass.cpp:13
@@ -12,3 +12,3 @@
 
-#include <pthread.h>
+#include "../src/threading_support.h"
 
----------------
Can we add the include directory as a header search path for the tests?  Seems it may be nicer, but, obviously, we can do that later.


https://reviews.llvm.org/D24864





More information about the cfe-commits mailing list