llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-coroutines Author: Yexuan Xiao (YexuanXiao) <details> <summary>Changes</summary> …WG2935) This patch attempts to implement the solution I proposed for [CWG2935 (Github)](https://github.com/cplusplus/CWG/issues/575), aligning Clang's behavior with GCC and MSVC instead of leaving it undefined. When `initial_suspend` (as well as `ready` and `suspend`) throws an exception, Clang fails to destroy the task even though the task has already been initialized (see https://godbolt.org/z/E4Y4bEn54). This patch updates CGCoroutine.cpp to clean up the coroutine return value after an exception is thrown when it is constructed in place, addressing CWG2935. I would like to hear more opinions on the solution and seek help to fix Clang. --- Full diff: https://github.com/llvm/llvm-project/pull/202279.diff 2 Files Affected: - (modified) clang/lib/CodeGen/CGCoroutine.cpp (+46) - (added) clang/test/CodeGenCoroutines/coro-cwg2935.cpp (+35) ``````````diff diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index 7b820dc9b99f5..700a8fd4a48c1 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -64,6 +64,12 @@ struct clang::CodeGen::CGCoroData { // statements jumps to this point after calling return_xxx promise member. CodeGenFunction::JumpDest FinalJD; + // A cleanup flag for the coroutine return value object when it is initialized + // directly by the get-return-object invocation. It models the standard's + // initial-await-resume-called guard for exceptions thrown during coroutine + // startup, before the initial await_resume starts. + Address InitialReturnObjectActiveFlag = Address::invalid(); + // Stores the llvm.coro.id emitted in the function so that we can supply it // as the first argument to coro.begin, coro.alloc and coro.free intrinsics. // Note: llvm.coro.id returns a token that cannot be directly expressed in a @@ -338,6 +344,10 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co // Emit await_resume expression. CGF.EmitBlock(ReadyBlock); + if (Kind == AwaitKind::Init && Coro.InitialReturnObjectActiveFlag.isValid()) { + Builder.CreateStore(Builder.getFalse(), Coro.InitialReturnObjectActiveFlag); + Coro.InitialReturnObjectActiveFlag = Address::invalid(); + } // Exception handling requires additional IR. If the 'await_resume' function // is marked as 'noexcept', we avoid generating this additional IR. @@ -753,6 +763,38 @@ struct GetReturnObjectManager { } } + void EmitDirectReturnObjectCleanup() { + if (!DirectEmit || !CGF.ReturnValue.isValid()) + return; + + QualType RetTy = CGF.FnRetTy; + QualType::DestructionKind DtorKind = RetTy.isDestructedType(); + if (DtorKind == QualType::DK_none) + return; + if (!CGF.needsEHCleanup(DtorKind)) + return; + + Address ActiveFlag = CGF.CreateTempAlloca( + Builder.getInt1Ty(), CharUnits::One(), "coro.result.active"); + Builder.CreateStore(Builder.getFalse(), ActiveFlag); + CGF.CurCoro.Data->InitialReturnObjectActiveFlag = ActiveFlag; + + auto OldTop = CGF.EHStack.stable_begin(); + CGF.pushDestroy(EHCleanup, CGF.ReturnValue, RetTy, + CGF.getDestroyer(DtorKind), + /*useEHCleanupForArray*/ true); + auto Top = CGF.EHStack.stable_begin(); + + for (auto B = CGF.EHStack.find(Top), E = CGF.EHStack.find(OldTop); B != E; + ++B) { + if (auto *Cleanup = dyn_cast<EHCleanupScope>(&*B)) { + assert(!Cleanup->hasActiveFlag() && "cleanup already has active flag?"); + Cleanup->setActiveFlag(ActiveFlag); + Cleanup->setTestFlagInEHCleanup(); + } + } + } + void EmitGroInit() { if (DirectEmit) { // ReturnValue should be valid as long as the coroutine's return type @@ -768,9 +810,13 @@ struct GetReturnObjectManager { // otherwise the call to get_return_object wouldn't be in front // of initial_suspend. if (CGF.ReturnValue.isValid()) { + EmitDirectReturnObjectCleanup(); CGF.EmitAnyExprToMem(S.getReturnValue(), CGF.ReturnValue, S.getReturnValue()->getType().getQualifiers(), /*IsInit*/ true); + if (CGF.CurCoro.Data->InitialReturnObjectActiveFlag.isValid()) + Builder.CreateStore(Builder.getTrue(), + CGF.CurCoro.Data->InitialReturnObjectActiveFlag); } return; } diff --git a/clang/test/CodeGenCoroutines/coro-cwg2935.cpp b/clang/test/CodeGenCoroutines/coro-cwg2935.cpp new file mode 100644 index 0000000000000..ebb8158ec590c --- /dev/null +++ b/clang/test/CodeGenCoroutines/coro-cwg2935.cpp @@ -0,0 +1,35 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 \ +// RUN: -fcxx-exceptions -fexceptions -emit-llvm -disable-llvm-passes \ +// RUN: %s -o - | FileCheck %s + +#include "Inputs/coroutine.h" + +struct task { + ~task(); + + struct promise_type { + task get_return_object(); + std::suspend_never initial_suspend(); + std::suspend_never final_suspend() noexcept; + void return_void(); + void unhandled_exception(); + }; +}; + +task f() { + co_return; +} + +// CHECK-LABEL: define{{.*}} void @_Z1fv( +// CHECK: %[[RESULT_ACTIVE:.+]] = alloca i1 +// CHECK: store i1 false, ptr %[[RESULT_ACTIVE]] +// CHECK: invoke void @_ZN4task12promise_type17get_return_objectEv( +// CHECK: store i1 true, ptr %[[RESULT_ACTIVE]] +// CHECK: invoke void @_ZN4task12promise_type15initial_suspendEv( +// CHECK: init.ready: +// CHECK-NEXT: store i1 false, ptr %[[RESULT_ACTIVE]] +// CHECK-NEXT: call void @_ZNSt13suspend_never12await_resumeEv( +// CHECK: %[[IS_ACTIVE:.+]] = load i1, ptr %[[RESULT_ACTIVE]] +// CHECK-NEXT: br i1 %[[IS_ACTIVE]], label %[[CLEANUP_ACTION:.+]], label %[[CLEANUP_DONE:.+]] +// CHECK: [[CLEANUP_ACTION]]: +// CHECK-NEXT: call void @_ZN4taskD1Ev( `````````` </details> https://github.com/llvm/llvm-project/pull/202279 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
