[PATCH] D51479: [AArch64] Implement aarch64_vector_pcs codegen support.

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added a reviewer: MatzeB.
thegameg added inline comments.


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:1524
+  auto getCSStackSize = [&CSRegs, &SavedRegs]() {
+    unsigned Size = 0;
+    for (unsigned i = 0; CSRegs[i]; ++i)
----------------
sdesmalen wrote:
> thegameg wrote:
> > Would this also work?
> > 
> > ```
> > for (unsigned Reg : SavedRegs.set_bits())
> >   Size += TRI.getRegSizeInBits(Reg, MRI);
> > ```
> > 
> > or something like `std::accumulate`.
> I tried it and it seems to work, but one of the unit tests fails. I think this is because it exposes a bug where Reg is AArch64::NoRegister (set on line 1515 above to ensure registers are being stored in pairs for MachO's compact unwind format).
I guess we could check for `!Reg` in the loop, or fix `getRegSizeInBits`. I am not sure if returning 0 in `getRegSizeInBits` would be a good idea but I will let others comment on that.
Either way, it could be a good idea to add a comment + an assert in `getRegSizeInBits`.


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:1588
+  // Recalculate the size of the CSRs
+  CSStackSize += 8 * (SavedRegs.count() - NumSavedRegs);
+  unsigned AlignedCSStackSize = alignTo(CSStackSize, 16);
----------------
Could you add a comment explaining that the `8` here is not `Scale` because we only add GPRs?


https://reviews.llvm.org/D51479