[PATCH] D40023: [RISCV] Implement ABI lowering

Eli Friedman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 14 13:08:44 PST 2017


efriedma added reviewers: efriedma, rjmccall.
efriedma added a comment.

You need more test coverage for the cases where arguments end up on the stack.  And some test coverage for varargs calls.



================
Comment at: lib/CodeGen/TargetInfo.cpp:8858
+  else
+    NeededArgGPRs = 1;
+
----------------
It looks like the ABI says there's a special rule for varargs here?


================
Comment at: lib/CodeGen/TargetInfo.cpp:8914
+Address RISCVABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
+                                QualType Ty) const {
+  CharUnits SlotSize = CharUnits::fromQuantity(XLen / 8);
----------------
The type of va_list and the behavior of va_arg should be documented in the ABI doc... but apparently it's missing at the moment?  (I mean, this is the obvious implementation from what is mentioned, but it would be nice to see something more explicit.)


================
Comment at: test/CodeGen/riscv32-abi.c:118
+// single 2*xlen-sized argument, to ensure that alignment can be maintained if
+// it spills to the stack
+
----------------
Please clarify this comment; the ABI doc doesn't say anything about aligning arguments whose alignment is 2✕XLEN, except in the case of varargs.


https://reviews.llvm.org/D40023





More information about the cfe-commits mailing list