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.
Chris
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
