Skip to content

Commit

Permalink
Revert "[C++20] [Coroutines] Mark await_suspend as noinline if the aw…
Browse files Browse the repository at this point in the history
…aiter is not empty"

This reverts commit f05226d.
  • Loading branch information
ChuanqiXu9 committed Sep 18, 2023
1 parent 2cfdebd commit 88bf774
Show file tree
Hide file tree
Showing 6 changed files with 0 additions and 358 deletions.
4 changes: 0 additions & 4 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -711,10 +711,6 @@ Bug Fixes in This Version
- Fix a hang on valid C code passing a function type as an argument to
``typeof`` to form a function declaration.
(`#64713 <https://github.com/llvm/llvm-project/issues/64713>_`)
- Fixed an issue where accesses to the local variables of a coroutine during
``await_suspend`` could be misoptimized, including accesses to the awaiter
object itself.
(`#56301 <https://github.com/llvm/llvm-project/issues/56301>`_)
- Clang now correctly diagnoses ``function_needs_feature`` when always_inline
callee has incompatible target features with caller.
- Removed the linking of libraries when ``-r`` is passed to the driver on AIX.
Expand Down
24 changes: 0 additions & 24 deletions clang/lib/CodeGen/CGCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5487,30 +5487,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline);
}

// 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.
//
// 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.
if (inSuspendBlock() && mayCoroHandleEscape())
Attrs = Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::NoInline);

// Disable inlining inside SEH __try blocks.
if (isSEHTryScope()) {
Attrs = Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::NoInline);
Expand Down
33 changes: 0 additions & 33 deletions clang/lib/CodeGen/CGCoroutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,36 +139,6 @@ static bool memberCallExpressionCanThrow(const Expr *E) {
return true;
}

/// Return true when the coroutine handle may escape from the await-suspend
/// (`awaiter.await_suspend(std::coroutine_handle)` expression).
/// Return false only when the coroutine wouldn't escape in the await-suspend
/// for sure.
///
/// While it is always safe to return true, return falses can bring better
/// performances.
///
/// See https://github.com/llvm/llvm-project/issues/56301 and
/// https://reviews.llvm.org/D157070 for the example and the full discussion.
///
/// FIXME: It will be much better to perform such analysis in the middle end.
/// See the comments in `CodeGenFunction::EmitCall` for example.
static bool MayCoroHandleEscape(CoroutineSuspendExpr const &S) {
CXXRecordDecl *Awaiter =
S.getCommonExpr()->getType().getNonReferenceType()->getAsCXXRecordDecl();

// Return true conservatively if the awaiter type is not a record type.
if (!Awaiter)
return true;

// In case the awaiter type is empty, the suspend wouldn't leak the coroutine
// handle.
//
// TODO: We can improve this by looking into the implementation of
// await-suspend and see if the coroutine handle is passed to foreign
// functions.
return !Awaiter->field_empty();
}

// Emit suspend expression which roughly looks like:
//
// auto && x = CommonExpr();
Expand Down Expand Up @@ -229,11 +199,8 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});

CGF.CurCoro.InSuspendBlock = true;
CGF.CurCoro.MayCoroHandleEscape = MayCoroHandleEscape(S);
auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
CGF.CurCoro.InSuspendBlock = false;
CGF.CurCoro.MayCoroHandleEscape = false;

if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
// Veto suspension if requested by bool returning await_suspend.
BasicBlock *RealSuspendBlock =
Expand Down
5 changes: 0 additions & 5 deletions clang/lib/CodeGen/CodeGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,6 @@ class CodeGenFunction : public CodeGenTypeCache {
struct CGCoroInfo {
std::unique_ptr<CGCoroData> Data;
bool InSuspendBlock = false;
bool MayCoroHandleEscape = false;
CGCoroInfo();
~CGCoroInfo();
};
Expand All @@ -348,10 +347,6 @@ class CodeGenFunction : public CodeGenTypeCache {
return isCoroutine() && CurCoro.InSuspendBlock;
}

bool mayCoroHandleEscape() const {
return isCoroutine() && CurCoro.MayCoroHandleEscape;
}

/// CurGD - The GlobalDecl for the current function being compiled.
GlobalDecl CurGD;

Expand Down
207 changes: 0 additions & 207 deletions clang/test/CodeGenCoroutines/coro-awaiter-noinline-suspend.cpp

This file was deleted.

Loading

0 comments on commit 88bf774

Please sign in to comment.