This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Provide Darwin variants of most calling conventions
ClosedPublic

Authored by thegameg on Jan 31 2020, 1:52 PM.

Details

Summary

With the new SVE stack layout, we now need to provide a Darwin variant for all the calling conventions based on the main AAPCS CSR save order.

This also changes APCS_SwiftError to have a Darwin and a non-Darwin version, assuming it could be used on other platforms these days, and restricts the AArch64_CXX_TLS calling convention to Darwin.

Diff Detail

Event Timeline

thegameg created this revision.Jan 31 2020, 1:52 PM
paquette added inline comments.Feb 3 2020, 9:57 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
65–67

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?

80

Also assert that the subtarget is Darwin?

151

Would be good to assert that the subtarget is Darwin here too?

154–158

These are the same as the non-Darwin case, so I guess that they don't have to be factored out here?

259–260

Don't need the else here.

if (...isTargetDarwin())
  return ...;
return CSR_AArch64_AAPCS_ThisReturn_RegMask;
paquette added inline comments.Feb 3 2020, 10:08 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
152

I feel like it's weird that this is an assert. Feels more like a fatal_error, since I think it should be surfaced to the user, but I'm not sure if this is the best place to add it.

163–168

Do we have a testcase that shows that these fail?

176

Don't need the else here.

aemerson added inline comments.Feb 3 2020, 10:10 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
65–67

We can get the Subtarget from MF right? Why not just keep the one getCalleeSavedRegs() and put the separate variants inside here?

thegameg marked 9 inline comments as done.Feb 3 2020, 2:41 PM
thegameg added inline comments.
llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
65–67

I don't think there is anything preventing other platforms for using CXX_FAST_TLS, but I could report_fatal_error and move it to the Darwin-specific call.

65–67

I chose to extract it in a separate function for readability. If you believe that it's easier to keep it in one function I can make that change.

aemerson added inline comments.Feb 3 2020, 3:07 PM
llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
65–67

You can just do the check here and if it's darwin call the (static) getDarwinCalleeSavedRegs(). That way the call sites won't need to be modified.

thegameg updated this revision to Diff 265317.May 20 2020, 12:03 PM
thegameg marked an inline comment as done.
thegameg edited the summary of this revision. (Show Details)

Address reviewer comments.

aemerson accepted this revision.May 20 2020, 3:55 PM

LGTM.

This revision is now accepted and ready to land.May 20 2020, 3:55 PM
This revision was automatically updated to reflect the committed changes.