| Message ID | AANLkTinb=SiyUezxej5kARB9MxtvA6A0YqZc6JHDBfmk@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Feb 1, 2011 at 23:58, Dave Korn <dave.korn.cygwin@gmail.com> wrote: > On 01/02/2011 21:12, Janne Blomqvist wrote: > >> my recent patch to use clock_gettime() made static linking break on >> Linux, since the driver didn't link in librt. It was also pointed out >> that always requiring librt in libgfortran might in principle reduce >> performance for non-threaded programs as librt pulls in libpthread and >> thus the weakrefed IO locking stuff becomes real locks. >> >> The attached patch fixes this by not linking in librt (and this >> libpthread) in libgfortran.so, and instead calls clock_gettime() >> through a weak reference, falling back to gettimeofday()/other >> fallbacks in case librt is not explicitly linked in (or implicitly >> e.g. via -fopenmp). For platforms that provide clock_gettime() in >> libc, clock_gettime() ought to always be used, although I'm not able >> to test this. >> >> Regtested on x86_64-unknown-linux-gnu, tested a simple program to >> ensure usage of clock_gettime() or gettimeofday() depending on whether >> librt is linked. Ok for trunk? > > Does SUPPORTS_WEAK necessarily imply supports weakref? I'm not certain > they're the same thing. Well, that macro is used in quadmath_weak.h and in gthr-posix.h in a similar way, so I guess it's ok.
On Tue, Feb 01, 2011 at 09:58:36PM +0000, Dave Korn wrote: > > Regtested on x86_64-unknown-linux-gnu, tested a simple program to > > ensure usage of clock_gettime() or gettimeofday() depending on whether > > librt is linked. Ok for trunk? > > Does SUPPORTS_WEAK necessarily imply supports weakref? I'm not certain > they're the same thing. Yes, it does. Jakub
On 01/02/2011 21:12, Janne Blomqvist wrote: > my recent patch to use clock_gettime() made static linking break on > Linux, since the driver didn't link in librt. It was also pointed out > that always requiring librt in libgfortran might in principle reduce > performance for non-threaded programs as librt pulls in libpthread and > thus the weakrefed IO locking stuff becomes real locks. > > The attached patch fixes this by not linking in librt (and this > libpthread) in libgfortran.so, and instead calls clock_gettime() > through a weak reference, falling back to gettimeofday()/other > fallbacks in case librt is not explicitly linked in (or implicitly > e.g. via -fopenmp). For platforms that provide clock_gettime() in > libc, clock_gettime() ought to always be used, although I'm not able > to test this. > > Regtested on x86_64-unknown-linux-gnu, tested a simple program to > ensure usage of clock_gettime() or gettimeofday() depending on whether > librt is linked. Ok for trunk? Does SUPPORTS_WEAK necessarily imply supports weakref? I'm not certain they're the same thing. cheers, DaveK
On Feb 1, 2011, at 1:40 PM, Jakub Jelinek wrote: > On Tue, Feb 01, 2011 at 09:58:36PM +0000, Dave Korn wrote: >>> Regtested on x86_64-unknown-linux-gnu, tested a simple program to >>> ensure usage of clock_gettime() or gettimeofday() depending on whether >>> librt is linked. Ok for trunk? >> >> Does SUPPORTS_WEAK necessarily imply supports weakref? I'm not certain >> they're the same thing. > > Yes, it does. darwin is special... Watch for failures on darwin, if you use weakref and plan on not defining everything that's weak.
On 01/02/2011 21:40, Jakub Jelinek wrote: > On Tue, Feb 01, 2011 at 09:58:36PM +0000, Dave Korn wrote: >>> Regtested on x86_64-unknown-linux-gnu, tested a simple program to >>> ensure usage of clock_gettime() or gettimeofday() depending on whether >>> librt is linked. Ok for trunk? >> Does SUPPORTS_WEAK necessarily imply supports weakref? I'm not certain >> they're the same thing. > > Yes, it does. Thanks for the update. I have no objections to the patch. cheers, DaveK
On 02/01/2011 10:12 PM, Janne Blomqvist wrote: > Regtested on x86_64-unknown-linux-gnu, tested a simple program to > ensure usage of clock_gettime() or gettimeofday() depending on whether > librt is linked. Ok for trunk? OK. Thanks for the patch - and watch out for failures on Darwin. Tobias > 2011-02-01 Janne Blomqvist<jb@gcc.gnu.org> > > PR libfortran/47571 > * configure: Regenerated. > * configure.ac: Don't add librt to LIBS. > * intrinsics/time_1.h (weak_gettime): Weakref trickery for > clock_gettime(). > (gf_gettime): Use weak_gettime() instead of clock_gettime(). > >
On Wed, Feb 2, 2011 at 10:15, Tobias Burnus <burnus@net-b.de> wrote: > On 02/01/2011 10:12 PM, Janne Blomqvist wrote: >> >> Regtested on x86_64-unknown-linux-gnu, tested a simple program to >> ensure usage of clock_gettime() or gettimeofday() depending on whether >> librt is linked. Ok for trunk? > > OK. Thanks for the patch - and watch out for failures on Darwin. Thanks for the review, Sending libgfortran/ChangeLog Sending libgfortran/configure Sending libgfortran/configure.ac Sending libgfortran/intrinsics/time_1.h Transmitting file data .... Committed revision 169517. > > Tobias > >> 2011-02-01 Janne Blomqvist<jb@gcc.gnu.org> >> >> PR libfortran/47571 >> * configure: Regenerated. >> * configure.ac: Don't add librt to LIBS. >> * intrinsics/time_1.h (weak_gettime): Weakref trickery for >> clock_gettime(). >> (gf_gettime): Use weak_gettime() instead of clock_gettime(). >> >> > >
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index ed1e2cc..8161659 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -486,11 +486,11 @@ AC_CHECK_LIB([m],[feenableexcept],[have_feenableexcept=yes AC_DEFINE([HAVE_FEENA # At least for glibc, clock_gettime is in librt. But don't pull that # in if it still doesn't give us the function we want. -# This test is copied from libgomp. +# This test is copied from libgomp, and modified to not link in -lrt +# as libgfortran calls clock_gettime via a weak reference. if test $ac_cv_func_clock_gettime = no; then AC_CHECK_LIB(rt, clock_gettime, - [LIBS="-lrt $LIBS" - AC_DEFINE(HAVE_CLOCK_GETTIME, 1, + [AC_DEFINE(HAVE_CLOCK_GETTIME, 1, [Define to 1 if you have the `clock_gettime' function.])]) fi diff --git a/libgfortran/intrinsics/time_1.h b/libgfortran/intrinsics/time_1.h index 58c51af..86e4331 100644 --- a/libgfortran/intrinsics/time_1.h +++ b/libgfortran/intrinsics/time_1.h @@ -189,6 +189,22 @@ gf_cputime (long *user_sec, long *user_usec, long *system_sec, long *system_usec #define GF_CLOCK_MONOTONIC GF_CLOCK_REALTIME #endif +/* Weakref trickery for clock_gettime(). On Glibc, clock_gettime() + requires us to link in librt, which also pulls in libpthread. In + order to avoid this by default, only call clock_gettime() through a + weak reference. */ +#ifdef HAVE_CLOCK_GETTIME +#ifdef SUPPORTS_WEAK +static int weak_gettime (clockid_t, struct timespec *) + __attribute__((__weakref__("clock_gettime"))); +#else +static inline int weak_gettime (clockid_t clk_id, struct timespec *res) +{ + return clock_gettime (clk_id, res); +} +#endif +#endif + /* Arguments: clock_id - INPUT, must be either GF_CLOCK_REALTIME or GF_CLOCK_MONOTONIC secs - OUTPUT, seconds @@ -208,14 +224,18 @@ gf_gettime (int clock_id __attribute__((unused)), time_t * secs, long * nanosecs) { #ifdef HAVE_CLOCK_GETTIME - struct timespec ts; - int err; - err = clock_gettime (clock_id, &ts); - *secs = ts.tv_sec; - if (nanosecs) - *nanosecs = ts.tv_nsec; - return err; -#elif HAVE_GETTIMEOFDAY + if (weak_gettime) + { + struct timespec ts; + int err; + err = weak_gettime (clock_id, &ts); + *secs = ts.tv_sec; + if (nanosecs) + *nanosecs = ts.tv_nsec; + return err; + } +#endif +#ifdef HAVE_GETTIMEOFDAY struct timeval tv; int err; err = gettimeofday (&tv, NULL);