Function clock() et al not implemented

[tuupola]

Currently all calls to clock() return -1. This is default behavior when _times() is not implemented. There is probably other time related functions affected too.

https://github.com/bminor/newlib/blob/master/newlib/libc/time/clock.c#L53-L63

Usually this is fixed by providing the _times() function as a stub for newlib, but browsing through Pico SDK code I am not sure where it should go.

[kilograham]

What would you expect to see here? wall time related functions make sense, but CPU time?

[tuupola]

AFAIK behaviour of clock() is defined in ANSI C. It should return amount of processor time used by the program. Unit of the return value is CLOCKS_PER_SEC ie. to get number of seconds since boot one could use clock() / CLOCKS_PER_SEC.

The clock() function can already be found in newlib. However since the _times() implementation for Pico is missing it returns -1. Newlib requires several functions to be implemented to be fully functional, I think list of them can be found in libnosys folder.

For some of my projects I am currently using below ad hoc implementation. It works but is not really a proper way to fix this.

#include <time.h>
#include <pico/stdlib.h>

clock_t clock() {
    return (clock_t) time_us_64() / 10000;
}

You could also check how this was implemented in Nuclei SDK.

[kilograham]

You missed my point. gettimeofday() makes perfect sense and should be implemented as it measures elapsed (calendar) time. I know what the clock()/times() spec says, and it refers to processor time.

Why would we make clock()/times() return calendar time (which it is not specced to) rather than -1 for unavailable, which it is specced to?

[tuupola]

I think I do not follow you. I have not suggested clock() should return calendar time. I suggested to implement clock() as defined by ANSI C ie to return processor time.

Or is there a reason why Pico SDK should not implement clock()?

[kilograham]

I suggested to implement clock() as defined by ANSI C ie to return processor time.

well, you posted a link to Nuclei SDK where it is implemented by returning the calendar time from _gettimeofday.

Or is there a reason why Pico SDK should not implement clock()?

My point is we have nothing interesting to report that corresponds to CPU/processor time vs calendar time.

[tuupola]

well, you posted a link to Nuclei SDK where it is implemented by returning the calendar time from _gettimeofday.

I did not request it to be implemented like that. I just posted an example to another SDK. However looking at the code there it seems _gettimeofday() returns value of the MCYCLE register which probably is correct. IIRC this was taken from libgloss.

In any case all this nitpicking is unproductive and it is trivial for user to provide working clock() implementation for third party code which needs it. I understand this is a wontfix?

[lurch]

Perhaps that third party code ought to be updated to cope with clock() returning -1, which as @kilograham says is totally within spec? 🤷

[kilograham]

In any case all this nitpicking is unproductive and it is trivial for user to provide working clock() implementation for third party code which needs it. I understand this is a wontfix?

Just trying to clarifdy what exactly you are asking for. If returning CPU time was trivial, then fine, we could implement it, but we have no useful h/w register to tell us that.

gettimeofday() and clock() are specced to return different things, which is what i'm "nit-picking" over

[tuupola]

For systems which do not provide real CPU time, a monotonic tick count since boot is often provided. This is trivial to implement.

Looking at libgloss code for the default arm implementation seems to use the 100Hz tick from SWI_Clock register. I do not know much about libgloss so I am not sure why this is not picked up. ESP-IDF seems to use FreeRTOS tick count (not since boot but since scheduler was started). RISCV also uses cycles since boot.

Pico SDK could use something similar, or yes just return -1 since real CPU time cannot be provided.

I am currently using the following glue function which provides newlib with 100Hz tick since boot. Works ok for me but does not provide real CPU time.

#include <time.h>
#include <sys/times.h>
#include <pico/stdlib.h>

clock_t _times (struct tms *buffer)
{
    /* Should be a 100Hz tick. Probably because by default arm libgloss */
    /* assumes usage of SWI_Clock which is in centiseconds. */
    clock_t timeval =  time_us_64() / 10000;

    if (buffer) {
        buffer->tms_utime  = timeval;
        buffer->tms_stime  = 0;
        buffer->tms_cutime = 0;
        buffer->tms_cstime = 0;
    }

    return timeval;
};

[kilograham]

OK, so the decision is whether it is more unhelpul to return -1 than to return a monotonically increasing value which represents some sort of time. I can see arguments either way, and I presume the case you are in is that some other code iis calling times() or clock() or something.

Anyway, as part of #27 I plan to move all the newlib syscalls into a new pico_sys library (fleshing out any sensible missing ones), but also add #defines to disable individual ones. (side note, that if we implement a syscall here you cannot then override it in app code without the #define, unless you instead have your build not include pico_sys which can be done a few ways at the build level)

At that point, I guess I don't mind implementing it similarly to above by default, with the expectation that an RTOS/SDK combo might choose to override it anyway.