This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Introduce Swift async calling convention.
ClosedPublic

Authored by varungandhi-apple on Jan 27 2021, 2:07 PM.

Details

Summary

This change is intended as initial setup. The plan is to add
more semantic checks later. I plan to update the documentation
as more semantic checks are added (instead of documenting the
details up front). Most of the code closely mirrors that for
the Swift calling convention. Three places are marked as
[FIXME: swiftasynccc]; those will be addressed once the
corresponding convention is introduced in LLVM.

Diff Detail

Event Timeline

varungandhi-apple published this revision for review.Jan 27 2021, 2:08 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 27 2021, 2:08 PM
clang/include/clang/Basic/AttrDocs.td
4547

I have left this as a TODO for now, so that it can be filled in later when the exact details of the convention are implemented in LLVM.

I don't get how you stack your change on top of someone else's change (the diff seems to have been created with only one commit), but this depends on https://reviews.llvm.org/D95228, which depends on https://reviews.llvm.org/D95044.

I don't get how you stack your change on top of someone else's change (the diff seems to have been created with only one commit), but this depends on https://reviews.llvm.org/D95228, which depends on https://reviews.llvm.org/D95044.

You should be able to click the "Edit Related Revisions" link in the top right, click "Parent", and search for / select "D95228".

clang/lib/AST/MicrosoftMangle.cpp
2731–2732

Whitespace needs fixing after comment was broken up by arc's linter.

llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
114

Need to add extra whitespace at end similar to __swiftcall__.

llvm/test/Demangle/ms-mangle.test
345

Broadly LGTM

clang/include/clang/Basic/AttrDocs.td
4547

We don't generally put precise CC details in this kind of documentation; the goal is to describe the calling convention at a high level. Here, you should describe it as being compatible with the low-level conventions of Swift async functions, provided it declares the right formal arguments. You should also talk about the special-case treatment of tail calls, especially the extension which adds return (void) to all language modes.

clang/lib/AST/MicrosoftMangle.cpp
2754

Please use consistent formatting with the other cases, here and elsewhere. Keeping the switch compact is better for readability than following an abstract style rule.

clang/lib/Sema/SemaDeclAttr.cpp
8110–8119

There seems to be some weirdness here in handleCallConvAttr where the codepath to attach the attribute to the decl isn't taken. Instead in my examples, it exits out at this line:

if (hasDeclarator(D)) return;

with an isa<DeclaratorDecl> check.

clang/lib/AST/MicrosoftMangle.cpp
2754

I'm pretty sure I wrote it the right way originally, but saying yes to the clang-tidy prompt by arc broke it. :sigh:

rjmccall added inline comments.Feb 2 2021, 1:20 PM
clang/lib/Sema/SemaDeclAttr.cpp
8110–8119

It doesn't need to be attached to the decl; it should be reflected in the type, the same as the other calling conventions. You would need it on the decl for something like an ObjC method.

  1. Update whitespace and add documentation for swiftasynccall.
  2. Update commit message.
varungandhi-apple marked 3 inline comments as done.Feb 4 2021, 10:03 AM
rjmccall added inline comments.Feb 4 2021, 11:55 AM
clang/include/clang/Basic/AttrDocs.td
4558

I don't think the "must be in tail position" here is true. Allow me to suggest an alternative way of saying this:

Within a swiftasynccall function, a call to a swiftasynccall function that is the immediate operand of a return statement is guaranteed to be performed as a tail call. This syntax is allowed even in C as an extension (a call to a void-returning function cannot be a return operand in standard C). If something in the calling function would semantically be performed after a guaranteed tail call, such as the non-trivial destruction of a local variable or temporary, then the program is ill-formed.

Correct documentation on tail call behavior as pointed out by John.

varungandhi-apple marked an inline comment as done.Feb 5 2021, 9:33 AM
clang/test/CodeGen/debug-info-cc.c
60–66

This needs to be changed in some way, it triggers a test failure in its current state. https://github.com/apple/llvm-project/pull/2444

Fix test case for debuginfo.

  • Remove semantic restriction of swift_async_context being only applicable to swiftasynccall.
  • Update target checks to make sure we only allow supported targets.
  • Update doc comment.

LGTM other than the TargetInfo thing.

clang/include/clang/Basic/TargetInfo.h
1424 ↗(On Diff #324389)

As I commented in the other patch, I think this would be cleaner in the long term if you just implemented it directly in the various target subclasses.

Address review feedback; remove centralized target check for CC_SwiftAsync.

This revision is now accepted and ready to land.Feb 24 2021, 9:36 PM

Permit swift_context and swift_indirect_result parameters for
swiftasynccall functions. (Earlier, these were restricted to
swiftcall-only.)

varungandhi-apple updated this revision to Diff 354073.EditedJun 23 2021, 1:59 PM

Combined D95984 into this patch since all necessary LLVM patches have landed, so having the patches separated out is not helpful. Leaving this up for a few days in case there are any final comments before landing.

This revision was landed with ongoing or failed builds.Jul 9 2021, 11:50 AM
This revision was automatically updated to reflect the committed changes.