This patch adds the XPLINK64 calling convention to the SystemZ backend. It specifies and implements the argument passing and return value conventions.
Details
Diff Detail
Event Timeline
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
27 | Please remove this unnecessary whitespace change here. | |
166 | Why are you changing the sequence here? I didn't think the order of CSRs in that list should make any particular difference ... | |
177 | Please revisit the comments here and below and look for wording and formatting issues. This comment would fit on one line and needs a '.' and the end. | |
188 | The last sentence doesn't seem to be correct -- according to the CSR definition above there are call-saved FPRs. | |
194 | Space after ','. | |
199 | Should we also provide more vector return registers for ABI extensions? Usually it should be fine to use all argument registers for returns too. | |
212 | If you want to have a comment here describing how the ABI works in general terms, that's of course good, but I'm not sure if the long table here is particularly useful. Maybe describing it in terms of rules would be better? Or, even better, providing a link to an offical ABI doc? | |
249 | So a difference to Linux ABI code here is that in Linux we use CCIfExtend, i.e. the extension only happens if the argument is marked using "sext" or "zext" in the IR (which gets set from clang depending on the source type, or omitted e.g. if we're passing a struct in register that cannot be "extended" as such). This code here calls CCPromoteToType unconditionally, so you'll get AnyExtend nodes in cases where there is no "sext" or "zext" marker. Is this intended? | |
306 | That last comment describes an action that doesn't appear to be done here, right? |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
166 | The CSR mask is generated in order based on this list. However, FPRs are preserved in the save area in reverse order, which is contrary to the current CSR mask. The previous commit not reversing the order of the callee-saved FPRs was an oversight. With all that said and done, if you prefer I can keep the sequence unchanged for now and create a separate patch that fixes this bug. |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
166 | If you need to save FPRs in some specific place in the save area for ABI reasons, that should be handled by code in SystemZFrameLowering.cpp (like is done for GPRs on Linux -- for FPRs there is no defined place where they need to be saved so it doesn't matter). But in any case this is an independent question, so I agree that this should best be handled in a separate patch. |
llvm/lib/Target/SystemZ/SystemZCallingConv.h | ||
---|---|---|
187 | Add . (dot) at end of sentence. |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
194 | What is this ABI non-compliant code that's getting support? How does a long double complex get returned? | |
212 | The comment seems misleading. Integer types get expanded to fill a whole GPR. So passing three chars uses 3 registers -> 24 bytes. Talking about bytes therefore doesn't seem useful. Worse, it's misleading to refer to "bytes" for GPR and not for the FPR since it implies that parms get packed or something. I see that you addressed this below, but it'd be better to not even provoke the thought in someone reading the code. I'd stick with "registers" instead of "bytes" in this sentence. One has to look closely at the table to see that FP parms can prevent a GPR from being used that would be used if there were no FP parms. So that bit could use a description in english. A link to the ABI doc would be useful, or at least the name of it. Since links go stale, the name is probably a minimum. Actually, how about copying the tables that are (were?) in the ABI documentation? Last time I looked IBM had a list of examples showing how FP values can push around GPR values, and how large numbers of integer parms can prevent FP values from being passed in FP registers despite FP registers being available. (I admit it's been years since I looked at the documentation.) | |
279 | Don't you need to hedge a bit in this comment? Some FP arguments aren't passed in registers if there are too many integer arguments. (Unless I'm wrong and the restriction only applies to 32-bit XPLINK?) |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
279 | If I am not mistaken you are referring to the requirement that a FP argument is within 15 words of the previous FP argument in the argument list? If so, then yes - XPLINK64 has no such clause. |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
279 | That's the one, yes. It must be a 31-bit only restriction then. Noted. Thanks for the clarification! |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
212 | Now that I've looked at the current version of Vendor Interfaces, the table I was thinking of is in Appendix B, from pages 924 to 931. That's ... a lot. Oh, and appendix B doesn't include the documentation that is elsewhere in the book. XPLINK calling conventions require that much documentation. So a reference to the documentation may be better than trying to duplicate it. Ulrich, how about this: Instead of having this table below, and instead of trying to put 6-7 pages of documentation in comments here, how about a comment about how there are a number of common tricky cases and they're documented in <documentation bib entry here>? |
Added comment on how complex f128 types are passed. Added explanation on how floating point parameters could affect availability of GPRs for parameter passing. Added name of document that contains full xplink spec. Fixed some grammatical issues. Added CCIfExtend clause to XPLINK argument promotion.
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
249 | The clang front-end should generate sext/zext for those types anyways so we should be using CCIfExtend as well. Thanks! |
Add all param passing vector registers as return value regs too. Fix up some punctuation.
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
212 | I've updated the comments to include a note on when the full specification of the ABI can be found. I am hesitant to include a link for reason you mentioned above - links can go stale. Given the name of the document, it shouldn't be hard to find the latest version of the LE vendor interfaces. |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
212 | I agree the current wording of the comment seems fine to me. | |
224 | One more thing I just noticed here (b.t.w. applies also to the return value convention): I don't think we can ever end up with i1, i8, or i16 here, because those types are not "legal" in the SystemZ back-end, so they will already have been converted to i32 by common code (type legalization). That is why the Linux ABI code only checks for i32 here. I think this would still hold true for the XPLINK convention -- if so, it would be preferable to remove the small types here as well. (This should have no effect on codegen, just remove an unnecessary difference between the code for the two calling conventions.) | |
235 | Just to re-confirm: this handler will pass an i128 via implicit pointer (i.e. push the i128 to the stack somewhere outside the argument list, and pass a pointer to that location as argument). Is that actually correct for XPLINK? Reading the specs in the documentation referred to above, I get the impression that XPLINK never passes arguments via implicit pointers ... | |
260 | One more comment wording nit-pick: I'd say "The first 2 named long double arguments" just like the in previous two clauses, and then remove the "Unless ..." sentence. We should just the same wording for the same concept across those different clauses. |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
194 | Non ABI-compliant code here refers to LLVM front-ends other than clang, e.g. for languages other than C/C++ or for JIT use cases. Some of those use cases require multiple return values to be passed efficiently. While you could require any such users to define their own calling convention for that, this is quite tedious (and requires back-end changes). Therefore, it most LLVM back-ends provide an extension in their "default" calling convention for multiple return values, which is never used by LLVM IR generated by clang, but is available for these other use case. However, if it is indeed true that in XPLINK, the C/C++ complex long double is already returned in four FPRs, then that would not could as "non ABI-compliant" use. But that's just a comment wording issue. | |
286 | Note that the ELF calling convention makes special provisions for "short" vector types, i.e. those defined using attribute((vector_size(8))) or shorter. They may be passed in VRs like the "normal" 16 byte vectors, but if none are available they'll not get passed in a normal 16-byte stack slot, but rather in an 8-byte stack slot. I notice that this provision is not in place with the XPLINK convention. Now I guess this is a decision for you to make, given that such types would be an extension to the ABI anyway. I just wanted to make sure that this is considered choice and not just happening by accident. |
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
212 | Fair enough! |
Remove special handling for i1, i8, i16 in calling convention. SDAG legalize types phase should already extend these, there is no need to add handling to extend these again.
llvm/lib/Target/SystemZ/SystemZCallingConv.td | ||
---|---|---|
235 | Yes, that's the intention. | |
286 | Hi Uli, we actually hadn't considered that - thank you for pointing this out. After thinking about it for a while I think we should keep the current behavior, ie. pass in a 16-byte stack slot. The reason for this is if were to copy the ELF behavior, and pass short vector types in a 8-byte stack slot if passed on the stack, we would have to do one of two things.
|
clang-tidy: warning: invalid case style for function 'CC_XPLINK64_Shadow_Reg' [readability-identifier-naming]
not useful