[PATCH] D40023: [RISCV] Implement ABI lowering

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.

LGTM

Not sure if anyone's mentioned it yet, but there's a C ABI testing tool in clang/utils/ABITest/ which you'll probably want to try at some point.



================
Comment at: lib/CodeGen/TargetInfo.cpp:8913
+  }
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+}
----------------
asb wrote:
> efriedma wrote:
> > The spec says "Aggregates larger than 2✕XLEN bits are passed by reference and are replaced in the argument list with the address".  That's not byval.
> The LLVM backend lowers byval in that way. Given that at this point we have a trivial data type (plain struct or similar) which is copied-by-value by C semantics, the only difference is whether the generated IR indicates implicit copying with 'byval' or relies on the caller explicitly doing the copy. For some reason I thought 'byval' was preferred, but looking again it seems uncommon to do it this way. I've changed it to false - thanks for spotting this.
"byval" generally means the value is memcpy'ed into the argument list (so there is no pointer in the argument list). This is useful for handling C calling conventions which allow excessively large structs to be passed in the argument list, so the backend can emit a memcpy rather than expanding the operation into straight-line code.  The RISCV backend handling of byval is wrong, in the sense that it isn't consistent with what any other backend does.

This isn't relevant to the RISC-V C calling convention, but you should probably fix the backend at some point to avoid future confusion.


https://reviews.llvm.org/D40023