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


On Fri, May 14, 2004 at 06:12:55PM +0100, Paul Brook wrote:
> >
> >   * Makefile.in: regenerated
> >   * aclocal.m4: regenerated
> 
> Convention is to mention regenerated file, but not include them in the diff.

Okay. 

> 
> > 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. 

How do you deal with "gfortran -I8"? 

> Also, the second parameter of *_check should be the argument number that is 
> being checked (ie. not always zero).

I don't understand what you mean.  All arguments are optional and
they can appear in any order.  My patch correctly compiles

  program d
  integer c, cr
  call system_clock(count=c, count_rate=cr)
  print *,  c, cr 
  call system_clock(count_rate=cr,count=c)
  print *,  c, cr 
  end program d

> 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.

Okay, I'll fix this somehow.

> 
> +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.

Can you send me the quote from the standard?  I only have a draft
of the upcoming Fortran standard.  It states

   COUNT   shall be scalar and of type integer.  It is an INTENT(OUT)
           argument.  It is assigned a processor-dependent value based
           on the current value of the processor clock, or -HUGE(COUNT)
           if there is no clock.  The processor-dependent value is
           incremented by one for each clock count until the value
           COUNT_MAX is reached and is reset to zero at the next count.
           It lies in the range 0 to COUNT_MAX if there is a clock.

Two things to note.  COUNT is of type integer not *default* integer type.
The value is reset to 0 not wrapped.

> 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.

I'll look at changing to 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.

Yes, I need to multiply by TCK.  COUNT is the number of clock times.

> 
>     if (count >= 0)							\
>       *__count = (GFC_INTEGER_##KIND) count;				\
>     else								\
>       *__count = - GFC_INTEGER_##KIND##_HUGE;				\
> 
> This breaks when sizeof(int) > KIND.
> 


-- 
Steve


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