[PATCH v2] c-user: Document new clock manager directives

[Sebastian Huber]

On 11/11/2021 08:02, Sebastian Huber wrote:> On 09/11/2021 13:06, 
Sebastian Huber wrote:
>> On 09/11/2021 08:50, Sebastian Huber wrote:
>>> On 09/11/2021 08:41, Chris Johns wrote:
>>>>> We could also use something like this:
>>>>>
>>>>> static inline struct timespec rtems_clock_get_realtime(void)
>>>>> {
>>>>>    struct timespec time_snapshot;
>>>>>
>>>>>    _Timecounter_Nanotime( &time_snapshot );
>>>>>
>>>>>    return time_snapshot;
>>>>> }
>>>>>
>>>>> Unfortunately GCC is not able to optimize this.
>>>>>
>>>> Ah OK. This can be fixed and the performance improved but once the 
>>>> API is set it
>>>> cannot change or do you think we can add a check later and not break 
>>>> the API?
>>>
>>> I filed a GCC bug for this:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103150
>>>
>>> It seems I was not the only one noticing issues related to structure 
>>> returns:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101926
>>>
>>> However, if we want a foolproof API, then I would prefer the 
>>> structure return over the return status and pointer argument. 
>>> Compilers may get better in the future. clang has similar issues, so 
>>> this is not only a GCC problem.
>>
>> We have at least three options for the API:
>>
>> 1. Use the existing FreeBSD implementation as is:
>>
>> void rtems_clock_get_realtime(struct timespec *);
>>
>> This is the easiest and most efficient approach.
>>
>> 2. Check for NULL and return a status:
>>
>> rtems_status_code rtems_clock_get_realtime(struct timespec *);
>>
>> This requires a wrapper function which is a bit less efficient and 
>> needs more code/testing:
>>
>> rtems_status_code
>> rtems_clock_get_realtime(struct timespec *time_snapshot)
>> {
>>    if ( time_snapshot == NULL ) {
>>      return RTEMS_INVALID_ADDRESS;
>>    }
>>
>>    _Timecounter_Nanotime( time_snapshot );
>>    return RTEMS_SUCCESSFUL;
>> }
>>
>> 3. Return the structure and use the existing implementation:
>>
>> static inline struct timespec rtems_clock_get_realtime(void)
>> {
>>    struct timespec time_snapshot;
>>
>>    _Timecounter_Nanotime( &time_snapshot );
>>
>>    return time_snapshot;
>> }
>>
>> This is currently not well supported by existing compilers:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103150
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101926
>>
> 
> 4. Do nothing for a NULL pointer:
> 
> void rtems_clock_get_realtime(struct timespec *);
> 
> This requires a wrapper function which can use a tail call optimization:
> 
> void
> rtems_clock_get_realtime(struct timespec *time_snapshot)
> {
>    if ( time_snapshot == NULL ) {
>      return;
>    }
> 
>    _Timecounter_Nanotime( time_snapshot );
> }

How do we want to proceed with this? We ship the high performance and 
very useful clock routines from FreeBSD since 2015. I just would like to 
add an RTEMS signature for them and document them in the Clock Manager. 
Currently, these routines are the most efficient way to get clock values 
in RTEMS. Developers afraid of unchecked NULL pointers may use existing 
RTEMS directives or clock_get(). It would be nice if we can decide on a 
way forward.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/