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

Reply via email to