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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCCallingConv.cpp:24
 
+// This function handles the shadowing of GPRs for fp and vector types.
+inline bool CC_PPC64_ELF_Shadow_GPR_Regs(unsigned &ValNo, MVT &ValVT,
----------------
Please note the section of the ABI document that describes this allocation algorithms (and then below, which part of the section talks about the specific aspect - such as allocating even GPRs for vectors, skipping odd ones, etc.).


================
Comment at: llvm/lib/Target/PowerPC/PPCCallingConv.cpp:40
+
+  // Handle the shadowing of GPRs for floats and doubles. The number of GPRs
+  // to shadow depends on the size of the argument.
----------------
Why does it not suffice for the rest of this function to be something simple like the following:
```
// For single/double precision, shadow a single GPR.
if (LocVT == MVT::f32 || LocVT == MVT::f64)
  State.AllocateReg(ELF64ArgGPRs);
else if (LocVT.is128BitVector() || (LocVT == MVT::f128)) {
  // For vector and __float128, shadow two even GPRs (skipping
  // the odd one if it is next in the allocation order).
  if ((State.AllocateReg(ELF64ArgGPRs) - PPC::X3) % 2 == 0)
    State.AllocateReg(ELF64ArgGPRs);
  State.AllocateReg(ELF64ArgGPRs);
  }
}
```


================
Comment at: llvm/lib/Target/PowerPC/PPCCallingConv.cpp:47-48
+    unsigned GPRIndToShadow = FirstUnallocGPR + 1;
+    for (unsigned RegInd = FirstUnallocGPR; RegInd < GPRIndToShadow; RegInd++)
+      State.AllocateReg(ELF64ArgGPRs[RegInd]);
+  }
----------------
Is it possible for this loop to iterate more than once?


================
Comment at: llvm/lib/Target/PowerPC/PPCCallingConv.cpp:51
+  // Handle the shadowing of GPRs for vector arguments.
+  if (LocVT.is128BitVector() || (LocVT == MVT::f128)) {
+    // Shadow one GPR if the first unallocated GPR is an even register number
----------------
Please add a comment as to what happens with `MVT::ppcf128`. Does it get broken up into two `MVT::f64`'s before here? Do we just ignore/skip it for now? Should it be an assert until it is implemented if it isn't already implemented?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137504