[PATCH] D40023: [RISCV] Implement ABI lowering

[Alex Bradbury via Phabricator via cfe-commits]

asb added inline comments.


================
Comment at: lib/CodeGen/TargetInfo.cpp:8858
+  else
+    NeededArgGPRs = 1;
+
----------------
efriedma wrote:
> It looks like the ABI says there's a special rule for varargs here?
You're right, I neglected vararg calls. Now addressed and test cases added. The difference is only observable on RV64 I believe (as we only use the GPR tracking to decide whether an argument should be extended or not, and the argument promotion rules mean you'll never see signext/zeroext on varargs in RV32).


================
Comment at: lib/CodeGen/TargetInfo.cpp:8914
+Address RISCVABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
+                                QualType Ty) const {
+  CharUnits SlotSize = CharUnits::fromQuantity(XLen / 8);
----------------
efriedma wrote:
> 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.)
I've started an attempt to better document things https://github.com/riscv/riscv-elf-psabi-doc/pull/60


================
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
+
----------------
efriedma wrote:
> 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.
I've corrected the comment to say "if passed on the stack" and submitted a PR to clarify the ABI doc https://github.com/riscv/riscv-elf-psabi-doc/pull/59


https://reviews.llvm.org/D40023