revised fix for nanosleep check in GLIBCXX_ENABLE_LIBSTDCXX_TIME for darwin

Submitted by Jack Howarth on Oct. 9, 2012, 12:39 a.m.

Details

Message ID 20121009003921.GA25678@bromo.med.uc.edu
State New
Headers show

Commit Message

Jack Howarth Oct. 9, 2012, 12:39 a.m.
The --enable-libstdcxx-time=yes configure option fails to validate the
presence of a usable nanosleep() call on darwin due to its use of pre-2008
POSIX timers. As both nanosleep() and sched_yield() have always been available
on darwin, the attached patch simply defines _GLIBCXX_USE_NANOSLEEP and
_GLIBCXX_USE_SCHED_YIELD in config/os/bsd/darwin/os_defines.h. This also has
the advantage of effectively making --enable-libstdcxx-time=yes the default
on darwin. Regression tested on x86_64-apple-darwin12.
   Okay for gcc trunk as well as gcc-4_7-branch/gcc-4_6-branch/gcc-4_5-branch?
                Jack

libstdc++-v3/

2012-10-08  Jack Howarth  <howarth@bromo.med.uc.edu>

	PR libstdc++/54847
	* config/os/bsd/darwin/os_defines.h (_GLIBCXX_USE_NANOSLEEP): Define.
	(_GLIBCXX_USE_SCHED_YIELD): Likewise.

Comments

Jonathan Wakely Oct. 9, 2012, 8:21 a.m.
On 9 October 2012 01:39, Jack Howarth wrote:
>    The --enable-libstdcxx-time=yes configure option fails to validate the
> presence of a usable nanosleep() call on darwin due to its use of pre-2008
> POSIX timers. As both nanosleep() and sched_yield() have always been available
> on darwin, the attached patch simply defines _GLIBCXX_USE_NANOSLEEP and
> _GLIBCXX_USE_SCHED_YIELD in config/os/bsd/darwin/os_defines.h. This also has
> the advantage of effectively making --enable-libstdcxx-time=yes the default
> on darwin. Regression tested on x86_64-apple-darwin12.
>    Okay for gcc trunk as well as gcc-4_7-branch/gcc-4_6-branch/gcc-4_5-branch?

The 4.5 branch is closed, so definitely not alright there.

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.

OK for trunk, it's not a regression so I'm not sure about the
branches. If it doesn't cause problems on the trunk we can decide
whether to apply it to the 4.7 branch later.
Jack Howarth Oct. 9, 2012, 1:11 p.m.
On Tue, Oct 09, 2012 at 09:21:25AM +0100, Jonathan Wakely wrote:
> On 9 October 2012 01:39, Jack Howarth wrote:
> >    The --enable-libstdcxx-time=yes configure option fails to validate the
> > presence of a usable nanosleep() call on darwin due to its use of pre-2008
> > POSIX timers. As both nanosleep() and sched_yield() have always been available
> > on darwin, the attached patch simply defines _GLIBCXX_USE_NANOSLEEP and
> > _GLIBCXX_USE_SCHED_YIELD in config/os/bsd/darwin/os_defines.h. This also has
> > the advantage of effectively making --enable-libstdcxx-time=yes the default
> > on darwin. Regression tested on x86_64-apple-darwin12.
> >    Okay for gcc trunk as well as gcc-4_7-branch/gcc-4_6-branch/gcc-4_5-branch?
> 
> The 4.5 branch is closed, so definitely not alright there.
> 
> 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.

Since we are defining _GLIBCXX_USE_NANOSLEEP in os_defines.h and effectively
implementing half of the behavior of --enable-libstdcxx-time=yes, it seemed
odd to not complete the process and define _GLIBCXX_USE_SCHED_YIELD as well.
The usage is not as straight-forward as many other configure options 
(especially in light of the absence of rt timer support on darwin).

> 
> OK for trunk, it's not a regression so I'm not sure about the
> branches. If it doesn't cause problems on the trunk we can decide
> whether to apply it to the 4.7 branch later.

I guess the question is which branches have enough C++11 standard support to
make the change meaningful to the end user.
Jonathan Wakely Oct. 9, 2012, 3:33 p.m.
On 9 October 2012 14:11, Jack Howarth wrote:
> On Tue, Oct 09, 2012 at 09:21:25AM +0100, Jonathan Wakely 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.
>
> Since we are defining _GLIBCXX_USE_NANOSLEEP in os_defines.h and effectively
> implementing half of the behavior of --enable-libstdcxx-time=yes, it seemed
> odd to not complete the process and define _GLIBCXX_USE_SCHED_YIELD as well.
> The usage is not as straight-forward as many other configure options
> (especially in light of the absence of rt timer support on darwin).

Why does that absence affect the usage of the option?

For darwin there is no difference between --enable-libstdcxx-time=yes
and --enable-libstdcxx-time=rt, which should make it easier to use,
not harder, because there's no need to choose between the two.

>>
>> OK for trunk, it's not a regression so I'm not sure about the
>> branches. If it doesn't cause problems on the trunk we can decide
>> whether to apply it to the 4.7 branch later.
>
> I guess the question is which branches have enough C++11 standard support to
> make the change meaningful to the end user.

Surely the question is the usual one of whether to make a change to a
release branch if it doesn't fix a regression.
Benjamin Kosnik Oct. 9, 2012, 5:49 p.m.
> 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.

-benjamin
Jack Howarth Oct. 9, 2012, 6:23 p.m.
On Tue, Oct 09, 2012 at 10:49:28AM -0700, 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. 
 
    Since darwin has always supported nanosleep() and sched_yield() 
and the atypical behavior of the -enable-libstdcxx-time configure option
obviously confuses some end-users, it seems rational to just default to
--enable-libstdcxx-time=yes on for darwin. Is there another way to
achieve this short of defining both _GLIBCXX_USE_NANOSLEEP and
_GLIBCXX_USE_SCHED_YIELD in config/os/bsd/darwin/os_defines.h? If so,
I'll revise the patch to use that approach.
           Jack

> 
> At least a comment in the configure bits admitting defeat, people.
> 
> -benjamin

Patch hide | download patch | download mbox

Index: libstdc++-v3/config/os/bsd/darwin/os_defines.h
===================================================================
--- libstdc++-v3/config/os/bsd/darwin/os_defines.h	(revision 192222)
+++ libstdc++-v3/config/os/bsd/darwin/os_defines.h	(working copy)
@@ -42,4 +42,9 @@ 
 // Static initializer macro is buggy in darwin, see libstdc++/51906
 #define _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC
 
+// Use nanosleep and sched_yield in libc for time.clock and 
+// thread.thread.this in C++11 standard.
+#define _GLIBCXX_USE_NANOSLEEP 1
+#define _GLIBCXX_USE_SCHED_YIELD 1
+
 #endif