[PATCH] D40023: [RISCV] Implement ABI lowering

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added inline comments.


================
Comment at: lib/CodeGen/TargetInfo.cpp:8790
+private:
+  unsigned XLen;
+  static const int NumArgGPRs = 8;
----------------
rjmccall wrote:
> Is "XLen" a term that anyone working with RISCV would recognize?  Because even if it is, it feels like something you should document here — just "standard pointer size in bits" would be fine.
+1


================
Comment at: lib/CodeGen/TargetInfo.cpp:8821
+  // rewrite the argument list and pass indirectly on RV32.
+  bool IsRetIndirect = FI.getReturnInfo().getKind() == ABIArgInfo::Indirect ||
+                       getContext().getTypeSize(RetTy) > 2 * XLen;
----------------
I think it's better to use parenthesis here to aid readability and avoid ambiguity?


================
Comment at: lib/CodeGen/TargetInfo.cpp:8824
+
+  // We must track the number of GPRs used in order to conform to the RISC-V
+  // ABI, as integer scalars passed in registers should have signext/zeroext
----------------
I observed that you use RISCV and RISC-V interchangeably in comments. This is not a problem,  per se but can make this uniform if we want to be *really* particular about this :)


https://reviews.llvm.org/D40023