gmane.comp.gcc.libstdc++.devel

Re: [patch] Default to --enable-libstdcxx-time=auto

Jakub Jelinek — 2013-05-25T07:56:53

Additional comment to that, the fact that the OS has clock_gettime with
CLOCK_REALTIME resp. gettimeofday still doesn't say anything about the
resolution/precision of the clock, you can have clock_gettime
(CLOCK_REALTIME, &tp) that only provides second or millisecond resolution,
or hundreds if microseconds, tens of nanoseconds etc.  There is clock_getres
in POSIX, but <chrono> doesn't query this and if the resolution decision
has to be static (nanoseconds vs. microseconds vs. seconds), I'd hope it
should be ok to always use nanoseconds duration, especially when most of the
important targets will either in 4.8.1+ or at least in 4.9.0+ use
clock_gettime.

Jakub

Re: [patch] Default to --enable-libstdcxx-time=auto

Jakub Jelinek — 2013-05-25T06:21:46

Yeah, I think that is reasonable, with one nit, see below.



In this _V2 inline namespace, I think we should change that:
    struct system_clock
    {
#ifdef _GLIBCXX_USE_CLOCK_REALTIME
      typedef chrono::nanoseconds                               duration;
#elif defined(_GLIBCXX_USE_GETTIMEOFDAY)
      typedef chrono::microseconds                              duration;
#else
      typedef chrono::seconds                                   duration;
#endif
into:
    struct system_clock
    {
      typedef chrono::nanoseconds                               duration;

Won't the templates then DTRT in:
  return time_point(duration(chrono::seconds(tv.tv_sec)
    + chrono::microseconds(tv.tv_usec)));
(multiply microseconds by 1000 and seconds by 1000000000)
      return system_clock::from_time_t(__sec);
(multiply seconds by 1000000000)?

While this change isn't really necessary for Linux, where usually
_GLIBCXX_USE_CLOCK_REALTIME will be in 4.8.1+ defined and thus the
nanoseconds resolution will be used anyw

Re: [patch] Default to --enable-libstdcxx-time=auto

Benjamin De Kosnik — 2013-05-25T03:26:32


Right, ditto, yes.
 

If we want to care about C++11 ABI compat on release branches
(?), here's another solution.

It scraps the renaming/aliasing approach, and just creates a
compatibility-chrono.cc that mimics the default configuration in 4.8.0.
Users who specially-configured a build with --enable-libstdcxx-time
configure options in 4.8.0 are breaking an experimental
C++ ABI anyway, so I'm not going to solve for anything but the default
case.

For chrono.cc, there is an inline namespace that mangles system_clock
and steady_clock in a new way to prevent ambiguous uses. In addition, I
make a controversial decision and just forward-declare clocks that
libstdc++ cannot do correctly, instead of using typedefs that clearly
are only going to get us into trouble as we change things down the
road. That's probably only appropriate for trunk.

Flame on, cowboys, flame on.

This still doesn't solve the solaris 9/10 librt issue.

tested x86_64/linux

-benjamin

Re: [patch] Default to --enable-libstdcxx-time=auto

Jonathan Wakely — 2013-05-25T01:32:24

Right.


Or use the new ABI attribute so the "new" system_clock mangles
differently to the "old" system_clock.

I prefer to break it though - if we're going to paint ourselves into a
corner and insist on compatibility with semi-functional previous
releases we should stop calling it "experimental".  Alternatively,
while we still call it experimental, let's fix it properly and get it
right.


IMHO yes, that's a big issue.  I believe most users are more
interested in system_clock than in steady_clock, and we should provide
a nanosecond-resolution system_clock if the OS supports it.



Yeah, this stuff's hard.  But I hope we can get a stable ABI for 4.9,
even if it's incompatible with previous releases.

Re: [patch] Default to --enable-libstdcxx-time=auto

Benjamin De Kosnik — 2013-05-24T20:23:04

So you'll need conditional LDFLAGS for solaris 9 and 10 only?


This is ok.Thanks!

-benjamin

Re: [google gcc-4_7,gcc-4_8,integration] Add bounds checks to vector

Diego Novillo — 2013-05-24T18:22:19

OK.


Diego.

Re: [patch] Default to --enable-libstdcxx-time=auto

Jakub Jelinek — 2013-05-24T15:32:35

Well, I've hand edited the Linux baseline_symbols.txt rather than really
regenerating (can't bootstrap/regtest that quickly on all arches),
but I see no reason why Solaris can't be done the same way.
If I understand it right, Solaris doesn't allow foo< at >VER_1 together
with foo< at >< at >VER_2, thus the attached updated patch should contain what I
expect on Solaris, thus just regtest of that should reveal if the assumption
is right.

Ok, 4.8 x86_64/i686-linux bootstraps running now, will do trunk ones next,
and simultaneously with that ppc/ppc64/s390/s390x/arm-linux.

Jakub
2013-05-24  Jakub Jelinek  <jakub< at >redhat.com>

* src/c++11/chrono.cc: If _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL,
include unistd.h and sys/syscall.h.  If _GLIBCXX_COMPATIBILITY_CXX0X,
don't define system_clock::is_steady, system_clock::now() and
steady_clock::is_steady.
(std::chrono::system_clock::now()): If
_GLIBCXX_USE_CLOCK_GETTIME_SYSCALL, call
syscall (SYS_clock_gettime, ...) instead of clock_gettime (...).
(std::chrono::system_clock::now()

Re: [patch] Default to --enable-libstdcxx-time=auto

Rainer Orth — 2013-05-24T13:51:50


Yep, I'd seen that.


I've now fired off the various Solaris bootstraps.  It may take until
tomorrow that I can report final results.

Rainer

Re: [patch] Default to --enable-libstdcxx-time=auto

Rainer Orth — 2013-05-24T13:35:15


I'm not so sure: to me, that's just an implementation detail.


It should work on the 4.8 branch (I'll include it in my bootstraps this
weekend), but on mainline the failures due to missing nanosleep/-lrt
will remain.

I've just successfully bootstrapped the following patch on
i386-pc-solaris2.10 and x86_64-unknown-linux-gnu.  It fixes the
libstdc++ testsuite failures I'd reported before.

Rainer


2013-05-24  Rainer Orth  <ro< at >CeBiTec.Uni-Bielefeld.DE>

gcc/cp:
* g++spec.c (TIMELIB): Define.
(WITHLIBC, SKIPOPT): Adjust values.
(lang_specific_driver): Add TIME_LIBRARY if not passed explicitly.

gcc:
* config/sol2.h (TIME_LIBRARY): Define.

Re: [patch] Default to --enable-libstdcxx-time=auto

Rainer Orth — 2013-05-24T09:00:26


It occured to me that there might be a far less intrusive option to still
allow a Solaris backport: instead of going the libstdc++.spec route
(which I still think is the correct way forward), statically handle -lrt
addition in g++spec.c, controlled by a macro defined only in config/sol2.h.

Such a patch could be added to mainline and 4.8 branch now, and mainline
later changed to use libstdc++.spec instead.

Rainer

Re: [patch] Default to --enable-libstdcxx-time=auto

Jakub Jelinek — 2013-05-24T08:58:23

Oops, posted too early, while that worked on Linux due to
_GLIBCXX_USE_CLOCK_MONOTONIC_SYSCALL, it didn't work when
_GLIBCXX_USE_CLOCK_MONOTONIC wasn't defined, the return of the other clock
now() result resulted in error.

So here is once more updated version that works even when
_GLIBCXX_USE_CLOCK_MONOTONIC isn't defined.

Jakub
2013-05-24  Jakub Jelinek  <jakub< at >redhat.com>

* src/c++11/chrono.cc: If _GLIBCXX_USE_CLOCK_MONOTONIC_SYSCALL,
include unistd.h and sys/syscall.h.  If _GLIBCXX_COMPATIBILITY_CXX0X,
don't define system_clock::is_steady, system_clock::now() and
steady_clock::is_steady.
(std::chrono::system_clock::now()): If
_GLIBCXX_USE_CLOCK_MONOTONIC_SYSCALL, call
syscall (SYS_clock_gettime, ...) instead of clock_gettime (...).
(std::chrono::system_clock::now()): Likewise.  Add weak attribute
if _GLIBCXX_COMPATIBILITY_CXX0X and compatibility-chrono.cc will
be non-empty.
* src/Makefile.am (cxx11_sources): Add compatibility-chrono.cc.
(compatibility-chrono.lo, compatibility-chrono.o): N

Re: [patch] Default to --enable-libstdcxx-time=auto

Daniel Krügler — 2013-05-24T08:54:18

2013/5/24 Jakub Jelinek <jakub< at >redhat.com>:

Yes, steady_clock::is_steady needs to be true and to be usable within
constant expressions.

- Daniel

Re: [patch] Default to --enable-libstdcxx-time=auto

Jakub Jelinek — 2013-05-24T07:57:05

Ugh, that makes =auto pretty much unbackportable, but it seems Solaris is
the only problematic OS here.  The goal of
_ZNSt6chrono12steady_clock3nowEv< at >< at >GLIBCXX_3.4.19
already in 4.8.1 was to allow Linux users (and with partial backport of
=auto not including Solaris perhaps also FreeBSD/NetBSD/OpenBSD) to let
users that get C++ core language feature completeness also use this
(Jonathan/Benjamin, is that right?).

So, let's talk about other options, is constexpr/const on
std::chrono::steady_clock::is_steady
required by the standard (as in, could we export
_ZNSt6chrono12steady_clock9is_steadyE< at >< at >GLIBCXX_3.4.19
and let the library say which case it is, it would be constexpr when
we know for sure, and a static variable otherwise)?
But looking at C++11 draft, in [time.clock.steady]
it even says that is_steady is true (then we are violating the standard
even when we represent steady_clock as typedef system_clock steady_clock;).

Thus, what about this version (just slightly modified, headers remain the
same, but we

Re: [patch] Default to --enable-libstdcxx-time=auto

Jakub Jelinek — 2013-05-23T21:06:11

In that case, either the --enable-libstdcxx-time=auto patch needs to be
backported to 4.8.1, or at least a small portion of it (do that auto thing
on Solaris only)?
Have steady_clock::now() as < at >GLIBCXX_3.4.17 + < at >< at >GLIBCXX_3.4.19 on Linux,
< at >< at >GLIBCXX_3.4.20 on Solaris, something else on other OSes would be quite
confusing.


Jakub

Re: [patch] Default to --enable-libstdcxx-time=auto

Rainer Orth — 2013-05-23T20:56:25


If I do so, I would probably get an abi_check failure: with your patch,
std::chrono::steady_clock::now() ends up in GLIBCXX_3.4.19 while it
should appear in GLIBCXX_3.4.20 since this is new in GCC 4.9.


Right, it only got enable by defaulting --enable-libstdcxx-time to auto.

Rainer

Re: [patch] Default to --enable-libstdcxx-time=auto

Jakub Jelinek — 2013-05-23T20:50:22

Ah, I see, gnu.ver has some
#ifdef HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT
#endif
guards there, does it work if you add it around the
GLIBCXX_3.4.17 std::chrono::steady_clock::now() definition?
Seems the solaris baseline_symbols.txt files don't meantion
this symbol, thus it wasn't exported before and thus the
compat definition there isn't really needed for Solaris.

Jakub

Re: 4.9 library ABI transition (was: string/list ABI changes)

Paolo Carlini — 2013-05-23T17:15:19


Hi

Jason Merrill <jason< at >redhat.com> ha scritto:


I understand, but frankly I don't feel like I understand the ABI strategy well enough. I'd rather prefer if say Benjamin could get the ball rolling with your help. Then I will be able to help for the rest of the work. For instance if you could show me in practice how you promote ext/vstring to basic_ string then I can certainly do the rest.

Paolo

Re: [google gcc-4_7,gcc-4_8,integration] Add bounds checks to vector

Paul Pluzhnikov — 2013-05-23T17:13:25


FWIW, we keep this under a separate macro so we can turn it on or off
independent of other build options.

Our current code looks like this:

#if !defined(__google_stl_debug_vector)
# if !defined(NDEBUG)
#  define __google_stl_debug_vector 1
# endif
#endif


Keying off NDEBUG rather than __OPTIMIZE__ seems like a more
consistent approach -- if you want assert()s, then you probably also
want these checks.

4.9 library ABI transition (was: string/list ABI changes)

Jason Merrill — 2013-05-23T17:00:53

Paolo, are you planning to work on this stuff?  I'd really like to get 
this going ASAP so that it has time to mature before 4.9.

Is there a list of all the classes you want to change?

Jason

Re: [google gcc-4_7,gcc-4_8,integration] Add bounds checks to vector

Jonathan Wakely — 2013-05-23T16:14:17

I was wondering the other day whether we should put these checks on
trunk and enable them automatically when !defined(__OPTIMIZE__)

Now that we have -Og you could use that to disable the checks without
sacrificing debuggability.

Re: [patch] Default to --enable-libstdcxx-time=auto

Jonathan Wakely — 2013-05-23T12:04:36

This looks great to me, thanks.


We should consider doing that yes, but it's less urgent.


I like that plan.