[PATCH] D101010: [SystemZ] [z/OS] Add XPLINK64 Calling Convention to SystemZ

[Kevin P. Neal via Phabricator via llvm-commits]

kpn added inline comments.


================
Comment at: llvm/lib/Target/SystemZ/SystemZCallingConv.td:193
+  // F4D and F6D, hence F4Q, are provided for ABI non-compliant code.
+  CCIfType<[f128], CCAssignToReg<[F0Q,F4Q]>>,
+
----------------
uweigand wrote:
> Space after ','.
What is this ABI non-compliant code that's getting support?

How does a long double complex get returned?


================
Comment at: llvm/lib/Target/SystemZ/SystemZCallingConv.td:211
+// space must be reserve for all the args on stack
+
+// 1           2           3              (register used, the 3 arg
----------------
uweigand wrote:
> 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?
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.)


================
Comment at: llvm/lib/Target/SystemZ/SystemZCallingConv.td:278
+  // The first 4 named  float and double arguments are passed in registers FPR0-FPR6.
+  // The rest will be passed in the user area.
+  CCIfType<[f32, f64], CCIfFixed<CCCustom<"CC_XPLINK64_Shadow_Reg">>>,
----------------
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?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101010/new/

https://reviews.llvm.org/D101010