[Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]

Hi all,

here is a straightforward patch for the intrinsic procedure
SYSTEM_CLOCK. It does two things:
1) It reduces the resolution of the int8 version from 1 nanosecond to
1 microsecond (COUNT_RATE = 1000000).
2) It adds an int16 version with nanosecond precision.

The motivation for item #1 was mainly that the actual precision is
usually not better than 1 microsec anyway (unless linking with -lrt).
This results in SYSTEM_CLOCK giving values whose last three digits are
zero. One can argue that this is not a dramatic misbehavior, but it
has disadvantages for certain applications, like e.g. using
SYSTEM_CLOCK to initialize the random seed in a Monte-Carlo
simulation. In general, I would say that the value of COUNT_RATE
should not be larger than the actual precision of the clock used.

Moreover, the microsecond resolution for int8 arguments has the
advantage that it is compatible with ifort's behavior. Also I think a
resolution of 1 microsecond is sufficient for most applications. If
someone really needs more, he can now use the int16 version (and link
with -lrt).

Regtested on x86_64-unknown-linux-gnu (although we don't actually seem
to have any test cases for SYSTEM_CLOCK yet). Ok for trunk?

Btw, does it make sense to also add an int2 version? If yes, which
resolution? Note that most other compilers seem to have an int2
version of SYSTEM_CLOCK ...

Cheers,
Janus


2012-12-01  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/55548
    * gfortran.map (GFORTRAN_1.5): Add _gfortran_system_clock_16.
    * intrinsics/system_clock.c (system_clock_8): Change resolution to
    one microsec.
    (system_clock_16): New function (with nanosecond resolution).

2012-12-01  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/55548
    * intrinsic.texi (SYSTEM_CLOCK): Update documentation of SYSTEM_CLOCK.

2012-12-01  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/55548
    * gfortran.dg/system_clock_1.f90: New test case.

[-- Attachment #2: pr55548.diff --]
[-- Type: application/octet-stream, Size: 4321 bytes --]

Index: gcc/fortran/intrinsic.texi
===================================================================
--- gcc/fortran/intrinsic.texi	(revision 193887)
+++ gcc/fortran/intrinsic.texi	(working copy)
@@ -12015,11 +12015,12 @@ available, the implementation falls back to a pote
 resolution realtime clock.
 
 @var{COUNT_RATE} and @var{COUNT_MAX} vary depending on the kind of the
-arguments.  For @var{kind=8} arguments, @var{COUNT} represents
-nanoseconds, and for @var{kind=4} arguments, @var{COUNT} represents
-milliseconds. Other than the kind dependency, @var{COUNT_RATE} and
-@var{COUNT_MAX} are constant, however the particular values are
-specific to @command{gfortran}.
+arguments.  For @var{kind=4} arguments, @var{COUNT} represents milliseconds,
+for @var{kind=8} arguments, @var{COUNT} represents microseconds, and for
+@var{kind=16} arguments, @var{COUNT} represents nanoseconds.
+@var{COUNT_MAX} usually equals @code{HUGE(COUNT_MAX)}. Other than the kind
+dependency, @var{COUNT_RATE} and @var{COUNT_MAX} are constant, however the
+particular values are specific to @command{gfortran}.
 
 If there is no clock, @var{COUNT} is set to @code{-HUGE(COUNT)}, and
 @var{COUNT_RATE} and @var{COUNT_MAX} are set to zero.
Index: libgfortran/gfortran.map
===================================================================
--- libgfortran/gfortran.map	(revision 193887)
+++ libgfortran/gfortran.map	(working copy)
@@ -1192,6 +1192,7 @@ GFORTRAN_1.4 {
 GFORTRAN_1.5 {
   global:
     _gfortran_ftell2;
+    _gfortran_system_clock_16;
 } GFORTRAN_1.4; 
 
 F2C_1.0 {
Index: libgfortran/intrinsics/system_clock.c
===================================================================
--- libgfortran/intrinsics/system_clock.c	(revision 193887)
+++ libgfortran/intrinsics/system_clock.c	(working copy)
@@ -108,11 +108,14 @@ export_proto(system_clock_4);
 extern void system_clock_8 (GFC_INTEGER_8 *, GFC_INTEGER_8 *, GFC_INTEGER_8 *);
 export_proto(system_clock_8);
 
+extern void system_clock_16 (GFC_INTEGER_16 *, GFC_INTEGER_16 *, GFC_INTEGER_16 *);
+export_proto(system_clock_16);
 
 /* prefix(system_clock_4) is the INTEGER(4) version of the SYSTEM_CLOCK
-   intrinsic subroutine.  It returns the number of clock ticks for the current
-   system time, the number of ticks per second, and the maximum possible value
-   for COUNT.  On the first call to SYSTEM_CLOCK, COUNT is set to zero. */
+   intrinsic subroutine (with millisecond resolution).
+   It returns the number of clock ticks for the current system time,
+   the number of ticks per second, and the maximum possible value for COUNT.
+   On the first call to SYSTEM_CLOCK, COUNT is set to zero. */
 
 void
 system_clock_4(GFC_INTEGER_4 *count, GFC_INTEGER_4 *count_rate,
@@ -159,14 +162,14 @@ system_clock_4(GFC_INTEGER_4 *count, GFC_INTEGER_4
 }
 
 
-/* INTEGER(8) version of the above routine.  */
+/* INTEGER(8) version of the above routine (with microsecond resolution).  */
 
 void
 system_clock_8 (GFC_INTEGER_8 *count, GFC_INTEGER_8 *count_rate,
 		GFC_INTEGER_8 *count_max)
 {
 #undef TCK
-#define TCK 1000000000
+#define TCK 1000000
   GFC_INTEGER_8 cnt;
   GFC_INTEGER_8 mx;
 
@@ -205,3 +208,51 @@ system_clock_8 (GFC_INTEGER_8 *count, GFC_INTEGER_
   if (count_max != NULL)
     *count_max = mx;
 }
+
+
+/* INTEGER(16) version with nanosecond resolution.  */
+
+void
+system_clock_16 (GFC_INTEGER_16 *count, GFC_INTEGER_16 *count_rate,
+		 GFC_INTEGER_16 *count_max)
+{
+#undef TCK
+#define TCK 1000000000
+  GFC_INTEGER_16 cnt;
+  GFC_INTEGER_16 mx;
+
+  time_t secs;
+  long nanosecs;
+
+  if (sizeof (secs) < sizeof (GFC_INTEGER_4))
+    internal_error (NULL, "secs too small");
+
+  if (gf_gettime_mono (&secs, &nanosecs) == 0)
+    {
+      GFC_UINTEGER_16 ucnt = (GFC_UINTEGER_16) secs * TCK;
+      ucnt += (nanosecs + 500000000 / TCK) / (1000000000 / TCK);
+      if (ucnt > GFC_INTEGER_16_HUGE)
+	cnt = ucnt - GFC_INTEGER_16_HUGE - 1;
+      else
+	cnt = ucnt;
+      mx = GFC_INTEGER_16_HUGE;
+    }
+  else
+    {
+      if (count != NULL)
+	*count = - GFC_INTEGER_16_HUGE;
+      if (count_rate != NULL)
+	*count_rate = 0;
+      if (count_max != NULL)
+	*count_max = 0;
+
+      return;
+    }
+
+  if (count != NULL)
+    *count = cnt;
+  if (count_rate != NULL)
+    *count_rate = TCK;
+  if (count_max != NULL)
+    *count_max = mx;
+}

[-- Attachment #3: system_clock_1.f90 --]
[-- Type: application/octet-stream, Size: 861 bytes --]

! { dg-do run }
!
! PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)
!
! Contributed by Janus Weil <janus@gcc.gnu.org>

program test_system_clock

  implicit none
  integer(4) :: t4, rate4, cmax4
  integer(8) :: t8, rate8, cmax8
  integer(16) :: t16, rate16, cmax16
  intrinsic :: SYSTEM_CLOCK, HUGE

  ! test integer(4): millisecond resolution
  call SYSTEM_CLOCK (t4, rate4, cmax4)
  if (rate4 /= 1000) call abort
  if (cmax4 /= HUGE(cmax4)) call abort

  ! test integer(8): microsecond resolution
  call SYSTEM_CLOCK (t8, rate8, cmax8)
  if (rate8 /= 1000000) call abort
  if (cmax8 /= HUGE(cmax8)) call abort

  ! test integer(16): nanosecond resolution
  call SYSTEM_CLOCK (t16, rate16, cmax16)
  if (rate16 /= 1000000000) call abort
  if (cmax16 /= HUGE(cmax16)) call abort

end program

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janne Blomqvist]

On Sat, Dec 1, 2012 at 1:17 PM, Janus Weil <janus@gcc.gnu.org> wrote:
> Hi all,
>
> here is a straightforward patch for the intrinsic procedure
> SYSTEM_CLOCK. It does two things:
> 1) It reduces the resolution of the int8 version from 1 nanosecond to
> 1 microsecond (COUNT_RATE = 1000000).
> 2) It adds an int16 version with nanosecond precision.
>
> The motivation for item #1 was mainly that the actual precision is
> usually not better than 1 microsec anyway (unless linking with -lrt).
> This results in SYSTEM_CLOCK giving values whose last three digits are
> zero. One can argue that this is not a dramatic misbehavior, but it
> has disadvantages for certain applications, like e.g. using
> SYSTEM_CLOCK to initialize the random seed in a Monte-Carlo
> simulation. In general, I would say that the value of COUNT_RATE
> should not be larger than the actual precision of the clock used.
>
> Moreover, the microsecond resolution for int8 arguments has the
> advantage that it is compatible with ifort's behavior. Also I think a
> resolution of 1 microsecond is sufficient for most applications. If
> someone really needs more, he can now use the int16 version (and link
> with -lrt).
>
> Regtested on x86_64-unknown-linux-gnu (although we don't actually seem
> to have any test cases for SYSTEM_CLOCK yet). Ok for trunk?
>
> Btw, does it make sense to also add an int2 version? If yes, which
> resolution? Note that most other compilers seem to have an int2
> version of SYSTEM_CLOCK ...

No, not Ok.

IIRC there was some discussion about COUNT_RATE back when the
nanosecond resolution system_clock feature was developed, and one idea
was to have different count rates depending on whether clock_gettime
is available, as you also suggest in the PR. In the end it was decided
to keep a constant count rate as a consistent rate was seen as more
important than "wasting" a few of the least significant bits when
nanosecond resolution is not available, since int8 has sufficient
range for several centuries even with nanosecond resolution. Anyway, I
don't feel particularly strongly about this, and if there now is a
consensus to have a changing count rate, I can live with that.

But, I do object to providing only microsecond resolution for the int8
version. Nanosecond resolution is indeed not necessary in most cases,
but like the saying goes, "Precision, like virginity, is never
regained once lost.". Reducing the resolution is a regression for
those users who have relied on this feature; I, for one, have several
test and benchmark programs which depend on nanosecond resolution for
the int8 system_clock. OTOH, if one wants a microsecond count rate,
converting the count value is just a single statement following the
system_clock call.

Needing to link with librt in order to access clock_gettime is an
unfortunate wart in glibc, but other C libraries exist out there
(heck, given the success of Android, glibc is certainly a minority
even if you limit yourself to Linux), and of those that provide
clock_gettime, most have it directly in libc and not in a separate
library.

As for the int16 and int2 variants, F2003 requires that the arguments
can be of any kind. AFAICS, this means that different arguments can be
of different kind. So in order to avoid a combinatorial explosion, the
only sensible way is for the frontend to create temporaries of
suitable kind, then call an appropriate library function etc. As
mentioned, int8 already provides several centuries range with
nanosecond resolution, and thus I see no reason to provide a int16
version, but rather the frontend should create int8 temporaries, call
system_clock_8, then copy the results back to the actual int16
arguments. F2003 requires than all supported kinds should work, which
would include kinds 1 and 2, although those versions obviously suffer
from a lack of either range, precision or both, to the extent that
they are quite useless in practice. I think they could be handled by
just setting count to -huge(), count_rate and count_max to 0 in the
frontend and not bothering to make a library call at all. Also, F2003
specifies than COUNT_RATE can be of type real as well; I think this
should be handled similarly, by the frontend copying the result from
an integer temporary. So in the end I don't think we need new library
entry points, but rather the temporary variable handling stuff in the
frontend.

-- 
Janne Blomqvist

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janus Weil]

Hi Janne,

thanks for your feedback ...


>> here is a straightforward patch for the intrinsic procedure
>> SYSTEM_CLOCK. It does two things:
>> 1) It reduces the resolution of the int8 version from 1 nanosecond to
>> 1 microsecond (COUNT_RATE = 1000000).
>> 2) It adds an int16 version with nanosecond precision.
>>
>> The motivation for item #1 was mainly that the actual precision is
>> usually not better than 1 microsec anyway (unless linking with -lrt).
>> This results in SYSTEM_CLOCK giving values whose last three digits are
>> zero. One can argue that this is not a dramatic misbehavior, but it
>> has disadvantages for certain applications, like e.g. using
>> SYSTEM_CLOCK to initialize the random seed in a Monte-Carlo
>> simulation. In general, I would say that the value of COUNT_RATE
>> should not be larger than the actual precision of the clock used.
>>
>> Moreover, the microsecond resolution for int8 arguments has the
>> advantage that it is compatible with ifort's behavior. Also I think a
>> resolution of 1 microsecond is sufficient for most applications. If
>> someone really needs more, he can now use the int16 version (and link
>> with -lrt).
>>
>> Regtested on x86_64-unknown-linux-gnu (although we don't actually seem
>> to have any test cases for SYSTEM_CLOCK yet). Ok for trunk?
>>
>> Btw, does it make sense to also add an int2 version? If yes, which
>> resolution? Note that most other compilers seem to have an int2
>> version of SYSTEM_CLOCK ...
>
> No, not Ok.
>
> IIRC there was some discussion about COUNT_RATE back when the
> nanosecond resolution system_clock feature was developed, and one idea
> was to have different count rates depending on whether clock_gettime
> is available, as you also suggest in the PR. In the end it was decided
> to keep a constant count rate as a consistent rate was seen as more
> important than "wasting" a few of the least significant bits when
> nanosecond resolution is not available, since int8 has sufficient
> range for several centuries even with nanosecond resolution. Anyway, I
> don't feel particularly strongly about this, and if there now is a
> consensus to have a changing count rate, I can live with that.

my patch does not implement such a "changing" (or library-dependent)
count rate, but I would indeed prefer this over the current behavior
(even more so if there is consensus to reject my current patch).


> But, I do object to providing only microsecond resolution for the int8
> version. Nanosecond resolution is indeed not necessary in most cases,
> but like the saying goes, "Precision, like virginity, is never
> regained once lost.". Reducing the resolution is a regression for
> those users who have relied on this feature;

well, I wouldn't count it as a real regression, since:
a) The Fortran standard does not require a certain resolution, but
defines it to be processor dependent.
b) You cannot rely on different compilers giving the same resolution
values, therefore you cannot expect a fixed resolution if you want to
write portable code.


> I, for one, have several
> test and benchmark programs which depend on nanosecond resolution for
> the int8 system_clock.

You mean they depend on the count values being delivered in units of
nanoseconds? In a portable code you cannot depend on getting actual
nanosecond precision, since it may not be available on a certain
system.


> OTOH, if one wants a microsecond count rate,
> converting the count value is just a single statement following the
> system_clock call.

Certainly converting the units is not a problem.

BUT: In a way I think that the current behavior is "lying" to the
user. It pretends to provide a nanosecond resolution, while in fact
the values you get might only be precise up to 1 microsecond.

In my opinion the COUNT_RATE argument should rather return a value
that corresponds to the actual precision of the clock. It should not
just report some fixed unit of measure, but tell me something about
the precision of my clock. If it gives me a value in microseconds, I
can simply convert it to nanoseconds if I need to (knowing that the
precision is only a microsecond). But it it gives me a value in
nanoseconds, how can I know that I'm not getting nanosecond precision?


In summary, I think we should do one of the following:
1) Provide different versions with fixed COUNT_RATE and let the user
choose one via the kind of the argument. This is what we do right now,
and my patch was expanding on this by providing an additional option.
If you want nanosecond resolution, you can still use the int16
version, but now you can also choose to have microsecond resolution
(via the int8 option).
2) Adapt the COUNT_RATE according to the actual precision of the clock
on the system.
3) A combination of both, where the user could choose via
int4/int8/int16 that he does not need more than milli/micro/nanosecond
resolution. Then SYSTEM_CLOCK would try to provide this precision, but
reduce COUNT_RATE to a lower value if the desired precision can not be
provided.

I think the last one would be most desirable. Any other opinions?


> As for the int16 and int2 variants, F2003 requires that the arguments
> can be of any kind. AFAICS, this means that different arguments can be
> of different kind.

I will not go into this discussion right now, since this was not the
original intention of my patch.

Cheers,
Janus

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 2726 bytes --]

Hi all,

since the first version of my SYSTEM_CLOCK patch was not so well
received, I propose here an alternative patch: It does not use a fixed
COUNT_RATE, but adjusts it to match the resolution of the underlying
library function which is used. For the int4 version it additionally
limits COUNT_RATE to 1000, in order to provide a reasonable maximum
timespan. The patch also has the effect that the reported COUNT_RATE
can now depend e.g. on the linker flags used (as does the precision of
the clock: 1 nanosec with -lrt and 1 microsec without).

Does anyone agree with me that this represents an improvement of the
SYSTEM_CLOCK behavior?

Cheers,
Janus



2012/12/1 Janus Weil <janus@gcc.gnu.org>:
> Hi all,
>
> here is a straightforward patch for the intrinsic procedure
> SYSTEM_CLOCK. It does two things:
> 1) It reduces the resolution of the int8 version from 1 nanosecond to
> 1 microsecond (COUNT_RATE = 1000000).
> 2) It adds an int16 version with nanosecond precision.
>
> The motivation for item #1 was mainly that the actual precision is
> usually not better than 1 microsec anyway (unless linking with -lrt).
> This results in SYSTEM_CLOCK giving values whose last three digits are
> zero. One can argue that this is not a dramatic misbehavior, but it
> has disadvantages for certain applications, like e.g. using
> SYSTEM_CLOCK to initialize the random seed in a Monte-Carlo
> simulation. In general, I would say that the value of COUNT_RATE
> should not be larger than the actual precision of the clock used.
>
> Moreover, the microsecond resolution for int8 arguments has the
> advantage that it is compatible with ifort's behavior. Also I think a
> resolution of 1 microsecond is sufficient for most applications. If
> someone really needs more, he can now use the int16 version (and link
> with -lrt).
>
> Regtested on x86_64-unknown-linux-gnu (although we don't actually seem
> to have any test cases for SYSTEM_CLOCK yet). Ok for trunk?
>
> Btw, does it make sense to also add an int2 version? If yes, which
> resolution? Note that most other compilers seem to have an int2
> version of SYSTEM_CLOCK ...
>
> Cheers,
> Janus
>
>
> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/55548
>     * gfortran.map (GFORTRAN_1.5): Add _gfortran_system_clock_16.
>     * intrinsics/system_clock.c (system_clock_8): Change resolution to
>     one microsec.
>     (system_clock_16): New function (with nanosecond resolution).
>
> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/55548
>     * intrinsic.texi (SYSTEM_CLOCK): Update documentation of SYSTEM_CLOCK.
>
> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/55548
>     * gfortran.dg/system_clock_1.f90: New test case.

[-- Attachment #2: pr55548_v2.diff --]
[-- Type: application/octet-stream, Size: 3100 bytes --]

Index: libgfortran/intrinsics/system_clock.c
===================================================================
--- libgfortran/intrinsics/system_clock.c	(revision 194017)
+++ libgfortran/intrinsics/system_clock.c	(working copy)
@@ -76,11 +76,12 @@ static int weak_gettime (clockid_t, struct timespe
    is set.
 */
 static int
-gf_gettime_mono (time_t * secs, long * nanosecs)
+gf_gettime_mono (time_t * secs, long * nanosecs, long * tck)
 {
   int err;
 #ifdef HAVE_CLOCK_GETTIME
   struct timespec ts;
+  *tck = 1000000000;
   err = clock_gettime (GF_CLOCK_MONOTONIC, &ts);
   *secs = ts.tv_sec;
   *nanosecs = ts.tv_nsec;
@@ -90,12 +91,14 @@ static int
   if (weak_gettime)
     {
       struct timespec ts;
+      *tck = 1000000000;
       err = weak_gettime (GF_CLOCK_MONOTONIC, &ts);
       *secs = ts.tv_sec;
       *nanosecs = ts.tv_nsec;
       return err;
     }
 #endif
+  *tck = 1000000;
   err = gf_gettime (secs, nanosecs);
   *nanosecs *= 1000;
   return err;
@@ -118,21 +121,20 @@ void
 system_clock_4(GFC_INTEGER_4 *count, GFC_INTEGER_4 *count_rate,
 	       GFC_INTEGER_4 *count_max)
 {
-#undef TCK
-#define TCK 1000
   GFC_INTEGER_4 cnt;
   GFC_INTEGER_4 mx;
 
   time_t secs;
-  long nanosecs;
+  long nanosecs, tck;
 
   if (sizeof (secs) < sizeof (GFC_INTEGER_4))
     internal_error (NULL, "secs too small");
 
-  if (gf_gettime_mono (&secs, &nanosecs) == 0)
+  if (gf_gettime_mono (&secs, &nanosecs, &tck) == 0)
     {
-      GFC_UINTEGER_4 ucnt = (GFC_UINTEGER_4) secs * TCK;
-      ucnt += (nanosecs + 500000000 / TCK) / (1000000000 / TCK);
+      tck = tck>1000 ? 1000 : tck;
+      GFC_UINTEGER_4 ucnt = (GFC_UINTEGER_4) secs * tck;
+      ucnt += (nanosecs + 500000000 / tck) / (1000000000 / tck);
       if (ucnt > GFC_INTEGER_4_HUGE)
 	cnt = ucnt - GFC_INTEGER_4_HUGE - 1;
       else
@@ -153,7 +155,7 @@ system_clock_4(GFC_INTEGER_4 *count, GFC_INTEGER_4
   if (count != NULL)
     *count = cnt;
   if (count_rate != NULL)
-    *count_rate = TCK;
+    *count_rate = tck;
   if (count_max != NULL)
     *count_max = mx;
 }
@@ -165,21 +167,19 @@ void
 system_clock_8 (GFC_INTEGER_8 *count, GFC_INTEGER_8 *count_rate,
 		GFC_INTEGER_8 *count_max)
 {
-#undef TCK
-#define TCK 1000000000
   GFC_INTEGER_8 cnt;
   GFC_INTEGER_8 mx;
 
   time_t secs;
-  long nanosecs;
+  long nanosecs, tck;
 
   if (sizeof (secs) < sizeof (GFC_INTEGER_4))
     internal_error (NULL, "secs too small");
 
-  if (gf_gettime_mono (&secs, &nanosecs) == 0)
+  if (gf_gettime_mono (&secs, &nanosecs, &tck) == 0)
     {
-      GFC_UINTEGER_8 ucnt = (GFC_UINTEGER_8) secs * TCK;
-      ucnt += (nanosecs + 500000000 / TCK) / (1000000000 / TCK);
+      GFC_UINTEGER_8 ucnt = (GFC_UINTEGER_8) secs * tck;
+      ucnt += (nanosecs + 500000000 / tck) / (1000000000 / tck);
       if (ucnt > GFC_INTEGER_8_HUGE)
 	cnt = ucnt - GFC_INTEGER_8_HUGE - 1;
       else
@@ -201,7 +201,7 @@ system_clock_8 (GFC_INTEGER_8 *count, GFC_INTEGER_
   if (count != NULL)
     *count = cnt;
   if (count_rate != NULL)
-    *count_rate = TCK;
+    *count_rate = tck;
   if (count_max != NULL)
     *count_max = mx;
 }

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Tobias Burnus]

Janne Blomqvist wrote:
> Needing to link with librt in order to access clock_gettime is an
> unfortunate wart in glibc, but other C libraries exist out there
> (heck, given the success of Android, glibc is certainly a minority
> even if you limit yourself to Linux), and of those that provide
> clock_gettime, most have it directly in libc and not in a separate
> library.

I think that has changed in GLIBC 2.17: 
http://sourceware.org/git/?p=glibc.git;a=blob_plain;f=NEWS;hb=HEAD

Tobias

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janus Weil]

>> Needing to link with librt in order to access clock_gettime is an
>> unfortunate wart in glibc, but other C libraries exist out there
>> (heck, given the success of Android, glibc is certainly a minority
>> even if you limit yourself to Linux), and of those that provide
>> clock_gettime, most have it directly in libc and not in a separate
>> library.
>
> I think that has changed in GLIBC 2.17:
> http://sourceware.org/git/?p=glibc.git;a=blob_plain;f=NEWS;hb=HEAD

good, having clock_gettime in the core library will certainly improve
the situation.

Still, for pre-2.17 versions of glibc or libraries which don't provide
clock_gettime, I think we should fix the discrepancy between nominal
resolution and actual precision of SYSTEM_CLOCK with int8 arguments.

I have provided two different patches in this direction, and I would
be very grateful for comments regarding which one would be preferable
(I now tend to prefer the second one), and what would be needed to
make it suitable for acceptance into trunk.

Cheers,
Janus

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janne Blomqvist]

On Sun, Dec 2, 2012 at 3:19 PM, Janus Weil <janus@gcc.gnu.org> wrote:
> Hi all,
>
> since the first version of my SYSTEM_CLOCK patch was not so well
> received, I propose here an alternative patch: It does not use a fixed
> COUNT_RATE, but adjusts it to match the resolution of the underlying
> library function which is used. For the int4 version it additionally
> limits COUNT_RATE to 1000, in order to provide a reasonable maximum
> timespan. The patch also has the effect that the reported COUNT_RATE
> can now depend e.g. on the linker flags used (as does the precision of
> the clock: 1 nanosec with -lrt and 1 microsec without).
>
> Does anyone agree with me that this represents an improvement of the
> SYSTEM_CLOCK behavior?

Hi,

this looks better than the first path, IMHO. I worry slightly that a
changing count rate could confuse users, but OTOH I agree you have a
point in that the current behavior can give users a false impression
of better accuracy than what is actually provided.

An issue with the idea of changing count rate is that using an OS API
providing some particular resolution doesn't in itself guarantee that
the underlying hardware clock is actually capable of that resolution.
But I suppose there's not much we can do about that anyway (if one has
clock_gettime, one probably also has clock_getres, but at least on
Linux that seems to always return 1 ns as the resolution).

Unless somebody else has objections, together with a documentation
patch and a ChangeLog entry, this is Ok for trunk.

(As a follow-up, somebody familiar with Windows could provide some
info on what the resolution there is, and can we do something to
improve our current implementation; e.g. do both mingw and cygwin
provide gettimeofday() or do we really fall back to time()? )

>
> Cheers,
> Janus
>
>
>
> 2012/12/1 Janus Weil <janus@gcc.gnu.org>:
>> Hi all,
>>
>> here is a straightforward patch for the intrinsic procedure
>> SYSTEM_CLOCK. It does two things:
>> 1) It reduces the resolution of the int8 version from 1 nanosecond to
>> 1 microsecond (COUNT_RATE = 1000000).
>> 2) It adds an int16 version with nanosecond precision.
>>
>> The motivation for item #1 was mainly that the actual precision is
>> usually not better than 1 microsec anyway (unless linking with -lrt).
>> This results in SYSTEM_CLOCK giving values whose last three digits are
>> zero. One can argue that this is not a dramatic misbehavior, but it
>> has disadvantages for certain applications, like e.g. using
>> SYSTEM_CLOCK to initialize the random seed in a Monte-Carlo
>> simulation. In general, I would say that the value of COUNT_RATE
>> should not be larger than the actual precision of the clock used.
>>
>> Moreover, the microsecond resolution for int8 arguments has the
>> advantage that it is compatible with ifort's behavior. Also I think a
>> resolution of 1 microsecond is sufficient for most applications. If
>> someone really needs more, he can now use the int16 version (and link
>> with -lrt).
>>
>> Regtested on x86_64-unknown-linux-gnu (although we don't actually seem
>> to have any test cases for SYSTEM_CLOCK yet). Ok for trunk?
>>
>> Btw, does it make sense to also add an int2 version? If yes, which
>> resolution? Note that most other compilers seem to have an int2
>> version of SYSTEM_CLOCK ...
>>
>> Cheers,
>> Janus
>>
>>
>> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/55548
>>     * gfortran.map (GFORTRAN_1.5): Add _gfortran_system_clock_16.
>>     * intrinsics/system_clock.c (system_clock_8): Change resolution to
>>     one microsec.
>>     (system_clock_16): New function (with nanosecond resolution).
>>
>> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/55548
>>     * intrinsic.texi (SYSTEM_CLOCK): Update documentation of SYSTEM_CLOCK.
>>
>> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/55548
>>     * gfortran.dg/system_clock_1.f90: New test case.



-- 
Janne Blomqvist

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janne Blomqvist]

On Sun, Dec 2, 2012 at 4:46 PM, Tobias Burnus <burnus@net-b.de> wrote:
> Janne Blomqvist wrote:
>>
>> Needing to link with librt in order to access clock_gettime is an
>> unfortunate wart in glibc, but other C libraries exist out there
>> (heck, given the success of Android, glibc is certainly a minority
>> even if you limit yourself to Linux), and of those that provide
>> clock_gettime, most have it directly in libc and not in a separate
>> library.
>
>
> I think that has changed in GLIBC 2.17:
> http://sourceware.org/git/?p=glibc.git;a=blob_plain;f=NEWS;hb=HEAD

Ah, nice! That means we can look forward to 1 ns resolution on Linux
systems regardless of whether librt is linked or not; the code is
there already in order to support other systems where clock_* is
already in libc, so all that is required is a recompile against a
newer glibc. As an aside, the reason why we don't unconditionally link
in librt is exactly that explained in the link.


-- 
Janne Blomqvist

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 4315 bytes --]

Hi,

>> since the first version of my SYSTEM_CLOCK patch was not so well
>> received, I propose here an alternative patch: It does not use a fixed
>> COUNT_RATE, but adjusts it to match the resolution of the underlying
>> library function which is used. For the int4 version it additionally
>> limits COUNT_RATE to 1000, in order to provide a reasonable maximum
>> timespan. The patch also has the effect that the reported COUNT_RATE
>> can now depend e.g. on the linker flags used (as does the precision of
>> the clock: 1 nanosec with -lrt and 1 microsec without).
>>
>> Does anyone agree with me that this represents an improvement of the
>> SYSTEM_CLOCK behavior?
>
> this looks better than the first path, IMHO. I worry slightly that a
> changing count rate could confuse users, but OTOH I agree you have a
> point in that the current behavior can give users a false impression
> of better accuracy than what is actually provided.
>
> An issue with the idea of changing count rate is that using an OS API
> providing some particular resolution doesn't in itself guarantee that
> the underlying hardware clock is actually capable of that resolution.
> But I suppose there's not much we can do about that anyway (if one has
> clock_gettime, one probably also has clock_getres, but at least on
> Linux that seems to always return 1 ns as the resolution).
>
> Unless somebody else has objections, together with a documentation
> patch and a ChangeLog entry, this is Ok for trunk.

thanks, Janne. Here is an updated patch with docu and ChangeLog. I
will wait another day before committing, in order to allow for further
comments.

Cheers,
Janus


2012-12-02  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/55548
    * intrinsics/system_clock.c (gf_gettime_mono): Add argument 'tck',
    which returns the clock resolution.
    (system_clock_4): Get resolution from gf_gettime_mono, but limit to
    1000/s.
    (system_clock_8): Get resolution from gf_gettime_mono.

2012-12-02  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/55548
    * intrinsic.texi (SYSTEM_CLOCK): Update documentation of SYSTEM_CLOCK.




>> 2012/12/1 Janus Weil <janus@gcc.gnu.org>:
>>> Hi all,
>>>
>>> here is a straightforward patch for the intrinsic procedure
>>> SYSTEM_CLOCK. It does two things:
>>> 1) It reduces the resolution of the int8 version from 1 nanosecond to
>>> 1 microsecond (COUNT_RATE = 1000000).
>>> 2) It adds an int16 version with nanosecond precision.
>>>
>>> The motivation for item #1 was mainly that the actual precision is
>>> usually not better than 1 microsec anyway (unless linking with -lrt).
>>> This results in SYSTEM_CLOCK giving values whose last three digits are
>>> zero. One can argue that this is not a dramatic misbehavior, but it
>>> has disadvantages for certain applications, like e.g. using
>>> SYSTEM_CLOCK to initialize the random seed in a Monte-Carlo
>>> simulation. In general, I would say that the value of COUNT_RATE
>>> should not be larger than the actual precision of the clock used.
>>>
>>> Moreover, the microsecond resolution for int8 arguments has the
>>> advantage that it is compatible with ifort's behavior. Also I think a
>>> resolution of 1 microsecond is sufficient for most applications. If
>>> someone really needs more, he can now use the int16 version (and link
>>> with -lrt).
>>>
>>> Regtested on x86_64-unknown-linux-gnu (although we don't actually seem
>>> to have any test cases for SYSTEM_CLOCK yet). Ok for trunk?
>>>
>>> Btw, does it make sense to also add an int2 version? If yes, which
>>> resolution? Note that most other compilers seem to have an int2
>>> version of SYSTEM_CLOCK ...
>>>
>>> Cheers,
>>> Janus
>>>
>>>
>>> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>>>
>>>     PR fortran/55548
>>>     * gfortran.map (GFORTRAN_1.5): Add _gfortran_system_clock_16.
>>>     * intrinsics/system_clock.c (system_clock_8): Change resolution to
>>>     one microsec.
>>>     (system_clock_16): New function (with nanosecond resolution).
>>>
>>> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>>>
>>>     PR fortran/55548
>>>     * intrinsic.texi (SYSTEM_CLOCK): Update documentation of SYSTEM_CLOCK.
>>>
>>> 2012-12-01  Janus Weil  <janus@gcc.gnu.org>
>>>
>>>     PR fortran/55548
>>>     * gfortran.dg/system_clock_1.f90: New test case.
>
>
>
> --
> Janne Blomqvist

[-- Attachment #2: pr55548_v3.diff --]
[-- Type: application/octet-stream, Size: 4620 bytes --]

Index: gcc/fortran/intrinsic.texi
===================================================================
--- gcc/fortran/intrinsic.texi	(revision 194017)
+++ gcc/fortran/intrinsic.texi	(working copy)
@@ -12014,12 +12014,11 @@ nanosecond resolution.  If a high resolution monot
 available, the implementation falls back to a potentially lower
 resolution realtime clock.
 
-@var{COUNT_RATE} and @var{COUNT_MAX} vary depending on the kind of the
-arguments.  For @var{kind=8} arguments, @var{COUNT} represents
-nanoseconds, and for @var{kind=4} arguments, @var{COUNT} represents
-milliseconds. Other than the kind dependency, @var{COUNT_RATE} and
-@var{COUNT_MAX} are constant, however the particular values are
-specific to @command{gfortran}.
+@var{COUNT_RATE} is system dependent and can vary depending on the kind of the
+arguments. For @var{kind=4} arguments, @var{COUNT} usually represents
+milliseconds, while for @var{kind=8} arguments, @var{COUNT} typically
+represents micro- or nanoseconds. @var{COUNT_MAX} usually equals
+@code{HUGE(COUNT_MAX)}.
 
 If there is no clock, @var{COUNT} is set to @code{-HUGE(COUNT)}, and
 @var{COUNT_RATE} and @var{COUNT_MAX} are set to zero.
Index: libgfortran/intrinsics/system_clock.c
===================================================================
--- libgfortran/intrinsics/system_clock.c	(revision 194017)
+++ libgfortran/intrinsics/system_clock.c	(working copy)
@@ -64,6 +64,7 @@ static int weak_gettime (clockid_t, struct timespe
    Arguments:
    secs     - OUTPUT, seconds
    nanosecs - OUTPUT, nanoseconds
+   tk       - OUTPUT, clock resolution [counts/sec]
 
    If the target supports a monotonic clock, the OUTPUT arguments
    represent a monotonically incrementing clock starting from some
@@ -76,11 +77,12 @@ static int weak_gettime (clockid_t, struct timespe
    is set.
 */
 static int
-gf_gettime_mono (time_t * secs, long * nanosecs)
+gf_gettime_mono (time_t * secs, long * nanosecs, long * tck)
 {
   int err;
 #ifdef HAVE_CLOCK_GETTIME
   struct timespec ts;
+  *tck = 1000000000;
   err = clock_gettime (GF_CLOCK_MONOTONIC, &ts);
   *secs = ts.tv_sec;
   *nanosecs = ts.tv_nsec;
@@ -90,12 +92,14 @@ static int
   if (weak_gettime)
     {
       struct timespec ts;
+      *tck = 1000000000;
       err = weak_gettime (GF_CLOCK_MONOTONIC, &ts);
       *secs = ts.tv_sec;
       *nanosecs = ts.tv_nsec;
       return err;
     }
 #endif
+  *tck = 1000000;
   err = gf_gettime (secs, nanosecs);
   *nanosecs *= 1000;
   return err;
@@ -118,21 +122,20 @@ void
 system_clock_4(GFC_INTEGER_4 *count, GFC_INTEGER_4 *count_rate,
 	       GFC_INTEGER_4 *count_max)
 {
-#undef TCK
-#define TCK 1000
   GFC_INTEGER_4 cnt;
   GFC_INTEGER_4 mx;
 
   time_t secs;
-  long nanosecs;
+  long nanosecs, tck;
 
   if (sizeof (secs) < sizeof (GFC_INTEGER_4))
     internal_error (NULL, "secs too small");
 
-  if (gf_gettime_mono (&secs, &nanosecs) == 0)
+  if (gf_gettime_mono (&secs, &nanosecs, &tck) == 0)
     {
-      GFC_UINTEGER_4 ucnt = (GFC_UINTEGER_4) secs * TCK;
-      ucnt += (nanosecs + 500000000 / TCK) / (1000000000 / TCK);
+      tck = tck>1000 ? 1000 : tck;
+      GFC_UINTEGER_4 ucnt = (GFC_UINTEGER_4) secs * tck;
+      ucnt += (nanosecs + 500000000 / tck) / (1000000000 / tck);
       if (ucnt > GFC_INTEGER_4_HUGE)
 	cnt = ucnt - GFC_INTEGER_4_HUGE - 1;
       else
@@ -153,7 +156,7 @@ system_clock_4(GFC_INTEGER_4 *count, GFC_INTEGER_4
   if (count != NULL)
     *count = cnt;
   if (count_rate != NULL)
-    *count_rate = TCK;
+    *count_rate = tck;
   if (count_max != NULL)
     *count_max = mx;
 }
@@ -165,21 +168,19 @@ void
 system_clock_8 (GFC_INTEGER_8 *count, GFC_INTEGER_8 *count_rate,
 		GFC_INTEGER_8 *count_max)
 {
-#undef TCK
-#define TCK 1000000000
   GFC_INTEGER_8 cnt;
   GFC_INTEGER_8 mx;
 
   time_t secs;
-  long nanosecs;
+  long nanosecs, tck;
 
   if (sizeof (secs) < sizeof (GFC_INTEGER_4))
     internal_error (NULL, "secs too small");
 
-  if (gf_gettime_mono (&secs, &nanosecs) == 0)
+  if (gf_gettime_mono (&secs, &nanosecs, &tck) == 0)
     {
-      GFC_UINTEGER_8 ucnt = (GFC_UINTEGER_8) secs * TCK;
-      ucnt += (nanosecs + 500000000 / TCK) / (1000000000 / TCK);
+      GFC_UINTEGER_8 ucnt = (GFC_UINTEGER_8) secs * tck;
+      ucnt += (nanosecs + 500000000 / tck) / (1000000000 / tck);
       if (ucnt > GFC_INTEGER_8_HUGE)
 	cnt = ucnt - GFC_INTEGER_8_HUGE - 1;
       else
@@ -201,7 +202,7 @@ system_clock_8 (GFC_INTEGER_8 *count, GFC_INTEGER_
   if (count != NULL)
     *count = cnt;
   if (count_rate != NULL)
-    *count_rate = TCK;
+    *count_rate = tck;
   if (count_max != NULL)
     *count_max = mx;
 }

Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)

[Janus Weil]

>> Unless somebody else has objections, together with a documentation
>> patch and a ChangeLog entry, this is Ok for trunk.
>
> thanks, Janne. Here is an updated patch with docu and ChangeLog. I
> will wait another day before committing, in order to allow for further
> comments.

Committed as r194105.

Cheers,
Janus



> 2012-12-02  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/55548
>     * intrinsics/system_clock.c (gf_gettime_mono): Add argument 'tck',
>     which returns the clock resolution.
>     (system_clock_4): Get resolution from gf_gettime_mono, but limit to
>     1000/s.
>     (system_clock_8): Get resolution from gf_gettime_mono.
>
> 2012-12-02  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/55548
>     * intrinsic.texi (SYSTEM_CLOCK): Update documentation of SYSTEM_CLOCK.