[PATCH] D137504: [PowerPC] Implement 64-bit ELFv2 Calling Convention in TableGen (for integers/floats/vectors in registers)

[Kai Nacke via Phabricator via llvm-commits]

Kai added a comment.

Some nit comments but in general looks good to me.



================
Comment at: llvm/lib/Target/PowerPC/PPCCallingConv.cpp:1
 //===-- PPCCallingConv.h - --------------------------------------*- C++ -*-===//
 //
----------------
Sorry, that is not really part of your functionality but it confused me a lot.


================
Comment at: llvm/lib/Target/PowerPC/PPCCallingConv.cpp:16-18
+static const MCPhysReg ELF64ArgGPRs[] = {PPC::X3, PPC::X4, PPC::X5, PPC::X6,
+                                         PPC::X7, PPC::X8, PPC::X9, PPC::X10};
+const unsigned ELF64NumArgGPRs = std::size(ELF64ArgGPRs);
----------------
Since this is only used in `CC_PPC64_ELF_Shadow_GPR_Regs` it can be moved inside the function.
You could also think about using an `ArrayRef` which would eliminate the need for `ELF64NumArgGPRs`:

```
static ArrayRef<MCPhysReg> ELF64ArgGPRs = {PPC::X3, PPC::X4, PPC::X5, PPC::X6,
                                           PPC::X7, PPC::X8, PPC::X9, PPC::X10};
```



================
Comment at: llvm/lib/Target/PowerPC/PPCCallingConv.cpp:36
+
+  unsigned FirstUnallocGPR = State.getFirstUnallocated(ELF64ArgGPRs);
+  // Handle the shadowing of GPRs for floats and doubles. The number of GPRs
----------------
I wonder if the following conditions will be easier to understand if you add an quick exit here:


```
if (FirstUnallocGPR == ELF64NumArgGPRs)
  return false;
```




================
Comment at: llvm/lib/Target/PowerPC/PPCCallingConv.td:134
+  CCIfType<[f32, f64],
+           CCIfNotVarArg<CCAssignToReg<[F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12, F13]>>>,
+
----------------
Please check the length of the line here and the long lines below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137504