Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libgloss: riscv: Add support of clock() for semihosting #59

Merged
merged 1 commit into from
May 16, 2024

Conversation

kolerov
Copy link
Contributor

@kolerov kolerov commented May 10, 2024

It's necessary to implement _times() in libsemihosting to add support of clock() function. SYS_ELAPSED and SYS_TICKFREQ semihosting calls are used to get a number of ticks which could be compatible with clock() interface and CLOCKS_PER_SEC value.

clock() function from libc must return value which gives us seconds when it's divided by CLOCKS_PER_SEC. By default, CLOCKS_PER_SEC is 10**6 for RISC-V. However, semihosting's tick frequency may differ and usually it's 10**9. Thus, we have to obtain a real value of the tick frequency for the current semihosting setup and calculate a multiplier and use it for adjusting semihosting's number of ticks. E.g., if semihosting's ticks frequency is 10**9 then multiplier is 10**9 / 10**6 = 1000.

Moreover, I found that Picolibc does not adjust CLOCKS_PER_SEC value in its semihosting implementation - it uses naked SYS_ELAPSED call for getting value for clock(). Thus, clock() / CLOCKS_PER_SEC does not return a valid number of seconds.

@kolerov kolerov self-assigned this May 10, 2024
It's necessary to implement _times() in libsemihosting to add
support of clock() function. SYS_ELAPSED and SYS_TICKFREQ
semihosting calls are used to get a number of ticks which
could be compatible with clock() interface and CLOCKS_PER_SEC
value.

clock() function from libc must return value which gives us seconds
when it's divided by CLOCKS_PER_SEC. By default, CLOCKS_PER_SEC
is 10**6 for RISC-V. However, semihosting's tick frequency may
differ and usually it's 10**9. Thus, we have to obtain a real value
of the tick frequency for the current semihosting setup and
calculate a multiplier and use it for adjusting semihosting's
number of ticks. E.g., if semihosting's ticks frequency is 10**9
then multiplier is 10**9 / 10**6 = 1000.

Signed-off-by: Yuriy Kolerov <kolerov93@gmail.com>
@kolerov kolerov merged commit edccb7a into arcvx May 16, 2024
2 checks passed

/* SYS_ELAPSED semihosting call returns a single 64-bit value for 64-bit
targets and two 32-bit values for 32-bit targets. */
#if __riscv_xlen == 32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make it more future proof, I'd do it like this:

#if __riscv_xlen == 32
  ...
#elif __riscv_xlen == 64
  ...
#else
  #error "Unexpected bitness."
#endif

of the tick frequency for the current semihosting setup and
calculate a multiplier and use it for adjusting semihosting's
number of ticks. E.g., if semihosting's ticks frequency is 10**9 then
multiplier is 10**9 / 10**6 = 1000. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: the multiplier


if (!initialized)
{
/* clock() function from libc must return value which gives us seconds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: ... return a value that when is multiplied by CLOCKS_PER_SEC would result in seconds passed.

/* SYS_ELAPSED semihosting call returns a number of ticks. */
syscall_errno (SEMIHOST_elapsed, data_block);

#if __riscv_xlen == 32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as earlier.

@kolerov kolerov deleted the arcvx-clock branch May 23, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants