bcraig added inline comments.
================
Comment at: include/__threading_support:201
@@ +200,3 @@
+// Mutex
+#define _LIBCPP_MUTEX_INITIALIZER nullptr
+struct __libcpp_platform_mutex_t;
----------------
rmaprath wrote:
> bcraig wrote:
> > rmaprath wrote:
> > > bcraig wrote:
> > > > I'm not sure I like taking the freedom to define
> > > > _LIBCPP_MUTEX_INITIALIZER away from implementers.
> > > >
> > > > Would it be too terrible to replace this entire #elif block with
> > > > something like the following?
> > > >
> > > > ```
> > > > #if !defined(__has_include) || __has_include(<os_provided_thread.h>)
> > > > #include <os_provided_thread.h>
> > > > #else
> > > > #error "_LIBCPP_THREAD_API_EXTERNAL requires the implementer to provide
> > > > <os_provided_thread.h> in the include path"
> > > > #endif
> > > > ```
> > > >
> > > >
> > > The problem is that, `std::mutex` constructor needs to be `constexpr` (as
> > > you pointed out earlier). And since `__libcpp_mutex_t` is a pointer type
> > > (for this externally threaded variant), sensible definitions for
> > > `_LIBCPP_MUTEX_INITIALIZER` are limited.
> > >
> > > Other than `nullptr`, one may be able to define
> > > `_LIBCPP_MUTEX_INITIALIZER` to be a pointer to some constant mutex
> > > (presuming that make it `constexpr` OK?) but I'm not convinced if such a
> > > setup would be very useful.
> > >
> > > Hope that sounds sensible?
> > If the implementer gets to provide an entire header, then they also get to
> > choose what libcpp_mutex_t will be. They could make it a struct.
> This externally-threaded library variant needs to be compiled against a set
> API, so we have this header with declarations like `__libcpp_mutex_lock()`
> which are referred from within the library sources (same function signatures
> as those used in `LIBCPP_THREAD_API_PTHREAD` - this allows us to keep the
> library source changes to a minimum).
>
> Now, in this particular library variant, we want to differ these calls to
> runtime rather than libcxx compile-time!
>
> On the other hand, I think you are proposing a compile-time (static) thread
> porting setup where the platform vendors need to supply a header file with
> appropriate definitions for these functions / types, and they would compile
> libcxx themselves? That sounds like a good idea, but the current patch is
> aiming for a different goal: we provide a pre-compiled libcxx library which
> calls out to those `__libcpp_xxx` functions at runtime, and platform vendors
> need to provide those functions, they don't have to compile libcxx
> themselves. This is what forces us to use opaque pointer types to capture
> platform-defined threading primitives.
>
> Perhaps I could improve the naming here, may be
> `LIBCPP_HAS_RUNTIME_THREAD_API`? Or `LIBCPP_HAS_DYNAMIC_THREAD_API`?
That is an excellent summary, and it does a good job of explaining the
disconnect we were having. I'm going to push forward with the disconnect
though :)
I think you can implement the dynamic case in terms of the static case without
loss of generality or performance. You ("Vendor X") could pre-compile your
library with a pointer-sized libcpp_mutex_t defined in <os_provided_thread.h>,
and at runtime call out to a different library that has an implementation of
libcpp_mutex_lock. Someone else ("Vendor Y") could have a larger
libcpp_mutex_t defined, and provide a static inline definition of
libcpp_mutex_lock.
http://reviews.llvm.org/D20328
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
