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

[Sebastian Huber]

On 16/11/2021 22:47, Chris Johns wrote:
> 
> On 16/11/21 4:27 am, Sebastian Huber wrote:
>> 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.
> I will leave this for Joel to decide.

Joel, what is your opinion on this? I accidentally committed this patch:

https://lists.rtems.org/pipermail/devel/2021-November/069984.html

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