[PATCH] D73805: [AArch64] Provide Darwin variants of most calling conventions

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp:65-67
     return MF->getInfo<AArch64FunctionInfo>()->isSplitCSR() ?
            CSR_AArch64_CXX_TLS_Darwin_PE_SaveList :
            CSR_AArch64_CXX_TLS_Darwin_SaveList;
----------------
Should we be returning Darwin anything from here if we're moving all of the Darwin-specific code to a new function? Is this actually Darwin-specific?


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp:80
+AArch64RegisterInfo::getDarwinCalleeSavedRegs(const MachineFunction *MF) const {
+  assert(MF && "Invalid MachineFunction pointer.");
+  if (MF->getFunction().getCallingConv() == CallingConv::CFGuard_Check)
----------------
Also assert that the subtarget is Darwin?


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp:151
+                                                CallingConv::ID CC) const {
+  assert(!MF.getFunction().hasFnAttribute(Attribute::ShadowCallStack) &&
+         "ShadowCallStack attribute not supported on Darwin.");
----------------
Would be good to assert that the subtarget is Darwin here too?


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp:154-158
+  if (CC == CallingConv::GHC)
+    // This is academic because all GHC calls are (supposed to be) tail calls
+    return CSR_AArch64_NoRegs_RegMask;
+  if (CC == CallingConv::AnyReg)
+    return CSR_AArch64_AllRegs_RegMask;
----------------
These are the same as the non-Darwin case, so I guess that they don't have to be factored out here?


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp:259-260
+    return CSR_Darwin_AArch64_AAPCS_ThisReturn_RegMask;
+  else
+    return CSR_AArch64_AAPCS_ThisReturn_RegMask;
 }
----------------
Don't need the else here.

```
if (...isTargetDarwin())
  return ...;
return CSR_AArch64_AAPCS_ThisReturn_RegMask;
```


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

https://reviews.llvm.org/D73805