Calling conventions for the lazily bound functions. #190

Hsiangkai · 2021-05-12T02:53:28Z

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.

kito-cheng · 2021-05-12T03:18:41Z

Thanks for come up this , it's basis of vector calling conversion and also address #66.

I would suggest using lazy binding rather than lazily bound, since lazy binding is more widely used term.
We need a new asm directive to specify symbol is specify is STO_RISCV_BIND_NOW, could you send a PR to riscv-asm-manual to add one?

riscv-elf.md

ebahapo · 2021-05-12T22:54:57Z

Somewhere, not necessarily in this document, it should be stated that dynamic functions using the V instructions should get this flag.

riscv-elf.md

Hsiangkai · 2021-05-12T22:58:02Z

Thanks for come up this , it's basis of vector calling conversion and also address #66.

I would suggest using lazy binding rather than lazily bound, since lazy binding is more widely used term.

We need a new asm directive to specify symbol is specify is STO_RISCV_BIND_NOW, could you send a PR to riscv-asm-manual to add one?

I replace "lazily bound functions" with "lazy binding".
I add a new asm directive here. Add a new directive .variant_cc for variant calling conventions. riscv-asm-manual#64

Hsiangkai · 2021-05-12T23:06:46Z

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.

jrtc27 · 2021-05-12T23:11:13Z

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.

ebahapo · 2021-05-12T23:13:18Z

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.

It's probably a good place to start.

Should functions using the V instructions get this flag or using the V arguments get this flag? All the vector registers are caller-saved.

Just those using V arguments, yes?

jrtc27 · 2021-05-12T23:14:31Z

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.

Hsiangkai · 2021-05-13T00:48:10Z

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.

kito-cheng · 2021-05-13T03:30:31Z

Should functions using the V instructions get this flag or using the V arguments get this flag? All the vector registers are caller-saved.

Just those using V arguments, yes?

It only for those function who have V arguments.

kito-cheng · 2021-05-16T16:17:37Z

Just note, for this changes we need to implement following component:

assembler: New syntax for setting a symbol is STO_RISCV_VARIANT_CC.
compiler: Must mark a symbol is STO_RISCV_VARIANT_CC if not using standard calling conversion or using parameter not passed in GPR/FPR.
linker: generate DT_RISCV_VARIANT_CC for dynamic section if any symbol with STO_RISCV_VARIANT_CC.
dynamic linker: glibc and all other dynamic linker must recognize this new flag and handle this.

kito-cheng

LGTM, but I would suggest we should have assembler, linker and dynamic linker PoC before merge.

palmer-dabbelt

Generally LGTM, just some wording stuff.

palmer-dabbelt · 2021-05-27T15:49:18Z

riscv-elf.md

+## <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


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.

palmer-dabbelt · 2021-05-27T15:52:54Z

riscv-elf.md

+  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


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.

My thinking for commentary while we aren't using asciidoc is something like:

NOTE: Bla bla bla

jrtc27 · 2021-06-01T06:15:03Z

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?

jrtc27 · 2021-06-01T06:18:21Z

Ah, never mind, it is checked in glibc, just obfuscated by being DT_AARCH64 (VARIANT_PCS) (grr why...), as a quick way to check whether it needs to scan the symbol table for variant PCS functions.

jrtc27 · 2021-06-01T06:21:16Z

riscv-elf.md

+  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.


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:.

jrtc27 · 2021-06-01T06:21:33Z

riscv-elf.md

+  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

frasercrmck · 2021-06-01T10:18:59Z

riscv-elf.md

+  tag in the Dynamic Section of the object.
+
+> NOTE:
+  Vector registers have variant size. It depends on the hardware implementation.


"variant size" doesn't read well to me. If it's a standard term then ignore this suggestion, but something like:

Suggested change

Vector registers have variant size. It depends on the hardware implementation.

Vector registers have a variable size depending on the hardware implementation.

frasercrmck · 2021-06-01T10:20:48Z

riscv-elf.md

+
+> 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


This sentence is missing something. Either of these read better to me:

Suggested change

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

Suggested change

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

jrtc27 · 2021-07-09T16:25:36Z

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.

Hsiangkai · 2021-07-12T09:01:37Z

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.

MaskRay

LGTM from my portion of LLVM viewpoint :)

The subject needs an update. STO_AARCH64_VARIANT_PCS should be in the subject.

MaskRay · 2021-07-30T22:25:08Z

riscv-elf.md

@@ -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


The wording needs to be careful but the current one is fine.

https://groups.google.com/g/generic-abi/c/ytficX3WEWg/m/TfPyG4MFCQAJ

jrtc27 · 2021-08-05T16:34:27Z

(Rebased to resolve conflicts)

jrtc27 · 2021-08-05T16:36:45Z

LGTM from my portion of LLVM viewpoint :)

The subject needs an update. STO_AARCH64_VARIANT_PCS should be in the subject.

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.

jrtc27 · 2021-08-05T16:43:47Z

Just note, for this changes we need to implement following component:

assembler: New syntax for setting a symbol is STO_RISCV_VARIANT_CC.

compiler: Must mark a symbol is STO_RISCV_VARIANT_CC if not using standard calling conversion or using parameter not passed in GPR/FPR.

linker: generate DT_RISCV_VARIANT_CC for dynamic section if any symbol with STO_RISCV_VARIANT_CC.

dynamic linker: glibc and all other dynamic linker must recognize this new flag and handle this.

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.

kito-cheng · 2021-08-09T02:34:51Z

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.

jim-wilson · 2021-08-10T20:54:43Z

The glibc patch is here
https://sourceware.org/pipermail/libc-alpha/2021-August/129931.html
I don't see the binutils patch in the obvious places I looked.

jim-wilson · 2021-08-11T18:16:40Z

The binutils patch is now
https://sourceware.org/pipermail/binutils/2021-August/117643.html

MaskRay

https://reviews.llvm.org/D107949 will add the LLVM binary utility support.

riscv-elf.md

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 <[email protected]>

jrtc27 · 2021-09-13T15:34:51Z

Rebased, converted to asciidoc and fixed dynamic tag wording to specify that it must be present if any such symbols are

jrtc27 · 2021-09-27T13:08:45Z

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

Hsiangkai mentioned this pull request May 12, 2021

On RISC-V, lazily bound functions must follow the ABI #66

Closed

kito-cheng requested review from jim-wilson, jrtc27 and palmer-dabbelt May 12, 2021 03:02

ebahapo reviewed May 12, 2021

View reviewed changes

riscv-elf.md Outdated Show resolved Hide resolved

ebahapo reviewed May 12, 2021

View reviewed changes

riscv-elf.md Outdated Show resolved Hide resolved

jim-wilson approved these changes May 19, 2021

View reviewed changes

kito-cheng approved these changes May 27, 2021

View reviewed changes

palmer-dabbelt reviewed May 27, 2021

View reviewed changes

jrtc27 reviewed Jun 1, 2021

View reviewed changes

kito-cheng reviewed Jun 1, 2021

View reviewed changes

riscv-elf.md Outdated Show resolved Hide resolved

frasercrmck reviewed Jun 1, 2021

View reviewed changes

jrtc27 added this to the First Release DoD milestone Jul 9, 2021

jrtc27 force-pushed the lazy-binding branch from 366e382 to a0866cf Compare July 20, 2021 05:04

MaskRay approved these changes Jul 30, 2021

View reviewed changes

jrtc27 force-pushed the lazy-binding branch from f40cac8 to 1f1f767 Compare August 5, 2021 16:34

jrtc27 force-pushed the lazy-binding branch from 1f1f767 to 9eadfe6 Compare August 5, 2021 16:35

kito-cheng mentioned this pull request Aug 12, 2021

Add a new directive .variant_cc for variant calling conventions. riscv-non-isa/riscv-asm-manual#64

Merged

MaskRay approved these changes Aug 12, 2021

View reviewed changes

riscv-elf.md Outdated Show resolved Hide resolved

jrtc27 force-pushed the lazy-binding branch from 9eadfe6 to 7f3912c Compare September 13, 2021 15:34

jrtc27 merged commit 1904684 into riscv-non-isa:master Sep 27, 2021

jrtc27 mentioned this pull request Sep 27, 2021

Add minimal description for vector register convention #218

Merged

sorear mentioned this pull request Jun 20, 2023

Proposal for Vector Calling Convention #389

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Calling conventions for the lazily bound functions. #190

Calling conventions for the lazily bound functions. #190

Hsiangkai commented May 12, 2021

kito-cheng commented May 12, 2021

ebahapo commented May 12, 2021

Hsiangkai commented May 12, 2021

Hsiangkai commented May 12, 2021

jrtc27 commented May 12, 2021

ebahapo commented May 12, 2021

jrtc27 commented May 12, 2021

Hsiangkai commented May 13, 2021

kito-cheng commented May 13, 2021

kito-cheng commented May 16, 2021

kito-cheng left a comment

palmer-dabbelt left a comment

palmer-dabbelt May 27, 2021

palmer-dabbelt May 27, 2021

jrtc27 May 27, 2021

jrtc27 commented Jun 1, 2021

jrtc27 commented Jun 1, 2021

jrtc27 Jun 1, 2021

jrtc27 Jun 1, 2021

frasercrmck Jun 1, 2021

frasercrmck Jun 1, 2021

jrtc27 commented Jul 9, 2021

Hsiangkai commented Jul 12, 2021

MaskRay left a comment

MaskRay Jul 30, 2021

jrtc27 commented Aug 5, 2021

jrtc27 commented Aug 5, 2021

jrtc27 commented Aug 5, 2021

kito-cheng commented Aug 9, 2021

jim-wilson commented Aug 10, 2021

jim-wilson commented Aug 11, 2021

MaskRay left a comment

jrtc27 commented Sep 13, 2021

jrtc27 commented Sep 27, 2021

		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.

	Vector registers have variant size. It depends on the hardware implementation.
	Vector registers have a variable size depending on the hardware implementation.

	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

	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

Calling conventions for the lazily bound functions. #190

Calling conventions for the lazily bound functions. #190

Conversation

Hsiangkai commented May 12, 2021

kito-cheng commented May 12, 2021

ebahapo commented May 12, 2021

Hsiangkai commented May 12, 2021

Hsiangkai commented May 12, 2021

jrtc27 commented May 12, 2021

ebahapo commented May 12, 2021

jrtc27 commented May 12, 2021

Hsiangkai commented May 13, 2021

kito-cheng commented May 13, 2021

kito-cheng commented May 16, 2021

kito-cheng left a comment

Choose a reason for hiding this comment

palmer-dabbelt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrtc27 commented Jun 1, 2021

jrtc27 commented Jun 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrtc27 commented Jul 9, 2021

Hsiangkai commented Jul 12, 2021

MaskRay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrtc27 commented Aug 5, 2021

jrtc27 commented Aug 5, 2021

jrtc27 commented Aug 5, 2021

kito-cheng commented Aug 9, 2021

jim-wilson commented Aug 10, 2021

jim-wilson commented Aug 11, 2021

MaskRay left a comment

Choose a reason for hiding this comment

jrtc27 commented Sep 13, 2021

jrtc27 commented Sep 27, 2021