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

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

Closed
palmer-dabbelt opened this issue Dec 20, 2017 · 13 comments · Fixed by #190
Closed

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

palmer-dabbelt opened this issue Dec 20, 2017 · 13 comments · Fixed by #190
Milestone

Comments

@palmer-dabbelt
Copy link
Contributor

See this x86 ABI bug for more details: https://sourceware.org/bugzilla/show_bug.cgi?id=21265

I vote we solve this on RISC-V by just requiring that lazily bound functions follow the standard ABI. This lets us avoid saving the rest of the X registers, and assuming we can do the fixup without any extensions (which seems reasonable, and is what we currently do despite it not actually being enforced) we won't need to save F or V state here.

I don't know where this should go in the manual, @asb?

@aswaterman
Copy link
Contributor

I'd phrase it as "Functions exported from shared libraries must follow the standard calling convention. Alternate calling conventions are permitted only for functions with internal linkage."

@palmer-dabbelt
Copy link
Contributor Author

The glibc people were strict about restricting this to lazily bound functions, as those aren't the default for shared libraries on some systems. IDK if those actually matter, though, as they're probably pretty esoteric.

@aswaterman
Copy link
Contributor

Fair enough

@jim-wilson
Copy link
Collaborator

jim-wilson commented Dec 20, 2017 via email

@palmer-dabbelt
Copy link
Contributor Author

-z now is good enough of a reason to make this distinction in the ABI document.

@asb
Copy link
Collaborator

asb commented Jan 12, 2018

It had never occurred to me that people might do something other than use the standard calling convention for lazily bound functions.

I'm not entirely sure where it should go. A note under "procedure calling convention" perhaps? Phrasing it as "must follow the standard calling convention" might be confusing - with RISC-V we of course have a range of standard calling conventions, and the requirement is really to conform to the specific standard calling convention used on that system.

@jrtc27
Copy link
Collaborator

jrtc27 commented Mar 16, 2021

I'm not convinced it really makes sense to document this? We define the calling convention for each ABI, and compilers have to implement that. If they want to perform optimisations that result in a different calling convention then they can do whatever they like provided it is a sound optimisation, just like anything else. Calling a lazily-bound function with a non-standard ABI is not a sound optimisation if it uses any of the call-clobbered non-argument registers. If we end up extending the psABI to use veneers for anything like AAPCS32/64 then we will need to be careful there to ensure we don't end up with veneers clobbering non-standard argument registers, but we do not have such a thing today.

@palmer-dabbelt
Copy link
Contributor Author

IMO this is more of a relocation issue than a calling convention issue: regardless of whether a symbol is a function under the standard calling convention, the act of resolving a lazy call imposes the standard calling on that symbol. This is important for two reasons:

  • We can speed up the dynamic loader by assuming that all symbol lookups are for functions of the standard calling convention. This is going to be the case in practice, but there's nothing in the document to prevent users from using the PLT call for an arbitrary jump (just like they could use the non-PLT call for an arbitrary jump).
  • The implementation currently supports a subset of the possible non-standard calling conventions over lazily bound symbol lookup, chiefly that we avoid touching any floating-point registers. This is similar to the x86 issue, where users started assuming that these implementation details were part of glibc's ABI and were meant to allow for resolving symbols that were functions with non-standard calling conventions.

The worry was that this may come up in the context of the V extension, where users may want non-standard calling conventions (maybe to pass vector arguments in registers, wouldn't really be describable in a C calling convention) and dynamic loader developers may want to rely on the standard calling convention (do to a vector memcpy without saving the V state, for example). As it's likely that users end up with V binaries way before glibc contains any V instructions, the glibc change would break user code and without something concrete to point at in a spec we'd have to argue.

@kito-cheng
Copy link
Collaborator

kito-cheng commented Mar 16, 2021

The worry was that this may come up in the context of the V extension, where users may want non-standard calling conventions (maybe to pass vector arguments in registers, wouldn't really be describable in a C calling convention) and dynamic loader developers may want to rely on the standard calling convention (do to a vector memcpy without saving the V state, for example). As it's likely that users end up with V binaries way before glibc contains any V instructions, the glibc change would break user code and without something concrete to point at in a spec we'd have to argue.

In fact, we (SiFive) have idea for alternative (or you can say it's extension) calling convention for vector function, the motivation is normal function call[1] is treat all register are caller-save which means all vector registers are not preserved across calls, and we want another calling convention like SVE's AAVPCS(aarch64_vector_pcs), which will not all caller-save for vector registers.

So I guess we might have that issue if we introduce another vector CC, Jim also point me ARM folks has hit this issue before, and seems that require bunch of fix for that[2], we might take more time for that.

@Hsiangkai

[1] #171
[2] https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00185.html

@jim-wilson
Copy link
Collaborator

This was inspired by an x86 toolchain bug report. Looking at that bug report now, I see that they originally refused to support non-standard calling sequence in the resolver, and then 6 months later a patch was checked in to make this work by using the fxsave or xsave or xsavec instruction as appropriate to save/restore all registers in the resolver routine. Testing showed that there was no appreciable performance difference from the change, as this only happens once for each function called via the plt.

The issue with V is similar to the x86 case. If we want an optimized C library that is allowed to use V instructions when it makes sense, and if we want to allow optimized user code that passes arguments in V registers when it makes sense, then the resolver needs to save/restore some (or all) of the V registers. Though the variable size of V registers makes this more complicated. It probably makes more sense to wait until we have a use case for the feature so we can better define what we need to make it work.

The aarch64 solution Kito pointed at marks vector routines as having a variant ABI in the symbol table, and then only those routines need the resolver to save the vector registers. We probably should implement a similar solution.

@palmer-dabbelt
Copy link
Contributor Author

It seems very short sighted to look at the x86 ABI break and the arm64 ABI break and declare that we shouldn't write anything down. If anything we should at least codify what the X and F register clobbers are on lazily bound PLT calls, as those are already de-facto ABI since glibc has the weakest correct implementation for both. If we were to support non-standard X or F calling conventions we'd need some ABI extension (the arm64 decorated symbols seem reasonable) -- I doubt that's worth it, though, as the non-standard calling conventions for X and F registers probably won't be worth supporting on systems with dynamic linking.

If you don't want to write anything down for the V ABIs then that's fine, I'll just re-open this when the bugs show up ;)

@kito-cheng
Copy link
Collaborator

I guess we should document down that those discussion, especially remind us that we should very carefully to handle new calling conversion with new register set like V.

So keep this opened until we document that :) @jrtc27 or me will written down that into the policy of add new calling convention (in near future I hope).

@Hsiangkai
Copy link
Contributor

I have written a draft about the lazily bound function in #190.
The encoding and name of the added flag and tag are picked arbitrarily. I have no strong opinion on it.

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 a pull request may close this issue.

7 participants