================ @@ -380,66 +380,7 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E, // __builtin_coro_resume so that the cleanup code are not inserted in-between // the resume call and return instruction, which would interfere with the // musttail call contract. - JustAddress = S.MaybeCreateExprWithCleanups(JustAddress); - return S.BuildBuiltinCallExpr(Loc, Builtin::BI__builtin_coro_resume, - JustAddress); -} - -/// The await_suspend call performed by co_await is essentially asynchronous -/// to the execution of the coroutine. Inlining it normally into an unsplit -/// coroutine can cause miscompilation because the coroutine CFG misrepresents -/// the true control flow of the program: things that happen in the -/// await_suspend are not guaranteed to happen prior to the resumption of the -/// coroutine, and things that happen after the resumption of the coroutine -/// (including its exit and the potential deallocation of the coroutine frame) -/// are not guaranteed to happen only after the end of await_suspend. -/// -/// See https://github.com/llvm/llvm-project/issues/56301 and -/// https://reviews.llvm.org/D157070 for the example and the full discussion. -/// -/// The short-term solution to this problem is to mark the call as uninlinable. -/// But we don't want to do this if the call is known to be trivial, which is -/// very common. -/// -/// The long-term solution may introduce patterns like: -/// -/// call @llvm.coro.await_suspend(ptr %awaiter, ptr %handle, -/// ptr @awaitSuspendFn) -/// -/// Then it is much easier to perform the safety analysis in the middle end. -/// If it is safe to inline the call to awaitSuspend, we can replace it in the -/// CoroEarly pass. Otherwise we could replace it in the CoroSplit pass. -static void tryMarkAwaitSuspendNoInline(Sema &S, OpaqueValueExpr *Awaiter, - CallExpr *AwaitSuspend) { - // The method here to extract the awaiter decl is not precise. - // This is intentional. Since it is hard to perform the analysis in the - // frontend due to the complexity of C++'s type systems. - // And we prefer to perform such analysis in the middle end since it is - // easier to implement and more powerful. - CXXRecordDecl *AwaiterDecl = - Awaiter->getType().getNonReferenceType()->getAsCXXRecordDecl(); - - if (AwaiterDecl && AwaiterDecl->field_empty()) - return; - - FunctionDecl *FD = AwaitSuspend->getDirectCallee(); - - assert(FD); - - // If the `await_suspend()` function is marked as `always_inline` explicitly, - // we should give the user the right to control the codegen. - if (FD->hasAttr<NoInlineAttr>() || FD->hasAttr<AlwaysInlineAttr>()) - return; - - // This is problematic if the user calls the await_suspend standalone. But on - // the on hand, it is not incorrect semantically since inlining is not part - // of the standard. On the other hand, it is relatively rare to call - // the await_suspend function standalone. - // - // And given we've already had the long-term plan, the current workaround - // looks relatively tolerant. - FD->addAttr( - NoInlineAttr::CreateImplicit(S.getASTContext(), FD->getLocation())); + return S.MaybeCreateExprWithCleanups(JustAddress); ---------------- ChuanqiXu9 wrote:
We also need to update the comments of the function after this patch. Also I feel like the above fixme can be removed. https://github.com/llvm/llvm-project/pull/79712 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits