Calling conventions for the lazily bound functions. #190
Conversation
|
Thanks for come up this , it's basis of vector calling conversion and also address #66.
|
|
Somewhere, not necessarily in this document, it should be stated that dynamic functions using the V instructions should get this flag. |
|
I think I should update #171. Should functions using the V instructions get this flag or using the V arguments get this flag? All the vector registers are caller-saved. |
|
To me this conflates implementation ("don't lazy bind") with higher-level justification ("this follows a non-standard calling convention"). We should be labelling symbols as using a non-standard calling convention and letting implementations make that imply a lack of lazy binding if needed, not being so fixed in the view of the world and calling it "bind now". Incidentally, AArch64 takes a similar approach and labels them as STO_AARCH64_VARIANT_PCS, not STO_AARCH64_BIND_NOW. |
It's probably a good place to start.
Just those using V arguments, yes? |
|
I do also feel we'll want to separate out a vector calling convention so that those can still be lazily bound with a suitable resolver, though that can always come later (and it's still be marked STO_RISCV_VARIANT_CC or whatever, just also STO_RISCV_VECTOR_CALL for implementations that want to handle that case). Otherwise you're boxed into a corner if you call it BIND_NOW and can never extend it without violating its original intent. |
It makes sense. STO_RISCV_VARIANT_CC is better. I will rename it. Thanks. |
It only for those function who have V arguments. |
|
Just note, for this changes we need to implement following component:
|
riscv-elf.md
Outdated
| ## <a name=dynamic-linking /> Dynamic linking | ||
|
|
||
| Lazy binding must follow the standard calling conventions. The resolver in the | ||
| dynamic linker will save/restore `a0-a7` for integer calling convention and |
There was a problem hiding this comment.
I'd cut out the bit about save/restore, and just say that binding must follow the standard calling conventions. IMO it's really up to the C library to determine what it needs to save/restore, and while we do currently just save/restore both X and F args in glibc that's just because we haven't figured out a reliable way to exclude the F extensions from the dynamic loader. These are all build system concerns in glibc and other C libraries may not have those concerns so I don't want to force them to spin up the FPU if they don't need to.
riscv-elf.md
Outdated
| the eager binding semantic of the function call with the non-standard calling | ||
| convention or any other special purpose to avoid going through the resolver. | ||
| For example, vector registers have variant size. It may be from 128 bits to | ||
| 4096 bits or larger. It depends on the hardware implementation. To |
There was a problem hiding this comment.
Much as above, I'd prefer to keep some of this extra wording out of the ABI. We don't really have a way to split out the normative text from comments, and while having these examples is nice to explain why we're making certain decisions it could be construed as an actual ABI requirement. From "128 bits to 4096 bits or larger" is pretty inclusive, but it's just unnecessarry.
Maybe the right answer here is to add some specific commentary decorations? We've got a handful of these in the ABI doc right now so it'd be a nice cleanup.
There was a problem hiding this comment.
My thinking for commentary while we aren't using asciidoc is something like:
NOTE: Bla bla bla
|
I don't understand why the dynamic tag needs to exist. AArch64 has DT_AARCH64_VARIANT_PCS, but as far as I can tell nothing actually consults it other than readelf-like things, according to code search.debian.net at least. Does anyone have any insight? |
|
Ah, never mind, it is checked in glibc, just obfuscated by being |
riscv-elf.md
Outdated
| If `STO_RISCV_VARIANT_CC` is set, the dynamic linker will always resolve the | ||
| symbol during program loading. The resolved symbol address will be filled | ||
| into GOT entry regardless if `LD_BIND_NOW` is set or not under dynamic | ||
| linking. | ||
|
|
||
| Static linkers must set the flag for the symbol following the eager binding | ||
| semantic in the dynamic symbol table and add a `DT_RISCV_VARIANT_CC` dynamic | ||
| tag in the Dynamic Section of the object. |
There was a problem hiding this comment.
This is overly prescriptive. If a dynamic linker wants to save and restore every single register, it can do so and then continue to lazily bind everything. It's likely a bad idea, but it's a correct implementation. This should only state what the requirements are; commentary about this likely implying eager binding belongs in a > NOTE:.
riscv-elf.md
Outdated
| the eager binding semantic of the function call with the non-standard calling | ||
| convention or any other special purpose to avoid going through the resolver. |
riscv-elf.md
Outdated
| tag in the Dynamic Section of the object. | ||
|
|
||
| > NOTE: | ||
| Vector registers have variant size. It depends on the hardware implementation. |
There was a problem hiding this comment.
"variant size" doesn't read well to me. If it's a standard term then ignore this suggestion, but something like:
| Vector registers have variant size. It depends on the hardware implementation. | |
| Vector registers have a variable size depending on the hardware implementation. |
riscv-elf.md
Outdated
|
|
||
| > NOTE: | ||
| Vector registers have variant size. It depends on the hardware implementation. | ||
| To save/restore all these vector arguments in the resolver may occupy a large |
There was a problem hiding this comment.
This sentence is missing something. Either of these read better to me:
| To save/restore all these vector arguments in the resolver may occupy a large | |
| To save/restore all these vector arguments, the resolver may occupy a large |
or
| To save/restore all these vector arguments in the resolver may occupy a large | |
| Saving/restoring all these vector arguments in the resolver may occupy a large |
|
This still talks about eager/lazy binding far too much in the normative text. Normative text should only specify that the function uses a calling convention that differs from the standard calling convention specified in this document. Only NOTE: text should talk about eager/lazy binding as a possible relevant thing. I'm happy to take over this PR if that would be easier to get this addressed sooner rather than later. |
Sure, please take over this PR to move it forward. Thanks. |
|
Rebased and fixed the typo. I believe this is just waiting on an LLVM ACK; @MaskRay / @frasercrmck / @ebahapo? |
riscv-elf.md
Outdated
| @@ -505,7 +521,20 @@ There are no RISC-V specific definitions relating to ELF string tables. | |||
|
|
|||
| ## <a name=symbol-table></a>Symbol Table | |||
|
|
|||
| There are no RISC-V specific definitions relating to ELF symbol tables. | |||
| * st_other: The lower 2 bits are used to specify a symbol's visibility. The | |||
| remaining 6 bits have no defined meaning in the ELF gABI. We use the highest | |||
There was a problem hiding this comment.
The wording needs to be careful but the current one is fine.
https://groups.google.com/g/generic-abi/c/ytficX3WEWg/m/TfPyG4MFCQAJ
|
(Rebased to resolve conflicts) |
Added both constant names to the commit message body. There isn't enough space in the subject to add either of them without having to make it too concise, in my opinion, unfortunately. |
I know the first two have patches for LLVM. Is there a patch for LLD or GNU ld yet for DT_RISCV_VARIANT_CC, and is there a patch for glibc yet (I know there isn't one for FreeBSD's rtld and that it's not something anyone at SiFive cares about)? If so we should merge this, but otherwise we're waiting on those to exist. |
We(SiFive) have GNU ld and glibc patch, let me ask @Hsiangkai and @Nelson1225 for send that out. |
|
The glibc patch is here |
|
The binutils patch is now |
There was a problem hiding this comment.
https://reviews.llvm.org/D107949 will add the LLVM binary utility support.
Currently, we save/restore a0-a7 and fa0-fa7 in the glibc and FreeBSD run-time linker lazy resolvers to avoid clobbering any function arguments. In order to define the vector calling convention, we need to specify the behavior of the resolver in the dynamic linker. Vector registers may have a large size and saving/restoring vector argument registers may occupy a large portion of stack space during run-time resolving. It is unreasonable to save/restore all these vector argument registers in the resolver. In this patch, we define STO_RISCV_VARIANT_CC, a special symbol attribute to mark symbols as using a calling convention that passes arguments in registers other than a0-a7 and fa0-fa7 (such as the vector calling convention), which run-time linkers can use to eagerly bind such functions. We also define DT_RISCV_VARIANT_CC, an additional dynamic tag to indicate there are symbols with the special attribute in the dynamic symbol table of the object used for PLT-based calls as a quick way for run-time linkers to skip iterating over the table at load time when no symbols need to be eagerly bound. Co-authored-by: Jessica Clarke <jrtc27@jrtc27.com>
jrtc27
force-pushed
the
lazy-binding
branch
from
9eadfe6
to
7f3912c
Compare
|
Rebased, converted to asciidoc and fixed dynamic tag wording to specify that it must be present if any such symbols are |
|
I think this has been open for feedback long enough at this point; if anyone has further feedback on the exact phrasing please file a new issue or PR, but let's finally move forward with this |
…NT_CC and DT_RISCV_VARIANT_CC STO_RISCV_VARIANT_CC marks that a symbol uses a non-standard calling convention or the vector calling convention. See riscv-non-isa/riscv-elf-psabi-doc#190 Differential Revision: https://reviews.llvm.org/D107949
This is the original discussion, riscv-non-isa/riscv-elf-psabi-doc#190 And here is the glibc part, https://sourceware.org/pipermail/libc-alpha/2021-August/129931.html For binutils part, we need to support a new direcitve: .variant_cc. The function symbol marked by .variant_cc means it need to be resolved directly without resolver for dynamic linker. We also add a new dynamic entry, STO_RISCV_VARIANT_CC, to indicate there are symbols with the special attribute in the dynamic symbol table of the object. I heard that llvm already have supported this in their mainline, so I think it's time to commit this. bfd/ * elfnn-riscv.c (riscv_elf_link_hash_table): Added variant_cc flag. It is used to check if relocations for variant CC symbols may be present. (allocate_dynrelocs): If the symbol has STO_RISCV_VARIANT_CC flag, then raise the variant_cc flag of riscv_elf_link_hash_table. (riscv_elf_size_dynamic_sections): Added dynamic entry for variant_cc. (riscv_elf_merge_symbol_attribute): New function, used to merge non-visibility st_other attributes, including STO_RISCV_VARIANT_CC. binutils/ * readelf.c (get_riscv_dynamic_type): New function. (get_dynamic_type): Called get_riscv_dynamic_type for riscv targets. (get_riscv_symbol_other): New function. (get_symbol_other): Called get_riscv_symbol_other for riscv targets. gas/ * config/tc-riscv.c (s_variant_cc): Marked symbol that it follows a variant CC convention. (riscv_elf_copy_symbol_attributes): Same as elf_copy_symbol_attributes, but without copying st_other. If a function symbol has special st_other value set via directives, then attaching an IFUNC resolver to that symbol should not override the st_other setting. (riscv_pseudo_table): Support variant_cc diretive. * config/tc-riscv.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Defined. * testsuite/gas/riscv/variant_cc-set.d: New testcase. * testsuite/gas/riscv/variant_cc-set.s: Likewise. * testsuite/gas/riscv/variant_cc.d: Likewise. * testsuite/gas/riscv/variant_cc.s: Likewise. include/ * elf/riscv.h (DT_RISCV_VARIANT_CC): Defined to (DT_LOPROC + 1). (STO_RISCV_VARIANT_CC): Defined to 0x80. ld/ * testsuite/ld-riscv-elf/variant_cc-1.s: New testcase. * testsuite/ld-riscv-elf/variant_cc-2.s: Likewise. * testsuite/ld-riscv-elf/variant_cc-now.d: Likewise. * testsuite/ld-riscv-elf/variant_cc-r.d: Likewise. * testsuite/ld-riscv-elf/variant_cc-shared.d: Likewise. * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
…NT_CC and DT_RISCV_VARIANT_CC STO_RISCV_VARIANT_CC marks that a symbol uses a non-standard calling convention or the vector calling convention. See riscv-non-isa/riscv-elf-psabi-doc#190 Differential Revision: https://reviews.llvm.org/D107949
The patch is split from D103435. The patch supported a new directive .variant_cc that annotates function with STO_RISCV_VARIANT_CC. Symbols marked with STO_RISCV_VARIANT_CC do not use standard calling conversion or use parameter not passed in GPR/FPR. Related: riscv-non-isa/riscv-elf-psabi-doc#190 Initial authored by: HsiangKai Reviewed By: MaskRay Differential Revision: https://reviews.llvm.org/D138352
riscv-non-isa/riscv-elf-psabi-doc#190 introduced STO_RISCV_VARIANT_CC. The linker should do: * Copy the STO_RISCV_VARIANT_CC bit to .symtab: already fulfilled after 82ed93e * Produce DT_RISCV_VARIANT_CC. Done by this patch Differential Revision: https://reviews.llvm.org/D107951
riscv-non-isa/riscv-elf-psabi-doc#190 introduced STO_RISCV_VARIANT_CC. The linker should: * Copy the STO_RISCV_VARIANT_CC bit to .symtab/.dynsym: already fulfilled after 82ed93e * Produce DT_RISCV_VARIANT_CC if at least one R_RISCV_JUMP_SLOT relocation references a symbol with the STO_RISCV_VARIANT_CC bit. Done by this patch. Reviewed By: kito-cheng Differential Revision: https://reviews.llvm.org/D107951
The patch is splitted from D103435. The patch emits .variant_cc [0] for those function calls that have vector arguments or vector return values. [0]: riscv-non-isa/riscv-elf-psabi-doc#190 Initial authored by: HsiangKai Reviewed By: reames Differential Revision: https://reviews.llvm.org/D139414
The patch is splitted from D103435. The patch emits .variant_cc [0] for those function calls that have vector arguments or vector return values. [0]: riscv-non-isa/riscv-elf-psabi-doc#190 Initial authored by: HsiangKai Reviewed By: reames Differential Revision: https://reviews.llvm.org/D139414
Currently, we save/restore a0-a7 and fa0-fa7 in glibc implementation to
avoid ruining the arguments of the resolving functions. In order to
define the vector calling convention, we need to specify the behavior of
the resolver in the dynamic linker. Vector registers may have a large
size and save/restore vector argument registers may occupy a large
portion of stack space during runtime resolving. It is unreasonable
large to save/restore all these vector argument registers in the
resolver.
In this patch, we define a special symbol attribute to avoid going
through the resolver for the special purpose, i.e., non-standard calling
convention or vector calling convention. We also define an additional
dynamic tag to indicate there are symbols with the special attribute in
the dynamic symbol table of the object.