riscv-software-src / riscv-overlay Goto Github PK

Opening a discussion from feedback on the Clang/LLVM patches.

https://reviews.llvm.org/D109372
https://reviews.llvm.org/D109371

Currently the naming of the attributes matches the spec, using separate attributes __attribute__((overlaycall)) and __attribute__((overlaydata)) for functions and global data respectively. There are a couple of possible changes that are worth discussing though:

• Renaming the attributes to overlay_call and overlay_data, or alternatively maybe something like overlay_function and overlay_data.
• Merging the attributes into a single overlay attribute.

The former change is mainly concerning readability. As far as I'm aware it's generally not consistent in GCC/Clang whether attributes include an underscore, but in generally snake case is acceptable and considered easier to read. Also I'm not sure whether overlay_function would be better than overlay_call because it's more explicit that the function exists in an overlay.

The second change is something which I implemented in a previous version of the Clang patch. It is possible to use a single overlay attribute and then differentiate entirely on what kind of Decl the attribute is applied to rather than using separate attributes. This makes the interface a bit simpler, but I can think of a couple of disadvatages:

• Diagnostics would either be less precise, or would have to differentiate functions vs data anyway
• Users might mistakenly believe that the attribute is a type attribute and then misunderstand the semantics when applying it to a global function pointer.

https://github.com/riscv/riscv-overlay/blob/master/docs/overlay-hld.adoc#2727-relaxations

I think it's unsafe transformer from lui t5, 0; addi t5, t5, 3 to addi t5, zero, 3; jalr ra, 0(t6), the later one will corrupt the ra but original one won't.

Currently the relocations have the following names:

R_RISCV_OVL_HI20
R_RISCV_OVL_LO12_I
R_RISCV_OVL32
R_RISCV_OVLPLT_HI20
R_RISCV_OVLPLT_LO12_I
R_RISCV_OVLPLT32

I propose that it might be better to rename the R_RISCV_OVL_* relocations to be R_RISCV_OVLTOK_*. My motive for this is that by using 'OVLTOK' it's a bit clearer that the value being encoded is a token value rather than an address. It also draws a clearer distinction from the OVLPLT relocations which really are addresses to a resident thunk.

There is an example of disassembly code in the high level document, but it's also necessary to define the assembly syntax.

Currently this is the syntax used in the LLVM patch (https://reviews.llvm.org/D109371)

Materializing an overlay token for a symbol:

lui a0, %ovl_hi(foo)
addi t5, a0, %ovl_lo(foo)
.word foo@ovl

Materializing the address of a overlay plt entry:

lui a0, %ovlplt_hi(foo)
addi t5, a0, %ovlplt_lo(foo)
.word foo@ovlplt

Making a call to the overlay engine:

jalr t6

In line with #29 I also propose that we update the %ovl_hi, %ovl_lo and @ovl syntax to use ovltok instead of just ovl too.

I saw there is relocation for 32-bit overlay plt entry address (R_RISCV_OVLPLT32), does it means overlay only support rv32?

Could you explain what is the behavior including compiler code gen for overlaydata? and what if user take of address of a overlay data?