This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add preserve_all calling convention.
ClosedPublic

Authored by danielkiss on Oct 11 2022, 1:37 AM.

Details

Summary

Clang accepts preserve_all for AArch64 while it is missing form the backed.

Fixes #58145

Diff Detail

Event Timeline

danielkiss created this revision.Oct 11 2022, 1:37 AM
danielkiss requested review of this revision.Oct 11 2022, 1:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2022, 1:37 AM

Precommit CI found a relevant issue. Also, can you add release notes to Clang for the improvement?

Also, is there a reason to support preserve_all but not preserve_most given their relationship?

Precommit CI found a relevant issue. Also, can you add release notes to Clang for the improvement?

Fix and added.

Also, is there a reason to support preserve_all but not preserve_most given their relationship?

There is support for preserve_most already, works as expected.
https://godbolt.org/z/an1T6YGss

danielkiss planned changes to this revision.Oct 11 2022, 11:33 AM

forgot to commit the test.

Also, is there a reason to support preserve_all but not preserve_most given their relationship?

There is support for preserve_most already, works as expected.
https://godbolt.org/z/an1T6YGss

Oh, well, that's all the more reason to support this work -- thank you for the fix! The Clang bits LGTM, but I'll leave it to LLVM folks to review that side of things.

efriedma added inline comments.
clang/include/clang/Basic/AttrDocs.td
5176

Please clarify which bits are actually getting preserved. It looks like it's the low 64 bits of each vector register?

danielkiss marked an inline comment as done.
danielkiss added a reviewer: rsandifo-arm.

To accommodate better to the vector ABI, AAPCS the vector parameter registers are not preserved by the callee but save the whole vector registers.

efriedma added inline comments.Oct 13 2022, 10:44 AM
clang/include/clang/Basic/AttrDocs.td
5176

Please clarify which bits are actually getting preserved. It looks like in the updated version, it's the low 128 bits of each vector register? (With SVE, that isn't necessarily the whole register.)

Matt added a subscriber: Matt.Oct 13 2022, 11:16 AM

update docs.

danielkiss marked an inline comment as done.Oct 13 2022, 1:29 PM
danielkiss added inline comments.
clang/include/clang/Basic/AttrDocs.td
5176

Sorry missed it, now it hopefully completely clear.

danielkiss marked an inline comment as done.Mar 22 2023, 8:47 AM

ping

This revision is now accepted and ready to land.Apr 27 2023, 2:55 PM

@efriedma Thanks!
Rebased.

This revision was landed with ongoing or failed builds.Apr 28 2023, 5:55 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2023, 5:55 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript