gettimeofday seconds rollover problem?

[Andrew Sinclair]

I agree with Chris, inline are definitely the way to go if we can.

Originally, I was a little concerned about the separate asm volatile
statements in Chris's example. Now I think that it is good. The article
mentions ordering between asm volatile statements could be an issue, but
those condition codes should set it right anyway.

Also flashing interrupts would need to be dealt with if it is called.
Sometimes checks are made following a flash interrupt that some variable
is still ok. What if that check was moved before a flash ?

#define M68K_BARRIER() asm volatile ( "" : : : "memory", "cc" )

...

#if ( M68K_COLDFIRE_ARCH == 1 )
#define m68k_flash_interrupts( _level ) \
   do { register unsigned32 _tmpsr = 0x0700; \
		asm __volatile__ ( "move.w %2,%%sr\n\t" \
		       "or.l   %2,%1\n\t" \
		       "move.w %1,%%sr" \
		       : "=d"(_tmpsr) : "0"(_tmpsr), "d"(_level)); \
         M68K_BARRIER( ); \
   } while( 0 )
#else
#define m68k_flash_interrupts( _level ) \
	do { \
	  asm __volatile__ ( "move.w  %0,%%sr\n\t" \
                 "or.w    #0x0700,%%sr" \
                    : : "d" (_level)) \
         M68K_BARRIER( ); \
   } while( 0 )
#endif


-----Original Message-----
From: Chris Johns [mailto:chrisj at rtems.org] 
Sent: Friday, 24 February 2006 9:15 AM
To: Eric Norum
Cc: Till Straumann; Andrew Sinclair; Ian Caddy; Joel Sherrill;
rtems-users
Subject: Re: gettimeofday seconds rollover problem?

Eric Norum wrote:
> On Feb 23, 2006, at 2:24 PM, Till Straumann wrote:
> 
>> One moment, folks -- I'm not sure the "memory" approach is best for 
>> all architectures. On CPUs with many registers, making 
>> rtems_interrupt_enable()/disable a real subroutine might be cheaper.
>>

I think for the m68k targets having the calls in lined is still the way
to go.

>> In any case, I believe that declaring the relevant global variables 
>> 'volatile' is the correct way to deal with this problem.
> 
> 
> I agree that  _TOD_Current and _TOD_Seconds_since_epoch should be 
> declared volatile since they are certainly updated by code in a way 
> that is not apparent to the optimizer.

I also agree. I have added RTEMS_VOLATILE to
sccore/include/rtems/system.h and changed tod.h. Joel, should I commit
this change ?

> However, I'm concerned that
> there are other places where code may be unexpectedly hoisted outside

> an enable/disable pair.   Adding the "memory" seems prudent.

I would like to see the barrier added. I would be concerned we miss a
place that should be handled that is not.

I also wonder if the compiler can move accesses to a volatile object to
a place it thinks is ok while still making sure the object access is
performed. I see we have 2 issues that should be addressed. In the case
of gettimeofday I tested volatile variables and/or a barrier and the
same code was generated, how-ever a newer compiler may need both.

> 
> Notice that the compiler is quite happy to use after the enable/
disable 
> the value which was saved in d0 before the enable/disable.   How has 
> adding the "memory" hurt anything?
> 

Do we only need the barrier on the "enable" interrupts ?

> Even if we don't add "memory" to the list of clobbered registers I 
> think that we should add "cc" since the condition codes really do get 
> mangled by the interrupt enable/disable operation.
> 

Yes.

Regards
Chris