D127209 fixed LLVM to bring it in line with the AAPCS. The fix affects
functions where the first SVE parameter appears in the 9th or later
arguments, and the function does not return an SVE type.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
From reading the release note my understanding is that before this fix the caller of a function would store z0-z7 in situations where it did not need to. Which seems low impact unless you were doing something that read the previous stack frame (but nevertheless a difference from the AAPCS).
If that's what you were trying to get across then great!
This affects functions where the first SVE parameter appears in the 9th or later arguments
I'm reading https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#642parameter-passing-rules. Wouldn't the type of the preceding 8 arguments also be relevant?
Though I was surprised to see that "NSRN" covers floating point and SVE registers, so I'm hardly an expert here.
and the function does not return an SVE type.
This is because the function would return in z0 therefore the caller must preserve at least z0 and the ABI tells you to preserve z0-z7 to do that?
I believe you are correct, with an inkling of doubt in the back of my mind, but I can't identify a specific worse issue at present.
Wouldn't the type of the preceding 8 arguments also be relevant?
My intent was to express that the issue is present where it's the 9th or later arguments.
This is because the function would return in z0 therefore the caller must preserve at least z0 and the ABI tells you to preserve z0-z7 to do that?
Yes.
Thinking a little harder, one breaking case is where a callee-compiled-with-wrong-logic looks at its signature, concludes it does not need to preserve the regs (because the SVE type is in the 9th argument, for example), but is called by a different compiler which correctly implements the ABI, so neither side preserves registers which require preservation.
clang/docs/ReleaseNotes.rst | ||
---|---|---|
507 | Pedantically, I think it should be “the registers z0-z7 or p0-p3”. |
My intent was to express that the issue is present where it's the 9th or later arguments.
So I take from that that it does not matter if arguments 1-8 were all integers and so did not get put in SIMD/floating point registers. I think I am confusing what the ABI says (and what llvm *now does*) with the way the previous llvm behaviour. So the statement answers the question "of the functions in my project which ones might change", it's the ones where the first SVE type argument was argument 9 or greater.
Which is exactly what it should do I was just looking at it the wrong way around.
Should the release note also say that the key is the *first* SVE type parameter being argument 9, not just having an SVE type in argument 9 at all?
Commit msg:
This affects functions where the first SVE parameter appears in the 9th or later arguments, and the function does not return an SVE type.
Note:
This would cause an incorrect use of the caller-preserved z8-z23 ABI for example if the 9th argument to a function were an SVE type.
Or in other words what about arguments 1-8, can they be SVE and still hit this issue? I realise this is not supposed to be a full on super detailed report but mentioning because the "first" stood out to me.
- Cite D127209 in the release note per tschuett comment.
- Be more specific about p0-p4, remove 'by analogy', per rsandifo-arm comment.
- Clarify 9th argument or greater, per DavidSpickett comment.
Thank you everyone for your inputs, hope this addresses everything so far, happy to receive further input.
clang/docs/ReleaseNotes.rst | ||
---|---|---|
507 | I've now switched to writing out all of the registers every time, hope that's clearer. |
clang/docs/ReleaseNotes.rst | ||
---|---|---|
507 | Should be p0-p3 rather than p0-p4. LGTM with that change. |
Pedantically, I think it should be “the registers z0-z7 or p0-p3”.