[PATCH 2/2] rtems: Add more clock tick functions

[Sebastian Huber]

On 24/08/14 11:42, Pavel Pisa wrote:
> Hello Sebastian,
>
> On Sunday 24 of August 2014 10:54:15 Sebastian Huber wrote:
>> On 08/22/2014 10:14 PM, Pavel Pisa wrote:
>>>>> +RTEMS_INLINE_ROUTINE bool rtems_clock_ticks_before(
>>>>>
>>>>>>> +  rtems_interval ticks
>>>>>>> +)
>>>>>>> +{
>>>>>>> +  return ( (int32_t) ticks - (int32_t) _Watchdog_Ticks_since_boot )
>>>>>>>> 0;
>>>>>
>>>>> Why not just return _Watchdog_Ticks_since_boot < ticks;
>>
>> Yes, this doesn't work if the counter overflows.
>>
>>> For sure not, to have correctly working overflow arithmetics
>>> it is required to use subtraction of unsigned types and then
>>> to limit result to signed type of same size.
>>>
>>> Overflow of subtraction of signed types is undefined according
>>> to the C standard. The implementation causing exception is correct
>>> but seldom used (can be enabled for MIPS on GCC). But comparison
>>> is often optimized. So for example next code
>>
>> Conversion of too large unsigned integers to singed integers is also
>> undefined.  I assume two's complement arithmetic here.
>
> I do not remember such uncertainty from C99 standard reading.
> Should be checked.

I used the wrong term, it is not undefined, it is implementation-defined.  See 
6.3.1.3 Signed and unsigned integers, paragraph 3.

> What I am sure about is, that C standard
> defines behavior equivalent to 2'complement arithmetics
> for shifts and overflows for unsigned types and conversion
> from/to signed has take that into account somehow even if
> signed representation is not 2'complement. And we know that
> on all RTEMS target signed is 2'complement. So standard
> declares signed as unsigned as safe.

Yes.

>
>>> int fnc(int32_t x)
>>> {
>>>     if ((x + 0x7fffffff) < -0x10000000) {
>>>       return 1;
>>>     } else {
>>>       return 0;
>>>     }
>>> }
>>>
>>> can be legally optimized to
>>>
>>> int fnc(int32_t x)
>>> {
>>>     return 0;
>>> }
>>>
>>> because for any valid x number <-0x80000000;0x7fffffff>
>>> arithmetic value of the sum cannot be smaller than -1.
>>> And this kind of optimization is seen in reality.
>>>
>>> So even Sebastian's above code which tries to prevent
>>> overflow case can be misoptimized and broken.
>>>
>>> Correct is
>>>
>>>     return ( (int32_t) ( (uint32_t) ticks - (uint32_t)
>>> _Watchdog_Ticks_since_boot ) ) > 0;
>>
>> This version works also with two's complement arithmetic.  I don't think
>> the compiler can optimize my version in the same way as your example,
>> since _Watchdog_Ticks_since_boot is a global volatile variable.  Linux
>> uses the same approach for time_before().
>
> But your code subtact int32_t values and it is not safe even if it
> is volatile. I.e. if volatile value is read to register then it is
> processed as regular value for given occurrence in the expression.
> I.e. for comparison with constant (not so probable for time) it can behave
> exactly as I have described. Even worse if compared to non constant
> ((in32_t)start_time + 1000) because then compiler expect that this
> subexpression never overflows and it does not compare final substraction
> for carry but for signed less and equal.

The difference is that we have > 0 and not < -0x10000000, so this range 
deduction by the compiler doesn't work.  Anyway, I will use your version now 
since you have the better arguments:

RTEMS_INLINE_ROUTINE bool rtems_clock_tick_before(
   rtems_interval ticks
)
{
   return (int32_t) ( ticks - _Watchdog_Ticks_since_boot ) > 0;
}

[...]


-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.