gettimeofday seconds rollover problem?

Thu Feb 23 21:09:48 UTC 2006

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Till,

up to date I had the impression, that the "asm volatile" statements are
unmovable for the optimizer, because that's what "volatile" was defined
for. My current view is, that this is only true for "volatile"
statements relative to each other, but not relative to non-volatile
statements. Therefore, the optimizer is allowed to move the
"non-volatile" accesses to _TOD* out from the "en/disable_interrupt"
section.

Setting the _TOD* variables as volatile definitively makes sense. But I
suspect, that it will take a long time until we have identified all
other code fragments, which might trip over rearranged
"en/disable_interrupt" problems.

So we definitively need a waterproof and nonmovable implementation of
the enable/disable macros. I like the "barrier" construct proposed, and
I think for many architectures this might be the most efficent
implementation. But due to the CPU dependent implementation, I think it
is no problem to make these macros call a function.

I still do not get the idea, why a function call would be more efficent
than a macro on some architectures.

wkr,
Thomas.

Till Straumann schrieb:
> 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.
> 
> In any case, I believe that declaring the relevant global variables
> 'volatile' is the correct way to deal with this problem.
> 
> The standard says:
> 
> "An object that has volatile-qualified type may be modified in ways
> unknown to the
> implementation or have other unknown side effects. Therefore any
> expression referring
> to such an object shall be evaluated strictly according to the rules of
> the abstract machine,
> as described in 5.1.2.3. Furthermore, at every sequence point the value
> last stored in the
> object shall agree with that prescribed by the abstract machine, except
> as modified by the
> unknown factors mentioned previously.114) What constitutes an access to
> an object that
> has volatile-qualified type is implementation-defined.
> 
> -----114) A volatile declaration may be used to describe an object
> corresponding to a memory-mapped
> input/output port or an object accessed by an asynchronously
> interrupting function. Actions on
> objects so declared shall not be ‘‘optimized out’’ by an implementation
> or reordered except as
> permitted by the rules for evaluating expressions."
> 
> Seems exactly what we want and indeed, while the powerpc port
> (gcc-4.0.2) currently
> also exhibits the problem, when recompiled with both _TOD_Current and
> _TOD_Seconts_since_epoch
> declared 'volatile' the respective read operations are not moved out of
> the protected
> section anymore.
> 
> -- Till
> 
> BTW: I also checked m68k/coldfire/gcc-4.0.2 -- declaring the relevant
> globals 'volatile' produces
> correctly protected code.
> 
> Eric Norum wrote:
> 
>> Ack!
>> Looks like gcc really is getting aggressive.
>>
>> Here's the code for a ColdFire MCF5282:
>>
>> (gdb) disassem gettimeofday
>> Dump of assembler code for function gettimeofday:
>> 0x00127d8c <gettimeofday+0>: moveal %sp@(4),%a0
>> 0x00127d90 <gettimeofday+4>: tstl %a0
>> 0x00127d92 <gettimeofday+6>: beqs 0x127dca <gettimeofday+62>
>> 0x00127d94 <gettimeofday+8>: movel #1792,%d0
>> 0x00127d9a <gettimeofday+14>: movel %d0,%d1
>> 0x00127d9c <gettimeofday+16>: movew %sr,%d0
>> 0x00127d9e <gettimeofday+18>: orl %d0,%d1
>> 0x00127da0 <gettimeofday+20>: movew %d1,%sr
>> 0x00127da2 <gettimeofday+22>: movel 0x219f22 <_TOD_Current+24>,%d1
>> 0x00127da8 <gettimeofday+28>: movew %d0,%sr
>> 0x00127daa <gettimeofday+30>: movel 0x219e46
>> <_TOD_Seconds_since_epoch>,%d0
>> 0x00127db0 <gettimeofday+36>: addil #567993600,%d0
>> 0x00127db6 <gettimeofday+42>: movel %d0,%a0@
>> 0x00127db8 <gettimeofday+44>: movel 0x219f72
>> <_TOD_Microseconds_per_tick>,%d0
>> 0x00127dbe <gettimeofday+50>: mulsl %d1,%d0
>> 0x00127dc2 <gettimeofday+54>: movel %d0,%a0@(4)
>> 0x00127dc6 <gettimeofday+58>: clrl %d0
>> 0x00127dc8 <gettimeofday+60>: rts
>>
>>
>> As you can see, the 'ticks' value is read with interrupts disabled,
>> but the 'seconds since epoch' is not.
>>
>>
>> norume at pyramidion 221> m68k-rtems4.7-gcc --version
>> m68k-rtems4.7-gcc (GCC) 4.0.2
>>
>>
>>
>> I made the following change:
>> Index: cpukit/score/cpu/m68k/rtems/score/m68k.h
>> ===================================================================
>> RCS file: /usr1/CVS/rtems/cpukit/score/cpu/m68k/rtems/score/m68k.h,v
>> retrieving revision 1.24
>> diff -u -r1.24 m68k.h
>> --- cpukit/score/cpu/m68k/rtems/score/m68k.h 7 Jun 2005 16:44:18 -0000
>> 1.24
>> +++ cpukit/score/cpu/m68k/rtems/score/m68k.h 23 Feb 2006 16:29:19 -0000
>> @@ -280,17 +280,19 @@
>> asm volatile ( "move.w %%sr,%0\n\t" \
>> "or.l %0,%1\n\t" \
>> "move.w %1,%%sr" \
>> - : "=d" (_level), "=d"(_tmpsr) : "1"(_tmpsr) ); \
>> + : "=d" (_level), "=d"(_tmpsr) : "1"(_tmpsr) \
>> + : "memory", "cc" ); \
>> } while( 0 )
>> #else
>> #define m68k_disable_interrupts( _level ) \
>> asm volatile ( "move.w %%sr,%0\n\t" \
>> "or.w #0x0700,%%sr" \
>> - : "=d" (_level))
>> + : "=d" (_level) \
>> + : : "memory", "cc" )
>> #endif
>> #define m68k_enable_interrupts( _level ) \
>> - asm volatile ( "move.w %0,%%sr " : : "d" (_level));
>> + asm volatile ( "move.w %0,%%sr " : : "d" (_level) : "memory", "cc");
>> #if ( M68K_COLDFIRE_ARCH == 1 )
>> #define m68k_flash_interrupts( _level ) \
>> @@ -298,13 +300,15 @@
>> asm volatile ( "move.w %2,%%sr\n\t" \
>> "or.l %2,%1\n\t" \
>> "move.w %1,%%sr" \
>> - : "=d"(_tmpsr) : "0"(_tmpsr), "d"(_level) ); \
>> + : "=d"(_tmpsr) : "0"(_tmpsr), "d"(_level) \
>> + : "memory", "cc"); \
>> } while( 0 )
>> #else
>> #define m68k_flash_interrupts( _level ) \
>> asm volatile ( "move.w %0,%%sr\n\t" \
>> "or.w #0x0700,%%sr" \
>> - : : "d" (_level))
>> + : : "d" (_level) \
>> + : "memory", "cc" )
>> #endif
>> #define m68k_get_interrupt_level( _level ) \
>>
>>
>>
>> Now the code looks good:
>>
>> (gdb) disassem gettimeofday
>> Dump of assembler code for function gettimeofday:
>> 0x00127d8c <gettimeofday+0>: movel %d2,%sp at -
>> 0x00127d8e <gettimeofday+2>: moveal %sp@(8),%a0
>> 0x00127d92 <gettimeofday+6>: tstl %a0
>> 0x00127d94 <gettimeofday+8>: beqs 0x127dcc <gettimeofday+64>
>> 0x00127d96 <gettimeofday+10>: movel #1792,%d0
>> 0x00127d9c <gettimeofday+16>: movel %d0,%d1
>> 0x00127d9e <gettimeofday+18>: movew %sr,%d0
>> 0x00127da0 <gettimeofday+20>: orl %d0,%d1
>> 0x00127da2 <gettimeofday+22>: movew %d1,%sr
>> 0x00127da4 <gettimeofday+24>: movel 0x219e06
>> <_TOD_Seconds_since_epoch>,%d1
>> 0x00127daa <gettimeofday+30>: movel 0x219ee2 <_TOD_Current+24>,%d2
>> 0x00127db0 <gettimeofday+36>: movew %d0,%sr
>> 0x00127db2 <gettimeofday+38>: addil #567993600,%d1
>> 0x00127db8 <gettimeofday+44>: movel %d1,%a0@
>> 0x00127dba <gettimeofday+46>: movel 0x219f32
>> <_TOD_Microseconds_per_tick>,%d0
>> 0x00127dc0 <gettimeofday+52>: mulsl %d2,%d0
>> 0x00127dc4 <gettimeofday+56>: movel %d0,%a0@(4)
>> 0x00127dc8 <gettimeofday+60>: clrl %d0
>> 0x00127dca <gettimeofday+62>: bras 0x127ddc <gettimeofday+80>
>> 0x00127dcc <gettimeofday+64>: jsr 0x185270 <__errno>
>> 0x00127dd2 <gettimeofday+70>: moveal %d0,%a0
>> 0x00127dd4 <gettimeofday+72>: movel #14,%a0@
>> 0x00127dda <gettimeofday+78>: moveq #-1,%d0
>> 0x00127ddc <gettimeofday+80>: movel %sp at +,%d2
>> 0x00127dde <gettimeofday+82>: rts
>>
>>

- --
- --------------------------------------------
IMD Ingenieurbuero fuer Microcomputertechnik
Thomas Doerfler           Herbststrasse 8
D-82178 Puchheim          Germany
email:    Thomas.Doerfler at imd-systems.de
PGP public key available at:
     http://www.imd-systems.de/pgpkey_en.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFD/iSbwHyg4bDtfjQRAm49AJ4yUC7Ebeyn/zJ97JWJvz00ZVSjJQCeJnjE
4bbVnY1n4YvyVPJj1E635B4=
=SxUT
-----END PGP SIGNATURE-----