[PATCH] D130249: [LoongArch] Improve the calling convention

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n added a comment.

I've lightly checked the test cases and they seem good, thanks.

The patch seems to be implementing more of the necessary ABI handling, not "improvement", so maybe a summary of "[LoongArch] Implement more of the ABI" is more appropriate.



================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:818
+
+// Eight general-purpose registers a0-a7 used for passing pass integer
+// arguments, with a0-a1 reused to return values. Generally, the GPRs are used
----------------
duplicate word


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:826
+// Eight floating-point registers fa0-fa7 used for passing pass floating-point
+// arguments, and fa0-fa1 are also used to return values except soft float ABI.
 const MCPhysReg ArgFPR32s[] = {LoongArch::F0, LoongArch::F1, LoongArch::F2,
----------------
nit: please rephrase, aren't all FPRs treated as non-existent on soft-float ABIs? Maybe moving this to the front of the sentence is better.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:888-889
+
+  // If passing a variadic argument, or if no floating-point argument registers
+  // are available.
+  bool UseGPRForFloat = true;
----------------
nit: singular form for "no": "no FPR is"

BTW you can use GPR and FPR across the whole patch to shorten lines. People working with this code are expected to understand such abbreviations (and many more).


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:899
+  case LoongArchABI::ABI_LP64F:
+    report_fatal_error("Undefined ABI");
+    break;
----------------
"Unimplemented" would be better?


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:1011
 
-  // TODO: Handle arguments passed without register.
-  return true;
+  // When a floating-point value is passed on the stack, no bit-conversion is
+  // needed.
----------------
bit-cast?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130249