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

Codify Save/Restore Frame Layout #70

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions src/toolchain-conventions.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,12 @@ have the following signatures:
- `void` `+__riscv_restore_<N>(void)+`
- `void` `+__riscv_restore_tailcall_<N>+` `(void * tail /* passed in t1 */)` (LLVM/compiler-rt only)

`<N>` is a value between 0 and 12 and corresponds to the number of
registers between `s0` and `s11` that are saved/restored. The return
address register `ra` is always included in the registers saved and restored.
`<N>` is a value between 0 and 12 and corresponds to the number of registers
between `s0` and `s11` that are saved/restored. When using the `ILP32E` ABI,
`<N>` can be at most 2, as `s3` to `s12` are not CSRs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you expand CSRs? I know it's the term used in LLVM for callee saved regs, but it might be ambiguous in the RISC-V land since Control and Status Register also called CSR.

(And caller-saved reg can also abbreviated as CSR as well in theory...)

Copy link
Author

Choose a reason for hiding this comment

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

Done.


The return address register `ra` is always included in the registers saved and
restored.

The `+__riscv_save_<N>+` functions are called from the prologue, using `t0` as
the link register to avoid clobbering `ra`. They allocate stack space for the
Expand All @@ -216,6 +219,37 @@ As of November 2021 the additional tail-call entry points are only
implemented in compiler-rt, and calls will only be generated by LLVM
when the option `-mllvm -save-restore-tailcall` is specified.

=== Save Restore Routine Stack Frame Layouts

While the implementation of the save restore routines are in the library, it is
the compiler's responsibility to emit the unwind information (CFI) for the
registers that are saved and restored by these routines, so the compilers and
the libraries must agree on the stack layouts used by these routines.
Comment on lines +224 to +227
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some random thing come to my mind:

GCC always use libgcc since it's not provide a command line option to let user use compiler-rt, but LLVM offer option to let user using either libgcc or compiler-rt?

However I guess it's may not the problem on the spec side, it's implementation side issue: either 1) align libgcc and compiler-rt or 2) LLVM will gen the CFI according the runtime library preference if libgcc and compiler-rt has different layout.

Copy link
Author

Choose a reason for hiding this comment

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

For most of the ABIs, we are already at 1 as far as I understand, and I think that is the way to go, on the basis that libgcc can either be a shared or a static link (I'm not sure if compiler-rt is the same).

I think the only place where anyone differs from the description is the ilp32e ABI, which is still unstable everywhere, so this change can be made to the libraries as long as it is noted (hence my PR to compiler-rt).

I would really prefer not to have to teach LLVM to emit two different CFI directive sets depending on the library chosen.

I recall in the investigations that ilp32e CFI information from GCC is not correct anyway, so needs re-checking? Maybe I'm wrong about that or it's been fixed since I noticed.


As the stack pointer must be correctly aligned at all times, the save restore
routines are expected to allocate more stack than they require to spill all
registers in many cases. Additional Callee-saved registers, beyond those
requested, may be saved and restored by these routines, in line with the
existing practice of saving and restoring registers in batches to match the
stack alignment (which saves on code size).

For the `LP64`, `LP64F`, `LP64D`, and `LP64Q` ABIs, the save restore routines
use `roundup(N+1, 2) * 8` bytes of stack space (where `roundup(val, multiple)`
rounds `value` up to a multiple of `multiple`).

For the `ILP32`, `ILP32F`, and `ILP32D` ABIs, the save restore routines use
`roundup(N+1, 4) * 4` bytes of stack space.

For the `ILP32E` ABI, the save restore routines use `(N+1) * 4` bytes of stack
space (which reflects the lower stack alignment used by this ABI).

In all the save restore routines, across all ABIs, `ra` is stored adjacent to
the incoming stack pointer (highest address), then the Callee-Saved registers in
register order from `s0` to `s11`. This follows the [Frame Pointer
Convention](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#frame-pointer-convention),
whether or not a frame pointer is actually being used, and contradicts with the
order used by Zcmp push/pop instructions.

== Conventions for vendor extensions

Support for custom instruction set extensions are an important part of RISC-V,
Expand Down