Skip to content

Commit 88bf774

Browse files
committed
Revert "[C++20] [Coroutines] Mark await_suspend as noinline if the awaiter is not empty"
This reverts commit f05226d.
1 parent 2cfdebd commit 88bf774

File tree

6 files changed

+0
-358
lines changed

6 files changed

+0
-358
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -711,10 +711,6 @@ Bug Fixes in This Version
711711
- Fix a hang on valid C code passing a function type as an argument to
712712
``typeof`` to form a function declaration.
713713
(`#64713 <https://github.com/llvm/llvm-project/issues/64713>_`)
714-
- Fixed an issue where accesses to the local variables of a coroutine during
715-
``await_suspend`` could be misoptimized, including accesses to the awaiter
716-
object itself.
717-
(`#56301 <https://github.com/llvm/llvm-project/issues/56301>`_)
718714
- Clang now correctly diagnoses ``function_needs_feature`` when always_inline
719715
callee has incompatible target features with caller.
720716
- Removed the linking of libraries when ``-r`` is passed to the driver on AIX.

clang/lib/CodeGen/CGCall.cpp

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5487,30 +5487,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
54875487
Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline);
54885488
}
54895489

5490-
// The await_suspend call performed by co_await is essentially asynchronous
5491-
// to the execution of the coroutine. Inlining it normally into an unsplit
5492-
// coroutine can cause miscompilation because the coroutine CFG misrepresents
5493-
// the true control flow of the program: things that happen in the
5494-
// await_suspend are not guaranteed to happen prior to the resumption of the
5495-
// coroutine, and things that happen after the resumption of the coroutine
5496-
// (including its exit and the potential deallocation of the coroutine frame)
5497-
// are not guaranteed to happen only after the end of await_suspend.
5498-
//
5499-
// The short-term solution to this problem is to mark the call as uninlinable.
5500-
// But we don't want to do this if the call is known to be trivial, which is
5501-
// very common.
5502-
//
5503-
// The long-term solution may introduce patterns like:
5504-
//
5505-
// call @llvm.coro.await_suspend(ptr %awaiter, ptr %handle,
5506-
// ptr @awaitSuspendFn)
5507-
//
5508-
// Then it is much easier to perform the safety analysis in the middle end.
5509-
// If it is safe to inline the call to awaitSuspend, we can replace it in the
5510-
// CoroEarly pass. Otherwise we could replace it in the CoroSplit pass.
5511-
if (inSuspendBlock() && mayCoroHandleEscape())
5512-
Attrs = Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::NoInline);
5513-
55145490
// Disable inlining inside SEH __try blocks.
55155491
if (isSEHTryScope()) {
55165492
Attrs = Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::NoInline);

clang/lib/CodeGen/CGCoroutine.cpp

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -139,36 +139,6 @@ static bool memberCallExpressionCanThrow(const Expr *E) {
139139
return true;
140140
}
141141

142-
/// Return true when the coroutine handle may escape from the await-suspend
143-
/// (`awaiter.await_suspend(std::coroutine_handle)` expression).
144-
/// Return false only when the coroutine wouldn't escape in the await-suspend
145-
/// for sure.
146-
///
147-
/// While it is always safe to return true, return falses can bring better
148-
/// performances.
149-
///
150-
/// See https://github.com/llvm/llvm-project/issues/56301 and
151-
/// https://reviews.llvm.org/D157070 for the example and the full discussion.
152-
///
153-
/// FIXME: It will be much better to perform such analysis in the middle end.
154-
/// See the comments in `CodeGenFunction::EmitCall` for example.
155-
static bool MayCoroHandleEscape(CoroutineSuspendExpr const &S) {
156-
CXXRecordDecl *Awaiter =
157-
S.getCommonExpr()->getType().getNonReferenceType()->getAsCXXRecordDecl();
158-
159-
// Return true conservatively if the awaiter type is not a record type.
160-
if (!Awaiter)
161-
return true;
162-
163-
// In case the awaiter type is empty, the suspend wouldn't leak the coroutine
164-
// handle.
165-
//
166-
// TODO: We can improve this by looking into the implementation of
167-
// await-suspend and see if the coroutine handle is passed to foreign
168-
// functions.
169-
return !Awaiter->field_empty();
170-
}
171-
172142
// Emit suspend expression which roughly looks like:
173143
//
174144
// auto && x = CommonExpr();
@@ -229,11 +199,8 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
229199
auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
230200

231201
CGF.CurCoro.InSuspendBlock = true;
232-
CGF.CurCoro.MayCoroHandleEscape = MayCoroHandleEscape(S);
233202
auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
234203
CGF.CurCoro.InSuspendBlock = false;
235-
CGF.CurCoro.MayCoroHandleEscape = false;
236-
237204
if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
238205
// Veto suspension if requested by bool returning await_suspend.
239206
BasicBlock *RealSuspendBlock =

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,6 @@ class CodeGenFunction : public CodeGenTypeCache {
334334
struct CGCoroInfo {
335335
std::unique_ptr<CGCoroData> Data;
336336
bool InSuspendBlock = false;
337-
bool MayCoroHandleEscape = false;
338337
CGCoroInfo();
339338
~CGCoroInfo();
340339
};
@@ -348,10 +347,6 @@ class CodeGenFunction : public CodeGenTypeCache {
348347
return isCoroutine() && CurCoro.InSuspendBlock;
349348
}
350349

351-
bool mayCoroHandleEscape() const {
352-
return isCoroutine() && CurCoro.MayCoroHandleEscape;
353-
}
354-
355350
/// CurGD - The GlobalDecl for the current function being compiled.
356351
GlobalDecl CurGD;
357352

clang/test/CodeGenCoroutines/coro-awaiter-noinline-suspend.cpp

Lines changed: 0 additions & 207 deletions
This file was deleted.

0 commit comments

Comments
 (0)