Skip to content

riscv-rt: Use callee-saved registers for preserving arguments #325

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

Closed
wants to merge 1 commit into from

Conversation

romancardenas
Copy link
Contributor

As pointed out by @janderholm , the current implementation of riscv-rt violates RAM utilization during the .init section. This PR replaces the stack with registers for preserving input parameters.

@hegza can you bless this PR for RVE targets?

Closes #315

@romancardenas romancardenas requested a review from a team as a code owner July 4, 2025 10:45
@romancardenas
Copy link
Contributor Author

romancardenas commented Jul 4, 2025

Another simpler option could be to preserve only register a0 in s1 and document that registers a1 and a2 are reserved in startup functions and therefore must remain unmodified. The more time I think about this option, the more I like it.

@hegza
Copy link
Contributor

hegza commented Jul 5, 2025

Acknowledged, on camp rn. Will read up on Mon.

Copy link
Contributor

@hegza hegza left a comment

Choose a reason for hiding this comment

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

Yeah it works for RVE. Also checked the compiled assembly. Could we use a6 instead of a5 (see below code comment)?

I understand mp_hook typically contains just the code to identify the current core. So constraints don't seem too bad as they're well documented.

//! - `s0`: Contains the original value of `a0` (hart ID).
//! - `s1`: Contains the original value of `a1` (usually a pointer to the device tree blob).
//! - **IN RISCVI TARGETS `s2`**: Contains the original value of `a2` (usually reserved for future use).
//! - **IN RISCVE TARGETS `a5`**: There are no more callee-saved registers, so `a2` is preserved in `a5`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The non-ratified EABI calling conventions reuse also x6/t1, x7/t2, and x14/a4 as callee saved registers (and call them s3--s5) but I understand we must assume RVE + standard ABI here.

Copy link
Contributor

Choose a reason for hiding this comment

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

One take away from EABI would be that it redeclares x16/a6 as s5 (callee save), and we could use that instead of x15/a5 since it's also currently unused. The other substitutions are x6/t1 -> s3 and x7/t2 -> s4 but t1 and t2 are used by the RAM initialization algorithm at that point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do RV32E targets have x16? I thought it was up to x15

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, my mistake on cross-referencing ABI tables. RV32E is up to x15 only (while---of course---EABI also specifies what to do with registers up to x31 on RVI targets).

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.

Stack is used before __pre_init.
3 participants