cc @JDevlieghere

Combined changes into D95561.

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.

1. Flip polarity of enable->disable.
2. Turn on checking by default (instead of being off by default).
3. Remove NDEBUG-dependence.
4. Add more explicit wording around mandatory musttail.
5. Update test case to check both ways and remove "REQUIRES: asserts".

Removed stray tee command used for debugging.

1. Added test case.
2. Updated LangRef to note mandatory musttail.
3. Added FIXME to code to remove the flag at a point in the future.
4. Fixed a typo "swiftail" -> "swifttail" in the error message.

This doesn't look like a particularly expensive verifier check (although maybe I've missed something). What's the motivation for hiding it behind EnableSwiftTailCCMustTailCheck?

I used Assert for two reasons: (1) the surrounding code uses Assert, so it seemed like the right thing to use and (2) in case there's a failure, it will display the instruction missing the musttail.

clang-format

1. Remove '::' when calling static function.
2. Fix bug in function merging around missing musttail.
3. Add off-by-default check in verifier for swifttailcc->swifttailcc tail calls that are not marked musttail.

Fix codegen + add tests for methods and function pointers.

Use musttail call instead of tail call. (This patch needs to be marked as dependent on another patch which implements proper musttail support, but that is not available yet.)

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

Add null check for callee; instead of using getFunctionType() directly.

Added checks for C++.

Generate tail call when doing codegen for return statement.

Address review feedback; remove centralized target check for CC_SwiftAsync.

I've updated the earlier patch https://reviews.llvm.org/D95561 to include this change, so that we don't have an in-between state where we erroneously claim that we support a bunch of targets which we don't actually support.

The TargetInfo classes are already target-architecture-specific, so it's somewhat strange for them all to funnel to a single function that then immediately switches on the target architecture.

• 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.

Tweak test case diff to adjust for change in previous commit.

Fix test case for debuginfo.

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

1. Update whitespace and add documentation for swiftasynccall.
2. Update commit message.

I am planning to add more tests, figured it is better to put up the patch sooner rather than later.

Could you add a short entry on swifttailcc to LangRef.rst?

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've fixed this by including an additional space in the string instead of using a method like outputSpaceIfNecessary; (1) that method doesn't work as it is and I think changing it for this purpose is not appropriate and (2) we probably want the space there unconditionally anyways.

Add extra space for better readability, as suggested by @compnerd.

Closing this in favor of a Swift-side fix: https://github.com/apple/swift/pull/35488.

The way you'd test it on the Clang side would be to just write a test case that passes such an aggregate and tests that it's passed as an i8.

I don't know how to test this on the LLVM-side (is there a way to do it?). Here's the Swift side test which trips over right now (but is fixed with this patch) by attempting a zext from an i8 to an i1 in Swift's NativeConventionSchema::mapIntoNative.

Thanks!

Going to land this as-is, will make suggested tweaks in a follow-up change.

Appease clang-tidy.

(I'm going to wait till at least Monday to land this, in case anyone has objections or would like to suggest changes to the implementation or tests.)

1. Allow many more operations in constexpr.
2. Update test to reflect use in constexpr instead of relying on implementation details.

This seems to only allow empty optionals to be constexpr. Should the non-default constructor also be constexpr?

Some of the changes probably don't make perfect sense in this revision because I initially started out with the whole change being one revision and then split things into commits, while trying to minimize the churn between revisions. Please also take a look at the follow-up revision D82811 and let me know if that's acceptable (less churn, less self-contained, things make sense when you look at the two together) or if you'd prefer that I make this revision more self-contained (that would create more churn in D82811).

Address review comments.

@yln could you please land the patch? I do not have commit access.

Thanks!

(Could you land the patch if it looks good? I don't have commit access.)

Updated commit message with reviewer information and revision link.

Oh, and sorry I forgot to update the "author" field to appropriately credit you when I committed these

Thanks! It seems a bit weird that this revision now only shows the one commit under Revision contents even though the 'Commits' above shows the six commits. 🤷

@dblaikie could you land the patch for me? I do not have bot permissions/commit privileges.

Split up single commit into several commits (one commit per set type).

I've added some unit tests. Are these adequate/do you have any other suggestions on what I should test?

Added comments and test cases addressing fhahn's review comment.

I don't understand the clang-tidy/clang-format complaints. I've formatted the code based on how the surrounding code is formatted.

Prior art:

Thanks for the review, it's a big patch. 😅 I'm a bit busy at the moment, I will respond to the other comments later this week or sometime next week.

Rebase after changes in D82808.

Rebase after changes in D82808.

Rebase after changes in D82808.

Addressed review comment: replace shows -> show.

Please review D82791: "[lit] Improve lit's output with default settings and --verbose." before reviewing this revision.