This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gfortran] Implementation SYSTEM_CLOCK intrinsic subroutine


After responding to a followup, I realised I hadn't responded to your original 
message yet. Hopefully it will make more sense now :)

> Changelog for gcc/libgfortran
>
>   2004-05-13  Steven G. Kargl  <kargls@comcast.net>
>
>   * makefile.am: Add intrinsics/system_clock.c
>   * intrinsics/system_clock.c: New file
>   * Makefile.in: regenerated
>   * aclocal.m4: regenerated

Convention is to mention regenerated file, but not include them in the diff.

> Changelog for gcc/gcc/fortran
>
>   2004-05-13  Steven G. Kargl  <kargls@comcast.net>
>
>   *check.c (gfc_check_system_clock): New function
>   *intrinsic.c (add_sym_3s): New function, use it.
>   *intrinsic.h (gfc_check_system_clock,gfc_resolve_system_clock): Add 
>   prototypes
>   *iresolve.c (gfc_resolve_system_clock): New function  

A few issues:

+try
+gfc_check_system_clock (gfc_expr * count, gfc_expr * count_rate,
+                        gfc_expr * count_max)
+{

The standard specifies that all arguments must be of *default* integer type. 
Also, the second parameter of *_check should be the argument number that is 
being checked (ie. not always zero).

We may wish to allow other integer kinds an an extension, but we must still 
ensure that all arguments have the same type, otherwise bad things will 
happen.

+gfc_resolve_system_clock (gfc_code * c)
+{
+  const char *name;
+
+  int kind;
+
+  if (c->ext.actual->expr != NULL)

Depending on the type of checking done above, the resolved name may just be 
based on gfc_default_integer_kind().

<system_clock.c>

#elif HAVE_TIME_H

Should be #elif defined(HAVE_TIME_H)

void
__system_clock_1(int *count, int *count_rate, int *count_max)

Should be declared static.

          t = ((double)tp.tv_sec + tp.tv_usec * 1.e-6) * TCK;
          if (t - t0 < (double)INT_MAX)
            *count = (int) (t - t0);
          else
            {
               *count = 0;
               t0 = t;
            }

This is wrong. In the case where t - t0 > INT_MAX we should wrap, not truncate 
to 0. This behaviour is specified by the standard.
I don't really like the use of double, though I guess I could cope. It's not 
all that hard to perform these calculations with integer arithmetic.

       t1 = TCK * (t - init_time);
       if (t1 < (time_t) INT_MAX)
         *count = (int) t1;
       else
         {
            *count = 0;
            init_time = t;
         }

Likewise. Also t1 may be negative, and you're also assuming sizeof(time_t) >= 
sizeof(INT_MAX). You shouldn't be multiplying by TCK.

    if (count >= 0)							\
      *__count = (GFC_INTEGER_##KIND) count;				\
    else								\
      *__count = - GFC_INTEGER_##KIND##_HUGE;				\

This breaks when sizeof(int) > KIND.

Paul


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]