[libcxx][rfc] Externalized threading support

[rmaprath]

Summary

This patch builds on D19412.

The motivation here is to allow toolchain vendors to build a version of libcxx with all the underlying os threading mechanics differed to runtime - sacrificing some performance for flexibility.

This API (_LIBCPP_THREAD_API_EXTERNAL) currently only works when libcxx is build as a static library. This is because the shared library builds pass -z defs linker flag, which disallows undefined symbols in the library. I'm not familiar with shared library limitations on specific platforms, but in theory, this could work on shared library builds as well.

Currently there are quite a lot of test failures because the testing infrastructure need to be updated to provide an implementation of this threading API (pthreads will do). Our plan is to hide this whole extension under some cmake variable at some point.

Before putting more effort into this, I thought of checking if the community if open to this kind of an API. Of course, we (ARM) will be committed to maintaining this API in the long run.

[rmaprath]

I forgot to mention that we will need a similar (although much smaller) porting layer for libcxxabi and libunwind, as those too currently have a pthread dependency.

[bcraig]

Note: My opinion doesn't count for much as far as community acceptance goes.

I'm fine with the idea. I think there is at least one significant conformance hurdle that you will need to address though. I also have some commentary on the implementation details.

include/__mutex_base

43 ↗

(On Diff #54659)

This is not C++14 conformant. The default constructor for mutex() needs to be constexpr.

I'm not sure if there is a good way around this issue given the problem you are trying to solve. There have been similar constexpr issues with this constructor with regards to musl libc.

include/__os_support

23

I would prefer that the internal types use an under-under-libcpp __libcpp prefix. I've had issues in the past with my under-under symbol names colliding with those from the underlying C library, especially for obvious names like under-under-mutex_t.

src/algorithm.cpp

65 ↗

(On Diff #54659)

I would rather the new branch be used across the board. I don't like littering the generic implementation files with preprocessor blocks everywhere.

Stated differently, get rid of

__libcpp_os_support::__os_mutex __rs_mut

and just use

mutex __rs_mut

everywhere instead.

src/mutex.cpp

31 ↗

(On Diff #54659)

My preference is for all of this to hide behind the __os_mutex_destroy call, instead of having the preprocessor block here. It looks like that has already been discussed in a prior review though ( http://reviews.llvm.org/D11781 ).

[rmaprath]

Addressing review comments from @bcraig:

Adjusted according to the changes of D19412. This patch is now mostly trivial.

There is a small catch in that, this version of the API takes pointer-to-pointer type arguments for most __os_xxx functions. This is partly because we need to use opaque pointer types to hide the underlying implementation details from libcxx internals. This can be worked-around by using some template hackery (along the lines of is_pointer), but I thought to leave it as it is to avoid cluttering up the sources for the normal use case.

[bcraig]

LGTM. You will still need to get approval from one of your original reviewers though.

[rmaprath]

Updating to syc-up with D19412. Obviously, this patch needs more work in terms of setting up the testing story (using a pthreads based implementation of the threading API).

[rmaprath]

Re-sync with D19412.

[rmaprath]

Minor re-spin on D19412.

[rmaprath]

Re-spun on top of D19412.

[rmaprath]

Superseded by D20328.

include/__os_support

     pthread_setspecific(__key, __p);
 }
 
-#else // !_LIBCPP_THREAD_API_PTHREAD
+#elif defined(_LIBCPP_THREAD_API_EXTERNAL)
+
+// Mutex
+#define __OS_MUTEX_INITIALIZER 0
+struct __libcpp_mutex_external;
+typedef __libcpp_mutex_external* __libcpp_mutex;
+
+int __os_mutex_init(__libcpp_mutex* __m, bool is_recursive);
+
+int __os_mutex_lock(__libcpp_mutex* __m);
+
+int __os_mutex_trylock(__libcpp_mutex* __m);
+
+int __os_mutex_unlock(__libcpp_mutex* __m);
+
+int __os_mutex_destroy(__libcpp_mutex* __m);
+
+// Condition variable
+#define __OS_COND_INITIALIZER 0
+struct __libcpp_condvar_external;
+typedef __libcpp_condvar_external* __libcpp_condvar;
+
+int __os_condvar_signal(__libcpp_condvar* __cv);
+
+int __os_condvar_broadcast(__libcpp_condvar* __cv);
+
+int __os_condvar_wait(__libcpp_condvar* __cv, __libcpp_mutex* __m);
+
+int __os_condvar_timedwait(__libcpp_condvar* __cv, __libcpp_mutex* __m, timespec* __ts);
+
+int __os_condvar_destroy(__libcpp_condvar* __cv);
+
+// Thread id
+typedef unsigned long __libcpp_thread_id;
+
+int __os_thread_id_compare(__libcpp_thread_id t1, __libcpp_thread_id t2);
+
+// Thread
+struct __libcpp_thread_external;
+typedef __libcpp_thread_external* __libcpp_thread;
+
+template<class _Func, class _Arg>
+int __os_thread_create(__libcpp_thread* __t, _Func&& __f, _Arg&& __arg);
+
+__libcpp_thread_id __os_thread_get_current_id();
+
+__libcpp_thread_id __os_thread_get_id(const __libcpp_thread* __t);
+
+int __os_thread_join(__libcpp_thread* __t);
+
+int __os_thread_detach(__libcpp_thread* __t);
+
+void __os_thread_yield();
+
+// Thread local storage
+typedef unsigned long __libcpp_tl_key;
+
+template<class _Func>
+int __os_tl_create(__libcpp_tl_key* __key, _Func&& __at_exit);
+
+void* __os_tl_get(__libcpp_tl_key __key);
+
+void __os_tl_set(__libcpp_tl_key __key, void* __p);
+
+#else
   #error "No thread API selected."
 #endif