[aapcs64] Standardize on "caller-saved" and "callee-saved"#371
Conversation
There was a problem hiding this comment.
Typo in the title (Stadardize -> Standardize)
Will be worth getting some wider input on whether standardising on caller-saved and callee-saved is the best choice.
FWIW despite its problems, like only having one letter difference in names, I'm happy with caller/callee saved as that seems to be the most commonly used term on the most active ABIs.
What I could check:
x86_64 caller/callee saved.
RISCV caller/callee saved.
GO caller/callee saved [1]
PPC volatile/non-volatile.
Sparc volatile.
Microsoft (including ARM64EC [2]) volatile/non-volatile
I couldn't find any saying call-preserved or call-clobbered, but I did find explanatory text that said preserved across calls.
[1] https://go.googlesource.com/go/+/refs/heads/master/src/cmd/compile/abi-internal.md (actually all caller-saved)
[2] https://learn.microsoft.com/en-us/cpp/build/arm64ec-windows-abi-conventions https://learn.microsoft.com/en-us/cpp/build/x64-calling-convention?view=msvc-180
| above. Informally, the scheme allows “ZA is callee-saved” to become a | ||
| dynamic rather than a static property: if S2 `complies with the lazy saving | ||
| scheme`_, S1 can test after the call to S2 whether the call did in fact | ||
| preserve ZA. If the call did not preserve ZA, S1 is able to restore the |
There was a problem hiding this comment.
As an aside. I tried to see if I could reword this in terms of callee-save but I don't think it can be. One of the advantages of preserve rather than save, is that callee-save is really callee-responsible-for-save, not that it must save, and preserve captures that.
There was a problem hiding this comment.
I agree, I think this is also relevant for Georges comment on FFR where I am hesitant to say "caller-saved" because I think it implies the wrong thing.
| The first eight registers, v0-v7, are used to pass argument values into a subroutine and to return result values from a function. They may also be used to hold intermediate values within a routine (but, in general, only between subroutine calls). | ||
|
|
||
| Registers v8-v15 must be preserved by a callee across subroutine calls; the remaining registers (v0-v7, v16-v31) do not need to be preserved (or should be preserved by the caller). Additionally, only the bottom 64 bits of each value stored in v8-v15 need to be preserved [#aapcs64-f7]_; it is the responsibility of the caller to preserve larger values. | ||
| Registers v8-v15 are callee-saved across subroutine calls; the remaining registers (v0-v7, v16-v31) do not need to be preserved (or should be preserved by the caller). Additionally, only the bottom 64 bits of each value stored in v8-v15 need to be preserved [#aapcs64-f7]_; it is the responsibility of the caller to preserve larger values. |
There was a problem hiding this comment.
I don't really see the need to change this phrase, it's completely clear and flows better with the second part of the sentence ("must be preserved"/"do not need to be preserved").
Generally, I would suggest standardising to "caller-saved"/"calle-saved" instead of using other adjectives, or if there's ambiguity, but not replace any phrase that speaks about (not) preserving.
There was a problem hiding this comment.
I agree in principle but I think this case is re-describing what "callee-saved" means so could be confusing to not use the term we standardize on?
There was a problem hiding this comment.
I have updated this and the surrounding section to only use "callee-saved" and "caller-saved" to refer to preservation of registers.
I do agree that these sections flow worse after these changes, but I think the change to be consistent is worth it overall. But I would be grateful for your feedback.
AlfieRichardsArm
changed the title
|
Just to add. I just noticed several places use "temporary register". This seems to mean "caller-saved" but come with some extra meaning too. Thoughts on changing these uses as well? There is a separate definition which I have merged with the caller-saved one for the next patch. |
AlfieRichardsArm
force-pushed
the
main
branch
3 times, most recently
from
074ebfa to
50a4e35
Compare
|
Rebased onto #373 (the fix of typos noticed by Peter) |
smithp35
left a comment
smithp35
left a comment
There was a problem hiding this comment.
Only a few small comments from me. This is looking close.
|
I'm an outsider on this, but as author/maintainer of musl libc and someone involved in multiple platforms' ABI stuff, I believe this is a change in the opposite direction of what we should be making. "Call-saved" and "call-clobbered" are the actual semantics here: per the ABI, does making a call require treating these registers as clobbered, or can they be assumed to be preserved? "Callee-saved" vs "caller-saved" presumes the presence of valuable information and that one party of the other has an obligation to save that. This is normally not the case. A callee never has an obligation to save anything it's not touching, and a caller never has any obligation to save something whose value it will not use again. |
|
|
||
| +-------------+--------------------+--------------+--------------+ | ||
| |Extension |Parameter and Result|Call-clobbered|Call-preserved| | ||
| |Extension |Parameter and Result|Caller-saved |Callee-saved | |
There was a problem hiding this comment.
IMHO 'call-clobbered' and 'call-preserved' are much better terms. 'Caller-saved' and 'callee-saved' are only a letter apart, making them unnecessarily easy to mix up when reading quickly...
There was a problem hiding this comment.
'Caller-saved' and 'callee-saved' are only a letter apart, making them unnecessarily easy to mix up when reading quickly...
This is indeed another very good reason to prefer the call-clobbered/call-preserved terminology.
There was a problem hiding this comment.
'Caller-saved' and 'callee-saved' are only a letter apart, making them unnecessarily easy to mix up when reading quickly...
This is indeed another very good reason to prefer the call-clobbered/call-preserved terminology.
Case in point: #385
|
Out compiler engineer bias might be showing here, but I'm under the subjective impression that "callee-saved" and "caller-saved" are much more established terms in the community and the literature. FWIW, I did a quick search in ACM DL:
|
| * X2-X15, X19-X29 and SP are call-preserved. | ||
| * Z0-Z31 are call-preserved. | ||
| * P0-P15 are call-preserved. | ||
| * X2-X15, X19-X29 and SP are callee-saved. |
There was a problem hiding this comment.
Since there was some confusion about this recently, we should explicitly state these private helpers must not clobber X18 (since it is reserved and has various uses). GCC currently treats X18 as clobbered by these helpers, which interferes with some uses of X18.
This also applies to the other helpers below.
There was a problem hiding this comment.
Thanks for the review. I may do this in another patch if that's okay with you? Keep this one "no functional change intended"?
smithp35
left a comment
smithp35
left a comment
There was a problem hiding this comment.
Thanks for the updates. I'm happy all other existing comments from me have been addressed.
On the caller-saved/callee-saved vs call-preserved/call-clobbered question.
I think what we have now is an awkward mix of the two terms, which is the worse situation. Standardising on one way, even if it is not our preference is a step in the right direction.
I do agree that there are advantages to call-preserved and call-clobbered that have been mentioned already. There are also advantages to caller and callee-saved, mostly that they are the most commonly used terms in ABIs, including historically the Arm and AArch64 ABIs and the compiler literature.
I think we should go ahead with this PR. Leaving the door open for a change to call-preserved, call-clobbered if that term becomes more widely used, or there is a consensus internally and externally that it is worth changing. We'll at least be starting from a consistent base at that point.
smithp35
left a comment
smithp35
left a comment
There was a problem hiding this comment.
Thanks for the addition of the links. I've no more comments and have given it my +1 with Approve.
We should leave this open for at least a week for any further comments from other reviewers.
This addresses the unrelated typos found in review of #371 noticed by Peter Smith.
|
@AlfieRichardsArm I've merged #373 I think this needs rebasing. As there have been no more comments since last week I can merge this once it has been rebased. |
|
@smithp35 Rebased now, thanks for your help with this! |
9 participants
This addresses some issues raised in #266 by standardizing on
"callee-saved" and "caller-saved" for describing register preservation
rules in PCS.
For now this only applies to AAPCS64.