[PATCH] D89788: [RISCV] Add GHC calling convention

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1968
+
+  if (LocVT == MVT::i64) {
+    // Pass in STG registers: Base, Sp, Hp, R1, R2, R3, R4, R5, R6, R7, SpLim
----------------
i32 works too, as with the fast calling convention. That way you can support RV32.

(Though maybe both should be asserting if they ever see an i64 on RV32 given that's not a legal type? Or just leave it up to the general legalisation/verification stuff)


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1969
+  if (LocVT == MVT::i64) {
+    // Pass in STG registers: Base, Sp, Hp, R1, R2, R3, R4, R5, R6, R7, SpLim
+    static const MCPhysReg GPRList[] = {
----------------
Do you have an explanation somewhere for why you've chosen these X and F registers for the calling convention? It'd also be useful to give their RISC-V ABI names here in the comments as the numbered registers aren't so easy to keep track of mentally.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2001
+
+  return true; // CC didn't match.
+}
----------------
Shouldn't this be a hard error? Otherwise it'll fall back on the normal C calling convention like for Fast, no?


================
Comment at: llvm/test/CodeGen/RISCV/ghc-cc.ll:1-3
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+f,+d \
+; RUN:   --target-abi lp64d < %s \
+; RUN:   | FileCheck %s
----------------
Please make this match the other tests in here, ie:
- match the llc invocation style
- test both rv32 and rv64
- use update_llc_test_checks.py


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

https://reviews.llvm.org/D89788