| Message ID | 20121009003921.GA25678@bromo.med.uc.edu |
|---|---|
| State | New |
| Headers | show |
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.
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.
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.
> 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
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
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
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.