espositofulvio added a comment.
In http://reviews.llvm.org/D11781#423520, @rmaprath wrote:
> @espositofulvio: Thanks for the patch! :)
>
> Committed as r268734.
Glad to see you land the patch! Great work :)
Repository:
rL LLVM
http://reviews.llvm.org/D11781
_
rmaprath added a comment.
@espositofulvio: Thanks for the patch! :)
Committed as r268734.
Repository:
rL LLVM
http://reviews.llvm.org/D11781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
rmaprath added a comment.
In http://reviews.llvm.org/D11781#403349, @rmaprath wrote:
> In http://reviews.llvm.org/D11781#403343, @theraven wrote:
>
> > In http://reviews.llvm.org/D11781#403335, @rmaprath wrote:
> >
> > > For us (ARM), a threads porting layer is important on several RTOSes
> > >
rmaprath added a comment.
In http://reviews.llvm.org/D11781#403378, @espositofulvio wrote:
> In http://reviews.llvm.org/D11781#400968, @rmaprath wrote:
>
> > Hi, could I know the status of this? I'd like to push this forward.
> >
> > @espositofulvio: Are you working on this? (just checking since
espositofulvio added a comment.
In http://reviews.llvm.org/D11781#400968, @rmaprath wrote:
> Hi, could I know the status of this? I'd like to push this forward.
>
> @espositofulvio: Are you working on this? (just checking since this has gone
> stale for a while). @EricWF: I can create a separate
rmaprath added a comment.
In http://reviews.llvm.org/D11781#403343, @theraven wrote:
> In http://reviews.llvm.org/D11781#403335, @rmaprath wrote:
>
> > For us (ARM), a threads porting layer is important on several RTOSes (where
> > a full-blown pthreads implementations is not available). I will
theraven added a comment.
In http://reviews.llvm.org/D11781#403335, @rmaprath wrote:
> For us (ARM), a threads porting layer is important on several RTOSes (where a
> full-blown pthreads implementations is not available). I will see if I can
> publish one of those porting layer implementations,
rmaprath added a comment.
Hi Eric, Thanks for the comments!
I'll wait a bit for @espositofulvio in case if he wants to push this, otherwise
I'll create a new diff with the suggested changes.
For us (ARM), a threads porting layer is important on several RTOSes (where a
full-blown pthreads imple
EricWF added a comment.
I would like to see this patch without the `support/pthread/*.cpp` files. There
are a couple of reasons for this.
1. The symbols in those files are private to the dylib, but they are declared
in the headers and given external visibility.
2. Those symbols frequently use s
rmaprath added a subscriber: rmaprath.
rmaprath added a comment.
Hi, could I know the status of this? I'd like to push this forward.
@espositofulvio: Are you working on this? (just checking since this has gone
stale for a while). @EricWF: I can create a separate diff for further review if
this
espositofulvio updated the summary for this revision.
espositofulvio updated this revision to Diff 34104.
espositofulvio added a comment.
- Addressed possible ABI breaks
- Reverted to not using a __config_file as @jroelofs has two separate patch for
that
Repository:
rL LLVM
http://reviews.ll
EricWF added a comment.
In http://reviews.llvm.org/D11781#227606, @espositofulvio wrote:
> In http://reviews.llvm.org/D11781#227446, @EricWF wrote:
>
> > This patch has a long way to go but it has also come a long way. Here are a
> > couple of problems I see with it.
> >
> > 2. This patch adds a
espositofulvio added inline comments.
Comment at: include/__mutex_base:36
@@ -35,3 +37,3 @@
#else
- mutex() _NOEXCEPT {__m_ = (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER;}
#endif
EricWF wrote:
> Why was the cast insignificant?
The cast was significant, but it
espositofulvio added a comment.
In http://reviews.llvm.org/D11781#227446, @EricWF wrote:
> This patch has a long way to go but it has also come a long way. Here are a
> couple of problems I see with it.
>
> 2. This patch adds a lot of headers. libc++ has historically tried to keep
> the number
EricWF added a comment.
Added more inline comments.
Comment at: include/__mutex_base:36
@@ -35,3 +37,3 @@
#else
- mutex() _NOEXCEPT {__m_ = (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER;}
#endif
Why was the cast insignificant?
Comment at: in
EricWF added a reviewer: EricWF.
EricWF added a comment.
This patch has a long way to go but it has also come a long way. Here are a
couple of problems I see with it.
1. There are still plenty of ABI breaks. I'll try and point them all out.
2. This patch adds a lot of headers. libc++ has histori
espositofulvio added a comment.
In http://reviews.llvm.org/D11781#222572, @joerg wrote:
> This feels a bit like a regression. Before, libc++ works fine by just
> pointing into the include directory.
With my change it still is fine pointing at the include directory. But as I
said, Jonathan fee
joerg added a subscriber: joerg.
joerg added a comment.
This feels a bit like a regression. Before, libc++ works fine by just pointing
into the include directory.
Repository:
rL LLVM
http://reviews.llvm.org/D11781
___
cfe-commits mailing list
cf
espositofulvio added a comment.
In http://reviews.llvm.org/D11781#222452, @mclow.lists wrote:
> How does this change interact with http://reviews.llvm.org/D11963 ?
The difference between this and @jroelofs's one is that this copy the
__config_site in the source directory which makes everythin
mclow.lists added a comment.
How does this change interact with http://reviews.llvm.org/D11963 ?
Repository:
rL LLVM
http://reviews.llvm.org/D11781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
jroelofs added inline comments.
Comment at: include/__config:744
@@ +743,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# ifndef _LIBCPP_THREAD_API
+# error "No thread API"
The reason to use `CMAKE_BINARY_DIR` over `CMAKE_CURRENT_SOURCE_DIR` as the
location for this buil
espositofulvio removed rL LLVM as the repository for this revision.
espositofulvio updated this revision to Diff 31874.
espositofulvio added a comment.
Thread library selection is done at configure time by CMake now.
http://reviews.llvm.org/D11781
Files:
.gitignore
CMakeLists.txt
include/
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
> jroelofs wrote:
> > espositofulvio wrote:
> > > theraven wrote:
> >
jroelofs added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
> espositofulvio wrote:
> > theraven wrote:
> > > espositofulvio wrote:
> >
mclow.lists added a subscriber: mclow.lists.
mclow.lists added a reviewer: mclow.lists.
mclow.lists added a comment.
I agree with @jroelofs that we don't want to `#include ` in
`<__config>`
Repository:
rL LLVM
http://reviews.llvm.org/D11781
___
jroelofs added a subscriber: EricWF.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
espositofulvio wrote:
> theraven wrote:
> > espositofulvio wrote:
> > > jroelofs wrote:
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
theraven wrote:
> espositofulvio wrote:
> > jroelofs wrote:
> > > espositofulvio wrote
theraven added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
espositofulvio wrote:
> jroelofs wrote:
> > espositofulvio wrote:
> > > jroelofs wrote:
> >
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
> espositofulvio wrote:
> > jroelofs wrote:
> > > jroelofs wrote:
> >
jroelofs added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
espositofulvio wrote:
> jroelofs wrote:
> > jroelofs wrote:
> > > @espositofulvio: @ed meant
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
> jroelofs wrote:
> > @espositofulvio: @ed meant this:
> >
> > ```
>
jroelofs added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
> @espositofulvio: @ed meant this:
>
> ```
> #ifndef _WIN32
> # include
>
jroelofs added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
@espositofulvio: @ed meant this:
```
#ifndef _WIN32
# include
# if _POSIX_THREADS > 0
..
espositofulvio marked 5 inline comments as done.
espositofulvio added a comment.
Repository:
rL LLVM
http://reviews.llvm.org/D11781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
espositofulvio updated this revision to Diff 31716.
espositofulvio added a comment.
Added __CloudABI__ to the list of platform using pthread
Repository:
rL LLVM
http://reviews.llvm.org/D11781
Files:
include/__config
include/__mutex_base
include/mutex
include/support/condition_variabl
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__linux__) ||
defined(__APPLE__)
+# define _LIBCPP_THREAD_API _LIBCPP_PTHREAD
ed wrot
theraven added a comment.
It would be nice to use rwlocks, but that's an ABI-breaking change. We'd need
to bump the .so version and other annoyances. Definitely something I'd be in
favour of seeing hidden behind some #ifdefs (and enabled for FreeBSD 11 and
probably for the libc++ in packages)
ed added a comment.
A general note I have regarding this change:
Now that we're introducing separate implementations for mutexes and condition
variables, could we also consider letting `shared_mutex` and friends simply use
`pthread_rwlock_*()`? We currently have it implemented as a wrapper on t
ed added a subscriber: ed.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__linux__) ||
defined(__APPLE__)
+# define _LIBCPP_THREAD_API _LIBCPP_PTHREAD
espositofulvio wro
On 8/8/15 2:23 PM, Fulvio Esposito wrote:
espositofulvio added inline comments.
Comment at: include/__config:742 @@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS +# if defined(__FreeBSD__) ||
defined(__NetBSD__) || defined(__linux__) || defined(__APPLE__) +#
define _LIBCPP_THREAD_
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__linux__) ||
defined(__APPLE__)
+# define _LIBCPP_THREAD_API _LIBCPP_PTHREAD
therave
theraven added a comment.
This version looks much better (well, the previous one is cleaner, but this one
looks like it shouldn't break the ABI - did you test this? It looks like it
should work, but I've not actually tried it).
Comment at: include/__config:742
@@ +741,3 @@
+#
espositofulvio added inline comments.
Comment at: include/__mutex_base:246
@@ -266,3 +245,3 @@
-class _LIBCPP_TYPE_VIS condition_variable
+class _LIBCPP_TYPE_VIS condition_variable : private
__libcxx_support::condition_variable
{
theraven wrote:
> espositofulv
theraven added inline comments.
Comment at: include/__mutex_base:246
@@ -266,3 +245,3 @@
-class _LIBCPP_TYPE_VIS condition_variable
+class _LIBCPP_TYPE_VIS condition_variable : private
__libcxx_support::condition_variable
{
espositofulvio wrote:
> theraven wro
espositofulvio added inline comments.
Comment at: include/__mutex_base:246
@@ -266,3 +245,3 @@
-class _LIBCPP_TYPE_VIS condition_variable
+class _LIBCPP_TYPE_VIS condition_variable : private
__libcxx_support::condition_variable
{
theraven wrote:
> espositofulv
espositofulvio added inline comments.
Comment at: include/__mutex_base:19
@@ +18,3 @@
+#ifndef _WIN32
+#include
+#endif
jroelofs wrote:
> I think it might make sense to create a file: `` where the
> contents are just:
>
> ```
> #ifndef _WIN32
> #include
> #end
theraven added inline comments.
Comment at: include/__mutex_base:246
@@ -266,3 +245,3 @@
-class _LIBCPP_TYPE_VIS condition_variable
+class _LIBCPP_TYPE_VIS condition_variable : private
__libcxx_support::condition_variable
{
espositofulvio wrote:
> theraven wro
theraven added inline comments.
Comment at: include/__mutex_base:246
@@ -266,3 +245,3 @@
-class _LIBCPP_TYPE_VIS condition_variable
+class _LIBCPP_TYPE_VIS condition_variable : private
__libcxx_support::condition_variable
{
Does this change the ABI for a mutex
48 matches
Mail list logo
