diff mbox

revised fix for nanosleep check in GLIBCXX_ENABLE_LIBSTDCXX_TIME for darwin

Message ID 20121011160515.GA23726@bromo.med.uc.edu
State New
Headers show

Commit Message

Jack Howarth Oct. 11, 2012, 4:05 p.m. UTC
On Thu, Oct 11, 2012 at 12:18:59AM +0100, Jonathan Wakely wrote:
> On 11 October 2012 00:09, Jonathan Wakely wrote:
> > On 9 October 2012 18:49, Benjamin De Kosnik wrote:
> >>
> >>> I don't like the sched_yield macro being set there because it's
> >>> detected correctly by configure anyway, but I'm not going to labour
> >>> that point any more.
> >>
> >> Indeed. Then somebody will waste hours in the future wondering why
> >> configure says no but their TU says yes.
> >>
> >> At least a comment in the configure bits admitting defeat, people.
> >
> > Committed to trunk like so.
> 
> Oops, that changelog entry missed out the PR number, so it hasn't
> updated Bugzilla, sorry.
> 
> Jack, please test and check and unpatched GCC trunk now supports
> this_thread::yield() and this_thread::sleep_for().

Jonathan,
    The committed patch results in the expected this_thread::yield() and this_thread::sleep_for()
support. I did run into a regression at -m32/-m64 on darwin10 (but not darwin11/12).

WARNING: program timed out.
FAIL: 30_threads/condition_variable/54185.cc execution test

This actually isn't that surprising because the pthread support is buggy on darwin10. We always randomly
fail the execution test for 20_util/shared_ptr/thread/default_weaktoshared.cc on darwin10. I filed that
as radr://7905357, "testcase 20_util/shared_ptr/thread/default_weaktoshared.cc thread handling bug" back
in 2010 and it was fixed in darwin11. Either we tolerate this regression on darwin10 or we can
add something like...


Hopefully Mike can chime in with an opinion here. I'll test this workaround later today on darwin10.
               Jack

> 
> Thanks for persevering to get it working.

Comments

Jonathan Wakely Oct. 11, 2012, 5:46 p.m. UTC | #1
On 11 October 2012 17:05, Jack Howarth wrote:
>
> Jonathan,
>     The committed patch results in the expected this_thread::yield() and this_thread::sleep_for()
> support. I did run into a regression at -m32/-m64 on darwin10 (but not darwin11/12).
>
> WARNING: program timed out.
> FAIL: 30_threads/condition_variable/54185.cc execution test

That's PR 54407, so not a regression.

> This actually isn't that surprising because the pthread support is buggy on darwin10. We always randomly
> fail the execution test for 20_util/shared_ptr/thread/default_weaktoshared.cc on darwin10. I filed that
> as radr://7905357, "testcase 20_util/shared_ptr/thread/default_weaktoshared.cc thread handling bug" back
> in 2010 and it was fixed in darwin11. Either we tolerate this regression on darwin10 or we can
> add something like...
>
> Index: libstdc++-v3/config/os/bsd/darwin/os_defines.h
> ===================================================================
> --- libstdc++-v3/config/os/bsd/darwin/os_defines.h      (revision 192370)
> +++ libstdc++-v3/config/os/bsd/darwin/os_defines.h      (working copy)
> @@ -43,8 +43,10 @@
>  #define _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC
>
>  // Configure checks for nanosleep fail on Darwin, but nanosleep and
> -// sched_yield are always available, so use them.
> +// sched_yield are always available, so use them 10.6 or later.
> +#if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1060
>  #define _GLIBCXX_USE_NANOSLEEP 1
>  #define _GLIBCXX_USE_SCHED_YIELD 1
> +#endif
>
>  #endif
>
> Hopefully Mike can chime in with an opinion here. I'll test this workaround later today on darwin10.

I don't think that will make any difference, because that test was
already failing anyway.
Jack Howarth Oct. 11, 2012, 6:01 p.m. UTC | #2
On Thu, Oct 11, 2012 at 06:46:47PM +0100, Jonathan Wakely wrote:
> On 11 October 2012 17:05, Jack Howarth wrote:
> >
> > Jonathan,
> >     The committed patch results in the expected this_thread::yield() and this_thread::sleep_for()
> > support. I did run into a regression at -m32/-m64 on darwin10 (but not darwin11/12).
> >
> > WARNING: program timed out.
> > FAIL: 30_threads/condition_variable/54185.cc execution test
> 
> That's PR 54407, so not a regression.

Jonathan,
   Thanks, I missed that. The pthreads support is definitely improved in darwin11 and later.
           Jack

> 
> > This actually isn't that surprising because the pthread support is buggy on darwin10. We always randomly
> > fail the execution test for 20_util/shared_ptr/thread/default_weaktoshared.cc on darwin10. I filed that
> > as radr://7905357, "testcase 20_util/shared_ptr/thread/default_weaktoshared.cc thread handling bug" back
> > in 2010 and it was fixed in darwin11. Either we tolerate this regression on darwin10 or we can
> > add something like...
> >
> > Index: libstdc++-v3/config/os/bsd/darwin/os_defines.h
> > ===================================================================
> > --- libstdc++-v3/config/os/bsd/darwin/os_defines.h      (revision 192370)
> > +++ libstdc++-v3/config/os/bsd/darwin/os_defines.h      (working copy)
> > @@ -43,8 +43,10 @@
> >  #define _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC
> >
> >  // Configure checks for nanosleep fail on Darwin, but nanosleep and
> > -// sched_yield are always available, so use them.
> > +// sched_yield are always available, so use them 10.6 or later.
> > +#if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1060
> >  #define _GLIBCXX_USE_NANOSLEEP 1
> >  #define _GLIBCXX_USE_SCHED_YIELD 1
> > +#endif
> >
> >  #endif
> >
> > Hopefully Mike can chime in with an opinion here. I'll test this workaround later today on darwin10.
> 
> I don't think that will make any difference, because that test was
> already failing anyway.
diff mbox

Patch

Index: libstdc++-v3/config/os/bsd/darwin/os_defines.h
===================================================================
--- libstdc++-v3/config/os/bsd/darwin/os_defines.h	(revision 192370)
+++ libstdc++-v3/config/os/bsd/darwin/os_defines.h	(working copy)
@@ -43,8 +43,10 @@ 
 #define _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC
 
 // Configure checks for nanosleep fail on Darwin, but nanosleep and
-// sched_yield are always available, so use them.
+// sched_yield are always available, so use them 10.6 or later.
+#if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1060
 #define _GLIBCXX_USE_NANOSLEEP 1
 #define _GLIBCXX_USE_SCHED_YIELD 1
+#endif
 
 #endif