[PATCH] D74977: [RISCV][GlobalISel] Add lowerFormalArguments for calling convention

[Nitin John Raj via Phabricator via llvm-commits]

nitinjohnraj added inline comments.


================
Comment at: llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/args.ll:257
+  ret void
+}
----------------
arsenm wrote:
> nitinjohnraj wrote:
> > arsenm wrote:
> > > Probably should have some byval, sret, and cases where the argument list runs out of registers. Same for return values / hidden sret
> > > Probably should have some byval, sret, 
> > Correct me if I'm wrong, but byval and sret both require support for aggregate types, which this patch doesn't support. 
> > 
> > > sret, and cases where the argument list runs out of registers.
> > Do you have an example of a similar test? I wasn't aware that we could run out of registers at this phase, I thought that we could have an unlimited number of virtual registers.
> This is wrong. sret and byval do not really have anything to do with aggregate types, and apply to pointers. You see them on a pointer argument, not aggregate. They would typically be used with an aggregate in memory type, but the in-memory type can be anything
> 
> 
> You're mapping to an ABI constraint. If you had 4 32-bit registers for argument passing, you would run out of argument passing registers and have one stack passed argument for (i32, i32, i32, i32, i32).
Thanks for the detailed feedback! I have added tests for sret and byval; however, this patch doesn't currently handle them correctly, I suspect. I'm referring to SelectionDAG for RISCV to figure out how to support them, but I plan to add them in an upcoming patch.

I have not added a test for when we run out of argument passing registers, as it requires the implementation of `getStackAddress`. That'll be added in an upcoming patch as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74977