diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0e9fcaa5fac6a..41c412730b033 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -136,6 +136,12 @@ Removed Compiler Flags Attribute Changes in Clang -------------------------- +- Introduced a new attribute ``[[clang::coro_await_suspend_destroy]]``. When + applied to a coroutine awaiter class, it causes suspensions into this awaiter + to use a new `await_suspend_destroy(Promise&)` method instead of the standard + `await_suspend(std::coroutine_handle<...>)`. The coroutine is then destroyed. + This improves code speed & size for "short-circuiting" coroutines. + Improvements to Clang's diagnostics ----------------------------------- - Added a separate diagnostic group ``-Wfunction-effect-redeclarations``, for the more pedantic diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 30efb9f39e4f4..341848be00e7d 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1352,6 +1352,14 @@ def CoroAwaitElidableArgument : InheritableAttr { let SimpleHandler = 1; } +def CoroAwaitSuspendDestroy: InheritableAttr { + let Spellings = [Clang<"coro_await_suspend_destroy">]; + let Subjects = SubjectList<[CXXRecord]>; + let LangOpts = [CPlusPlus]; + let Documentation = [CoroAwaitSuspendDestroyDoc]; + let SimpleHandler = 1; +} + // OSObject-based attributes. def OSConsumed : InheritableParamAttr { let Spellings = [Clang<"os_consumed">]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 2b095ab975202..0313d9b2cede4 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -9270,6 +9270,110 @@ Example: }]; } +def CoroAwaitSuspendDestroyDoc : Documentation { + let Category = DocCatDecl; + let Content = [{ + +The ``[[clang::coro_await_suspend_destroy]]`` attribute may be applied to a C++ +coroutine awaiter type. When this attribute is present, the awaiter must +implement ``void await_suspend_destroy(Promise&)``. If ``await_ready()`` +returns ``false`` at a suspension point, ``await_suspend_destroy`` will be +called directly. The coroutine being suspended will then be immediately +destroyed. + +The new behavior is equivalent to this standard code: + +.. code-block:: c++ + + void await_suspend_destroy(YourPromise&) { ... } + void await_suspend(auto handle) { + await_suspend_destroy(handle.promise()); + handle.destroy(); + } + +This enables `await_suspend_destroy()` usage in portable awaiters — just add a +stub ``await_suspend()`` as above. Without ``coro_await_suspend_destroy`` +support, the awaiter will behave nearly identically, with the only difference +being heap allocation instead of stack allocation for the coroutine frame. + +This attribute helps optimize short-circuiting coroutines. + +A short-circuiting coroutine is one where every ``co_await`` or ``co_yield`` +either immediately produces a value, or exits the coroutine. In other words, +they use coroutine syntax to concisely branch out of a synchronous function. +Here are close analogs in other languages: + +- Rust has ``Result`` and a ``?`` operator to unpack it, while + ``folly::result`` is a C++ short-circuiting coroutine, with ``co_await`` + acting just like ``?``. + +- Haskell has ``Maybe`` & ``Error`` monads. A short-circuiting ``co_await`` + loosely corresponds to the monadic ``>>=``, whereas a short-circuiting + ``std::optional`` coro would be an exact analog of ``Maybe``. + +The C++ implementation relies on short-circuiting awaiters. These either +resume synchronously, or immediately destroy the awaiting coroutine and return +control to the parent: + +.. code-block:: c++ + + T val; + if (awaiter.await_ready()) { + val = awaiter.await_resume(); + } else { + awaiter.await_suspend(); + return /* value representing the "execution short-circuited" outcome */; + } + +Then, a short-ciruiting coroutine is one where all the suspend points are +either (i) trivial (like ``std::suspend_never``), or (ii) short-circuiting. + +Although the coroutine machinery makes them harder to optimize, logically, +short-circuiting coroutines are like syntax sugar for regular functions where: + +- `co_await` allows expressions to return early. + +- `unhandled_exception()` lets the coroutine promise type wrap the function + body in an implicit try-catch. This mandatory exception boundary behavior + can be desirable in robust, return-value-oriented programs that benefit from + short-circuiting coroutines. If not, the promise can always re-throw. + +This attribute improves short-circuiting coroutines in a few ways: + +- **Avoid heap allocations for coro frames**: Allocating short-circuiting + coros on the stack makes code more predictable under memory pressure. + Without this attribute, LLVM cannot elide heap allocation even when all + awaiters are short-circuiting. + +- **Performance**: Significantly faster execution and smaller code size. + +- **Build time**: Faster compilation due to less IR being generated. + +Marking your ``await_suspend_destroy`` method as ``noexcept`` can sometimes +further improve optimization. + +Here is a toy example of a portable short-circuiting awaiter: + +.. code-block:: c++ + + template + struct [[clang::coro_await_suspend_destroy]] optional_awaiter { + std::optional opt_; + bool await_ready() const noexcept { return opt_.has_value(); } + T await_resume() { return std::move(opt_).value(); } + void await_suspend_destroy(auto& promise) { + // Assume the return object of the outer coro defaults to "empty". + } + // Fallback for when `coro_await_suspend_destroy` is unavailable. + void await_suspend(auto handle) { + await_suspend_destroy(handle.promise()); + handle.destroy(); + } + }; + +}]; +} + def CountedByDocs : Documentation { let Category = DocCatField; let Content = [{ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 116341f4b66d5..58e7dd7db86d1 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12504,6 +12504,9 @@ def note_coroutine_promise_call_implicitly_required : Note< def err_await_suspend_invalid_return_type : Error< "return type of 'await_suspend' is required to be 'void' or 'bool' (have %0)" >; +def err_await_suspend_destroy_invalid_return_type : Error< + "return type of 'await_suspend_destroy' is required to be 'void' (have %0)" +>; def note_await_ready_no_bool_conversion : Note< "return type of 'await_ready' is required to be contextually convertible to 'bool'" >; diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index 827385f9c1a1f..883a45d2acfff 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -174,6 +174,66 @@ static bool StmtCanThrow(const Stmt *S) { return false; } +// Check if this suspend should be calling `await_suspend_destroy` +static bool useCoroAwaitSuspendDestroy(const CoroutineSuspendExpr &S) { + // This can only be an `await_suspend_destroy` suspend expression if it + // returns void -- `buildCoawaitCalls` in `SemaCoroutine.cpp` asserts this. + // Moreover, when `await_suspend` returns a handle, the outermost method call + // is `.address()` -- making it harder to get the actual class or method. + if (S.getSuspendReturnType() != + CoroutineSuspendExpr::SuspendReturnType::SuspendVoid) { + return false; + } + + // `CGCoroutine.cpp` & `SemaCoroutine.cpp` must agree on whether this suspend + // expression uses `[[clang::coro_await_suspend_destroy]]`. + // + // Any mismatch is a serious bug -- we would either double-free, or fail to + // destroy the promise type. For this reason, we make our decision based on + // the method name, and fatal outside of the happy path -- including on + // failure to find a method name. + // + // As a debug-only check we also try to detect the `AwaiterClass`. This is + // secondary, because detection of the awaiter type can be silently broken by + // small `buildCoawaitCalls` AST changes. + StringRef SuspendMethodName; // Primary + CXXRecordDecl *AwaiterClass = nullptr; // Debug-only, best-effort + if (auto *SuspendCall = + dyn_cast(S.getSuspendExpr()->IgnoreImplicit())) { + if (auto *SuspendMember = dyn_cast(SuspendCall->getCallee())) { + if (auto *BaseExpr = SuspendMember->getBase()) { + // `IgnoreImplicitAsWritten` is critical since `await_suspend...` can be + // invoked on the base of the actual awaiter, and the base need not have + // the attribute. In such cases, the AST will show the true awaiter + // being upcast to the base. + AwaiterClass = BaseExpr->IgnoreImplicitAsWritten() + ->getType() + ->getAsCXXRecordDecl(); + } + if (auto *SuspendMethod = + dyn_cast(SuspendMember->getMemberDecl())) { + SuspendMethodName = SuspendMethod->getName(); + } + } + } + if (SuspendMethodName == "await_suspend_destroy") { + assert(!AwaiterClass || + AwaiterClass->hasAttr()); + return true; + } else if (SuspendMethodName == "await_suspend") { + assert(!AwaiterClass || + !AwaiterClass->hasAttr()); + return false; + } else { + llvm::report_fatal_error( + "Wrong method in [[clang::coro_await_suspend_destroy]] check: " + "expected 'await_suspend' or 'await_suspend_destroy', but got '" + + SuspendMethodName + "'"); + } + + return false; +} + // Emit suspend expression which roughly looks like: // // auto && x = CommonExpr(); @@ -220,51 +280,54 @@ namespace { RValue RV; }; } -static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Coro, - CoroutineSuspendExpr const &S, - AwaitKind Kind, AggValueSlot aggSlot, - bool ignoreResult, bool forLValue) { - auto *E = S.getCommonExpr(); - - auto CommonBinder = - CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(), E); - auto UnbindCommonOnExit = - llvm::make_scope_exit([&] { CommonBinder.unbind(CGF); }); - - auto Prefix = buildSuspendPrefixStr(Coro, Kind); - BasicBlock *ReadyBlock = CGF.createBasicBlock(Prefix + Twine(".ready")); - BasicBlock *SuspendBlock = CGF.createBasicBlock(Prefix + Twine(".suspend")); - BasicBlock *CleanupBlock = CGF.createBasicBlock(Prefix + Twine(".cleanup")); - // If expression is ready, no need to suspend. - CGF.EmitBranchOnBoolExpr(S.getReadyExpr(), ReadyBlock, SuspendBlock, 0); +// The simplified `await_suspend_destroy` path avoids suspend intrinsics. +// +// If a coro has only `await_suspend_destroy` and trivial (`suspend_never`) +// awaiters, then subsequent passes are able to allocate its frame on-stack. +// +// As of 2025, there is still an optimization gap between a realistic +// short-circuiting coro, and the equivalent plain function. For a +// guesstimate, expect 4-5ns per call on x86. One idea for improvement is to +// also elide trivial suspends like `std::suspend_never`, in order to hit the +// `HasCoroSuspend` path in `CoroEarly.cpp`. +static void emitAwaitSuspendDestroy(CodeGenFunction &CGF, CGCoroData &Coro, + llvm::Function *SuspendWrapper, + llvm::Value *Awaiter, llvm::Value *Frame, + bool AwaitSuspendCanThrow) { + SmallVector DirectCallArgs; + DirectCallArgs.push_back(Awaiter); + DirectCallArgs.push_back(Frame); + + if (AwaitSuspendCanThrow) { + CGF.EmitCallOrInvoke(SuspendWrapper, DirectCallArgs); + } else { + CGF.EmitNounwindRuntimeCall(SuspendWrapper, DirectCallArgs); + } - // Otherwise, emit suspend logic. - CGF.EmitBlock(SuspendBlock); + CGF.EmitBranchThroughCleanup(Coro.CleanupJD); +} +static void emitStandardAwaitSuspend( + CodeGenFunction &CGF, CGCoroData &Coro, CoroutineSuspendExpr const &S, + llvm::Function *SuspendWrapper, llvm::Value *Awaiter, llvm::Value *Frame, + bool AwaitSuspendCanThrow, SmallString<32> Prefix, BasicBlock *ReadyBlock, + AwaitKind Kind, CoroutineSuspendExpr::SuspendReturnType SuspendReturnType) { auto &Builder = CGF.Builder; - llvm::Function *CoroSave = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_save); - auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy); - auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr}); - - auto SuspendWrapper = CodeGenFunction(CGF.CGM).generateAwaitSuspendWrapper( - CGF.CurFn->getName(), Prefix, S); CGF.CurCoro.InSuspendBlock = true; - assert(CGF.CurCoro.Data && CGF.CurCoro.Data->CoroBegin && - "expected to be called in coroutine context"); - SmallVector SuspendIntrinsicCallArgs; - SuspendIntrinsicCallArgs.push_back( - CGF.getOrCreateOpaqueLValueMapping(S.getOpaqueValue()).getPointer(CGF)); - - SuspendIntrinsicCallArgs.push_back(CGF.CurCoro.Data->CoroBegin); + SuspendIntrinsicCallArgs.push_back(Awaiter); + SuspendIntrinsicCallArgs.push_back(Frame); SuspendIntrinsicCallArgs.push_back(SuspendWrapper); + BasicBlock *CleanupBlock = CGF.createBasicBlock(Prefix + Twine(".cleanup")); - const auto SuspendReturnType = S.getSuspendReturnType(); - llvm::Intrinsic::ID AwaitSuspendIID; + llvm::Function *CoroSave = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_save); + auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy); + auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr}); + llvm::Intrinsic::ID AwaitSuspendIID; switch (SuspendReturnType) { case CoroutineSuspendExpr::SuspendReturnType::SuspendVoid: AwaitSuspendIID = llvm::Intrinsic::coro_await_suspend_void; @@ -279,12 +342,6 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co llvm::Function *AwaitSuspendIntrinsic = CGF.CGM.getIntrinsic(AwaitSuspendIID); - // SuspendHandle might throw since it also resumes the returned handle. - const bool AwaitSuspendCanThrow = - SuspendReturnType == - CoroutineSuspendExpr::SuspendReturnType::SuspendHandle || - StmtCanThrow(S.getSuspendExpr()); - llvm::CallBase *SuspendRet = nullptr; // FIXME: add call attributes? if (AwaitSuspendCanThrow) @@ -332,6 +389,54 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co // Emit cleanup for this suspend point. CGF.EmitBlock(CleanupBlock); CGF.EmitBranchThroughCleanup(Coro.CleanupJD); +} + +static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Coro, + CoroutineSuspendExpr const &S, + AwaitKind Kind, AggValueSlot aggSlot, + bool ignoreResult, bool forLValue) { + auto *E = S.getCommonExpr(); + + auto CommonBinder = + CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(), E); + auto UnbindCommonOnExit = + llvm::make_scope_exit([&] { CommonBinder.unbind(CGF); }); + + auto Prefix = buildSuspendPrefixStr(Coro, Kind); + BasicBlock *ReadyBlock = CGF.createBasicBlock(Prefix + Twine(".ready")); + BasicBlock *SuspendBlock = CGF.createBasicBlock(Prefix + Twine(".suspend")); + + // If expression is ready, no need to suspend. + CGF.EmitBranchOnBoolExpr(S.getReadyExpr(), ReadyBlock, SuspendBlock, 0); + + // Otherwise, emit suspend logic. + CGF.EmitBlock(SuspendBlock); + + auto SuspendWrapper = CodeGenFunction(CGF.CGM).generateAwaitSuspendWrapper( + CGF.CurFn->getName(), Prefix, S); + + assert(CGF.CurCoro.Data && CGF.CurCoro.Data->CoroBegin && + "expected to be called in coroutine context"); + + // SuspendHandle might throw since it also resumes the returned handle. + const auto SuspendReturnType = S.getSuspendReturnType(); + const bool AwaitSuspendCanThrow = + SuspendReturnType == + CoroutineSuspendExpr::SuspendReturnType::SuspendHandle || + StmtCanThrow(S.getSuspendExpr()); + + llvm::Value *Awaiter = + CGF.getOrCreateOpaqueLValueMapping(S.getOpaqueValue()).getPointer(CGF); + llvm::Value *Frame = CGF.CurCoro.Data->CoroBegin; + + if (useCoroAwaitSuspendDestroy(S)) { // Call `await_suspend_destroy` & cleanup + emitAwaitSuspendDestroy(CGF, Coro, SuspendWrapper, Awaiter, Frame, + AwaitSuspendCanThrow); + } else { // Normal suspend path -- can actually suspend, uses intrinsics + emitStandardAwaitSuspend(CGF, Coro, S, SuspendWrapper, Awaiter, Frame, + AwaitSuspendCanThrow, Prefix, ReadyBlock, Kind, + SuspendReturnType); + } // Emit await_resume expression. CGF.EmitBlock(ReadyBlock); @@ -341,6 +446,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co CXXTryStmt *TryStmt = nullptr; if (Coro.ExceptionHandler && Kind == AwaitKind::Init && StmtCanThrow(S.getResumeExpr())) { + auto &Builder = CGF.Builder; Coro.ResumeEHVar = CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh")); Builder.CreateFlagStore(true, Coro.ResumeEHVar); diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index d193a33f22393..83fe7219c9997 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -289,6 +289,45 @@ static ExprResult buildCoroutineHandle(Sema &S, QualType PromiseType, return S.BuildCallExpr(nullptr, FromAddr.get(), Loc, FramePtr, Loc); } +// To support [[clang::coro_await_suspend_destroy]], this builds +// *static_cast( +// __builtin_coro_promise(handle, alignof(Promise), false)) +static ExprResult buildPromiseRef(Sema &S, QualType PromiseType, + SourceLocation Loc) { + uint64_t Align = + S.Context.getTypeAlign(PromiseType) / S.Context.getCharWidth(); + + // Build the call to __builtin_coro_promise() + SmallVector Args = { + S.BuildBuiltinCallExpr(Loc, Builtin::BI__builtin_coro_frame, {}), + S.ActOnIntegerConstant(Loc, Align).get(), // alignof(Promise) + S.ActOnCXXBoolLiteral(Loc, tok::kw_false).get()}; // false + ExprResult CoroPromiseCall = + S.BuildBuiltinCallExpr(Loc, Builtin::BI__builtin_coro_promise, Args); + + if (CoroPromiseCall.isInvalid()) + return ExprError(); + + // Cast to Promise* + ExprResult CastExpr = S.ImpCastExprToType( + CoroPromiseCall.get(), S.Context.getPointerType(PromiseType), CK_BitCast); + if (CastExpr.isInvalid()) + return ExprError(); + + // Dereference to get Promise& + return S.CreateBuiltinUnaryOp(Loc, UO_Deref, CastExpr.get()); +} + +static bool hasCoroAwaitSuspendDestroyAttr(Expr *Awaiter) { + QualType AwaiterType = Awaiter->getType(); + if (auto *RD = AwaiterType->getAsCXXRecordDecl()) { + if (RD->hasAttr()) { + return true; + } + } + return false; +} + struct ReadySuspendResumeResult { enum AwaitCallType { ACT_Ready, ACT_Suspend, ACT_Resume }; Expr *Results[3]; @@ -399,15 +438,30 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, VarDecl *CoroPromise, Calls.Results[ACT::ACT_Ready] = S.MaybeCreateExprWithCleanups(Conv.get()); } - ExprResult CoroHandleRes = - buildCoroutineHandle(S, CoroPromise->getType(), Loc); - if (CoroHandleRes.isInvalid()) { - Calls.IsInvalid = true; - return Calls; + // For awaiters with `[[clang::coro_await_suspend_destroy]]`, we call + // `void await_suspend_destroy(Promise&)` & promptly destroy the coro. + CallExpr *AwaitSuspend = nullptr; + bool UseAwaitSuspendDestroy = hasCoroAwaitSuspendDestroyAttr(Operand); + if (UseAwaitSuspendDestroy) { + ExprResult PromiseRefRes = buildPromiseRef(S, CoroPromise->getType(), Loc); + if (PromiseRefRes.isInvalid()) { + Calls.IsInvalid = true; + return Calls; + } + Expr *PromiseRef = PromiseRefRes.get(); + AwaitSuspend = cast_or_null( + BuildSubExpr(ACT::ACT_Suspend, "await_suspend_destroy", PromiseRef)); + } else { // The standard `await_suspend(std::coroutine_handle<...>)` + ExprResult CoroHandleRes = + buildCoroutineHandle(S, CoroPromise->getType(), Loc); + if (CoroHandleRes.isInvalid()) { + Calls.IsInvalid = true; + return Calls; + } + Expr *CoroHandle = CoroHandleRes.get(); + AwaitSuspend = cast_or_null( + BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle)); } - Expr *CoroHandle = CoroHandleRes.get(); - CallExpr *AwaitSuspend = cast_or_null( - BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle)); if (!AwaitSuspend) return Calls; if (!AwaitSuspend->getType()->isDependentType()) { @@ -417,25 +471,37 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, VarDecl *CoroPromise, // type Z. QualType RetType = AwaitSuspend->getCallReturnType(S.Context); - // Support for coroutine_handle returning await_suspend. - if (Expr *TailCallSuspend = - maybeTailCall(S, RetType, AwaitSuspend, Loc)) + auto EmitAwaitSuspendDiag = [&](unsigned int DiagCode) { + S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(), DiagCode) << RetType; + S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required) + << AwaitSuspend->getDirectCallee(); + Calls.IsInvalid = true; + }; + + // `await_suspend_destroy` must return `void` -- and `CGCoroutine.cpp` + // critically depends on this in `hasCoroAwaitSuspendDestroyAttr`. + if (UseAwaitSuspendDestroy) { + if (RetType->isVoidType()) { + Calls.Results[ACT::ACT_Suspend] = + S.MaybeCreateExprWithCleanups(AwaitSuspend); + } else { + EmitAwaitSuspendDiag( + diag::err_await_suspend_destroy_invalid_return_type); + } + // Support for coroutine_handle returning await_suspend. + } else if (Expr *TailCallSuspend = + maybeTailCall(S, RetType, AwaitSuspend, Loc)) { // Note that we don't wrap the expression with ExprWithCleanups here // because that might interfere with tailcall contract (e.g. inserting // clean up instructions in-between tailcall and return). Instead // ExprWithCleanups is wrapped within maybeTailCall() prior to the resume // call. Calls.Results[ACT::ACT_Suspend] = TailCallSuspend; - else { + } else { // non-class prvalues always have cv-unqualified types if (RetType->isReferenceType() || (!RetType->isBooleanType() && !RetType->isVoidType())) { - S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(), - diag::err_await_suspend_invalid_return_type) - << RetType; - S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required) - << AwaitSuspend->getDirectCallee(); - Calls.IsInvalid = true; + EmitAwaitSuspendDiag(diag::err_await_suspend_invalid_return_type); } else Calls.Results[ACT::ACT_Suspend] = S.MaybeCreateExprWithCleanups(AwaitSuspend); diff --git a/clang/test/CodeGenCoroutines/coro-await-suspend-destroy-errors.cpp b/clang/test/CodeGenCoroutines/coro-await-suspend-destroy-errors.cpp new file mode 100644 index 0000000000000..6a082c15f2581 --- /dev/null +++ b/clang/test/CodeGenCoroutines/coro-await-suspend-destroy-errors.cpp @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s + +#include "Inputs/coroutine.h" + +// Coroutine type with `std::suspend_never` for initial/final suspend +struct Task { + struct promise_type { + Task get_return_object() { return {}; } + std::suspend_never initial_suspend() { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void return_void() {} + void unhandled_exception() {} + }; +}; + +struct [[clang::coro_await_suspend_destroy]] WrongReturnTypeAwaitable { + bool await_ready() { return false; } + bool await_suspend_destroy(auto& promise) { return true; } // expected-error {{return type of 'await_suspend_destroy' is required to be 'void' (have 'bool')}} + void await_suspend(auto handle) { + await_suspend_destroy(handle.promise()); + handle.destroy(); + } + void await_resume() {} +}; + +Task test_invalid_destroying_await() { + co_await WrongReturnTypeAwaitable{}; // expected-note {{call to 'await_suspend_destroy' implicitly required by coroutine function here}} +} + +struct [[clang::coro_await_suspend_destroy]] MissingMethodAwaitable { + bool await_ready() { return false; } + // Missing await_suspend_destroy method + void await_suspend(auto handle) { + handle.destroy(); + } + void await_resume() {} +}; + +Task test_missing_method() { + co_await MissingMethodAwaitable{}; // expected-error {{no member named 'await_suspend_destroy' in 'MissingMethodAwaitable'}} +} + +struct [[clang::coro_await_suspend_destroy]] WrongParameterTypeAwaitable { + bool await_ready() { return false; } + void await_suspend_destroy(int x) {} // expected-note {{passing argument to parameter 'x' here}} + void await_suspend(auto handle) { + await_suspend_destroy(handle.promise()); + handle.destroy(); + } + void await_resume() {} +}; + +Task test_wrong_parameter_type() { + co_await WrongParameterTypeAwaitable{}; // expected-error {{no viable conversion from 'std::coroutine_traits::promise_type' (aka 'Task::promise_type') to 'int'}} +} diff --git a/clang/test/CodeGenCoroutines/coro-await-suspend-destroy.cpp b/clang/test/CodeGenCoroutines/coro-await-suspend-destroy.cpp new file mode 100644 index 0000000000000..fa1dbf475e56c --- /dev/null +++ b/clang/test/CodeGenCoroutines/coro-await-suspend-destroy.cpp @@ -0,0 +1,129 @@ +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s \ +// RUN: -disable-llvm-passes | FileCheck %s --check-prefix=CHECK-INITIAL +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s \ +// RUN: -O2 | FileCheck %s --check-prefix=CHECK-OPTIMIZED + +#include "Inputs/coroutine.h" + +// Awaitable with `coro_await_suspend_destroy` attribute +struct [[clang::coro_await_suspend_destroy]] DestroyingAwaitable { + bool await_ready() { return false; } + void await_suspend_destroy(auto& promise) {} + void await_suspend(auto handle) { + await_suspend_destroy(handle.promise()); + handle.destroy(); + } + void await_resume() {} +}; + +// Awaitable without `coro_await_suspend_destroy` (normal behavior) +struct NormalAwaitable { + bool await_ready() { return false; } + void await_suspend(std::coroutine_handle<> h) {} + void await_resume() {} +}; + +// Coroutine type with `std::suspend_never` for initial/final suspend +struct Task { + struct promise_type { + Task get_return_object() { return {}; } + std::suspend_never initial_suspend() { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void return_void() {} + void unhandled_exception() {} + }; +}; + +// Single co_await with coro_await_suspend_destroy. +// Should result in no allocation after optimization. +Task test_single_destroying_await() { + co_await DestroyingAwaitable{}; +} + +// CHECK-INITIAL-LABEL: define{{.*}} void @_Z28test_single_destroying_awaitv +// CHECK-INITIAL: call{{.*}} @llvm.coro.alloc +// CHECK-INITIAL: call{{.*}} @llvm.coro.begin + +// CHECK-OPTIMIZED-LABEL: define{{.*}} void @_Z28test_single_destroying_awaitv +// CHECK-OPTIMIZED-NOT: call{{.*}} @llvm.coro.alloc +// CHECK-OPTIMIZED-NOT: call{{.*}} malloc +// CHECK-OPTIMIZED-NOT: call{{.*}} @_Znwm + +// Test multiple `co_await`s, all with `coro_await_suspend_destroy`. +// This should also result in no allocation after optimization. +Task test_multiple_destroying_awaits(bool condition) { + co_await DestroyingAwaitable{}; + co_await DestroyingAwaitable{}; + if (condition) { + co_await DestroyingAwaitable{}; + } +} + +// CHECK-INITIAL-LABEL: define{{.*}} void @_Z31test_multiple_destroying_awaitsb +// CHECK-INITIAL: call{{.*}} @llvm.coro.alloc +// CHECK-INITIAL: call{{.*}} @llvm.coro.begin + +// CHECK-OPTIMIZED-LABEL: define{{.*}} void @_Z31test_multiple_destroying_awaitsb +// CHECK-OPTIMIZED-NOT: call{{.*}} @llvm.coro.alloc +// CHECK-OPTIMIZED-NOT: call{{.*}} malloc +// CHECK-OPTIMIZED-NOT: call{{.*}} @_Znwm + +// Mixed awaits - some with `coro_await_suspend_destroy`, some without. +// We should still see allocation because not all awaits destroy the coroutine. +Task test_mixed_awaits() { + co_await NormalAwaitable{}; // Must precede "destroy" to be reachable + co_await DestroyingAwaitable{}; +} + +// CHECK-INITIAL-LABEL: define{{.*}} void @_Z17test_mixed_awaitsv +// CHECK-INITIAL: call{{.*}} @llvm.coro.alloc +// CHECK-INITIAL: call{{.*}} @llvm.coro.begin + +// CHECK-OPTIMIZED-LABEL: define{{.*}} void @_Z17test_mixed_awaitsv +// CHECK-OPTIMIZED: call{{.*}} @_Znwm + + +// Check the attribute detection affects control flow. +Task test_attribute_detection() { + co_await DestroyingAwaitable{}; + // Unreachable in OPTIMIZED, so those builds don't see an allocation. + co_await NormalAwaitable{}; +} + +// Check that we skip the normal suspend intrinsic and go directly to cleanup. +// +// CHECK-INITIAL-LABEL: define{{.*}} void @_Z24test_attribute_detectionv +// CHECK-INITIAL: call{{.*}} @_Z24test_attribute_detectionv.__await_suspend_wrapper__await +// CHECK-INITIAL-NEXT: br label %cleanup5 +// CHECK-INITIAL-NOT: call{{.*}} @llvm.coro.suspend +// CHECK-INITIAL: call{{.*}} @_Z24test_attribute_detectionv.__await_suspend_wrapper__await +// CHECK-INITIAL: call{{.*}} @llvm.coro.suspend +// CHECK-INITIAL: call{{.*}} @_Z24test_attribute_detectionv.__await_suspend_wrapper__final + +// Since `co_await DestroyingAwaitable{}` gets converted into an unconditional +// branch, the `co_await NormalAwaitable{}` is unreachable in optimized builds. +// +// CHECK-OPTIMIZED-NOT: call{{.*}} @llvm.coro.alloc +// CHECK-OPTIMIZED-NOT: call{{.*}} malloc +// CHECK-OPTIMIZED-NOT: call{{.*}} @_Znwm + +// Template awaitable with `coro_await_suspend_destroy` attribute +template +struct [[clang::coro_await_suspend_destroy]] TemplateDestroyingAwaitable { + bool await_ready() { return false; } + void await_suspend_destroy(auto& promise) {} + void await_suspend(auto handle) { + await_suspend_destroy(handle.promise()); + handle.destroy(); + } + void await_resume() {} +}; + +Task test_template_destroying_await() { + co_await TemplateDestroyingAwaitable{}; +} + +// CHECK-OPTIMIZED-LABEL: define{{.*}} void @_Z30test_template_destroying_awaitv +// CHECK-OPTIMIZED-NOT: call{{.*}} @llvm.coro.alloc +// CHECK-OPTIMIZED-NOT: call{{.*}} malloc +// CHECK-OPTIMIZED-NOT: call{{.*}} @_Znwm diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index 05693538252aa..43327744ffc8a 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -62,6 +62,7 @@ // CHECK-NEXT: Convergent (SubjectMatchRule_function) // CHECK-NEXT: CoroAwaitElidable (SubjectMatchRule_record) // CHECK-NEXT: CoroAwaitElidableArgument (SubjectMatchRule_variable_is_parameter) +// CHECK-NEXT: CoroAwaitSuspendDestroy (SubjectMatchRule_record) // CHECK-NEXT: CoroDisableLifetimeBound (SubjectMatchRule_function) // CHECK-NEXT: CoroLifetimeBound (SubjectMatchRule_record) // CHECK-NEXT: CoroOnlyDestroyWhenComplete (SubjectMatchRule_record)