rmaprath abandoned this revision.
rmaprath added a comment.
Reason for abandoning: This patch uses opaque pointers to delegate `std::mutex`
and `std::condition_variable` types to the underlying platform implementation.
This creates problems given that these two types have `constexpr`
rmaprath added a comment.
In http://reviews.llvm.org/D20328#456241, @mclow.lists wrote:
> > Apologies, not at program launch time but link time.
>
>
> I'm OK with that; I think that's unnecessary complication, but not a
> deal-breaker.
> The choosing at program launch seems unworkable to me.
>
rmaprath added a comment.
In http://reviews.llvm.org/D20328#456271, @mclow.lists wrote:
> > I've adopted a initialize-on-first-use policy to workaround this
> > particular problem.
>
>
> That's not constexpr.
>
> You say it in your comment "This prohibits any prospects of calling a runtime
>
mclow.lists added a comment.
> I've adopted a initialize-on-first-use policy to workaround this particular
> problem.
That's not constexpr.
You say it in your comment "This prohibits any prospects of calling a runtime
initialization routine", but then you have a runtime initialization
mclow.lists added a comment.
> Apologies, not at program launch time but link time.
I'm OK with that; I think that's unnecessary complication, but not a
deal-breaker.
The choosing at program launch seems unworkable to me.
[ I think that the threading implementation should be chosen by the
rmaprath added a comment.
@mclow.lists, @EricWF: Ping.
http://reviews.llvm.org/D20328
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rmaprath added a comment.
@mclow.lists, @EricWF: Gentle (and shameless) ping!
http://reviews.llvm.org/D20328
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rmaprath marked 2 inline comments as done.
rmaprath added a comment.
http://reviews.llvm.org/D20328
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rmaprath updated this revision to Diff 60017.
rmaprath added a comment.
Addressed comments from @mclow.lists.
http://reviews.llvm.org/D20328
Files:
CMakeLists.txt
include/__config
include/__config_site.in
include/__dynamic_threading
include/__threading_support
include/thread
rmaprath added a comment.
In http://reviews.llvm.org/D20328#451210, @STL_MSFT wrote:
> [Asiri Rathnayake]
>
> > The `constexpr` mutex constructor is apparently also a
>
> > pain point on windows [1], I'm sure @STL_MSFT knows more about it.
>
>
> That's because of MSVC-specific issues, where we
[Asiri Rathnayake]
> The `constexpr` mutex constructor is apparently also a
> pain point on windows [1], I'm sure @STL_MSFT knows more about it.
That's because of MSVC-specific issues, where we need to dynamically switch
between WinAPI and ConcRT implementations depending on whether the end
rmaprath added a comment.
In http://reviews.llvm.org/D20328#451197, @rmaprath wrote:
> In http://reviews.llvm.org/D20328#451178, @mclow.lists wrote:
>
> > Also, I don't see how this can be retargeted "at runtime"; are you implying
> > that someone can choose at program launch time what
rmaprath added a subscriber: STL_MSFT.
rmaprath added a comment.
In http://reviews.llvm.org/D20328#451178, @mclow.lists wrote:
> Also, I don't see how this can be retargeted "at runtime"; are you implying
> that someone can choose at program launch time what threading system to use?
Yup,
mclow.lists added a comment.
Some nits while reading this. More to come.
Also, I don't see how this can be retargeted "at runtime"; are you implying
that someone can choose at program launch time what threading system to use?
How could that work given (say) a constexpr constructor for a mutex
rmaprath added a comment.
@mclow.lists, @ericwf: Ping.
@bcraig: I guess you wouldn't be able to let this through on your own? (in case
if there isn't much interest in this functionality from others). Eric might be
away though (didn't see many emails from him), so I'll hold off for now.
/
rmaprath updated this revision to Diff 59519.
rmaprath added a comment.
Added a comment about `__static_threading` and `__dynamic_threading` header
includes.
@EricWF: Ping (sorry).
http://reviews.llvm.org/D20328
Files:
CMakeLists.txt
include/__config
include/__config_site.in
bcraig added inline comments.
Comment at: include/__threading_support:201
@@ +200,3 @@
+# if !defined(__has_include) || __has_include(<__static_threading>)
+#include<__static_threading>
+# else
This #include deserves a comment. Some perplexed developer is
rmaprath updated this revision to Diff 59363.
rmaprath added a comment.
Addressed review comments from @bcraig.
@EricWF: Gentle ping.
http://reviews.llvm.org/D20328
Files:
CMakeLists.txt
include/__config
include/__config_site.in
include/__dynamic_threading
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:
> rmaprath wrote:
> > bcraig wrote:
> > > rmaprath wrote:
> > > > bcraig
rmaprath 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:
> > > >
rmaprath added inline comments.
Comment at: include/__threading_support:201
@@ +200,3 @@
+// Mutex
+#define _LIBCPP_MUTEX_INITIALIZER nullptr
+struct __libcpp_platform_mutex_t;
bcraig wrote:
> rmaprath wrote:
> > bcraig wrote:
> > > rmaprath wrote:
> > > > bcraig
rmaprath added inline comments.
Comment at: include/__threading_support:201
@@ +200,3 @@
+// Mutex
+#define _LIBCPP_MUTEX_INITIALIZER nullptr
+struct __libcpp_platform_mutex_t;
bcraig wrote:
> rmaprath wrote:
> > bcraig wrote:
> > > I'm not sure I like taking the
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:
> > I'm not sure I like taking the freedom to define
rmaprath added inline comments.
Comment at: include/__threading_support:201
@@ +200,3 @@
+// Mutex
+#define _LIBCPP_MUTEX_INITIALIZER nullptr
+struct __libcpp_platform_mutex_t;
bcraig wrote:
> I'm not sure I like taking the freedom to define
bcraig added a comment.
Note: You'll want to look at http://reviews.llvm.org/D20573, as there will be
confilicts for whoever submits second.
Comment at: include/__threading_support:201
@@ +200,3 @@
+// Mutex
+#define _LIBCPP_MUTEX_INITIALIZER nullptr
+struct
rmaprath added a comment.
@mclow.lists, @EricWF: Any comments on this? If it helps, I could split off the
library source changes and test setup into two separate patches. I thought to
keep the two together as the test setup gives an idea of the overall objective.
Thanks.
/ Asiri
26 matches
Mail list logo
