[PATCH] D117318: [RISCV][GlobalISel] Add lowerReturn for calling conv

[Nitin John Raj via Phabricator via llvm-commits]

nitinjohnraj requested review of this revision.
nitinjohnraj added a comment.

This has been stagnant for a while, and there were a few open comments, so could I reaffirm the approval of this patch?



================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.h:710
 
+private:
   void analyzeInputArgs(MachineFunction &MF, CCState &CCInfo,
----------------
eopXD wrote:
> Can we avoid this change?
This change is in order to use `RISCVCCAssignFn` in `RISCVOutgoingValueAssigner` above. We could maybe avoid that by inlining the type instead of changing the access specifier, but should we?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.h:837
+
+namespace RISCV {
+
----------------
eopXD wrote:
> Why do we need to promote the static functions into the header here?
In order to use them on line 111 in llvm/lib/Target/RISCV/GISel/RISCVCallLowering.cpp. Should we maybe move this outside the RISCV namespace?


================
Comment at: llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/ret.ll:144
+  ret [2 x i32] [i32 1, i32 2]
+}
----------------
arsenm wrote:
> Probably should have some vectors
If I understand correctly, this patch doesn't handle vectors. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117318