[llvm-branch-commits] [clang] 88bf774 - Revert "[C++20] [Coroutines] Mark await_suspend as noinline if the awaiter is not empty"

2023-09-18 Thread Chuanqi Xu via llvm-branch-commits 

Author: Chuanqi Xu
Date: 2023-09-18T15:09:00+08:00
New Revision: 88bf774c565080e30e0a073676c316ab175303af

URL: 
https://github.com/llvm/llvm-project/commit/88bf774c565080e30e0a073676c316ab175303af
DIFF: 
https://github.com/llvm/llvm-project/commit/88bf774c565080e30e0a073676c316ab175303af.diff

LOG: Revert "[C++20] [Coroutines] Mark await_suspend as noinline if the awaiter 
is not empty"

This reverts commit f05226d7e38c36efe029a0eb4201b0843f81b5e8.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGCoroutine.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 
clang/test/CodeGenCoroutines/coro-awaiter-noinline-suspend.cpp
clang/test/CodeGenCoroutines/pr56301.cpp



diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8ab7e065d218412..a4b61b9074d91e0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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 _`)
-- 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 `_)
 - 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.

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 0d1e9ad439b7dc9..6b8af9bf18c1fff 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -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);

diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 810ae7d51ec10c2..8437cda79beb2a7 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -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

[llvm-branch-commits] [clang] 2cfdebd - Revert "[NFC] [C++20] [Coroutines] Mention the side effect of a fix may bring regressions"

2023-09-18 Thread Chuanqi Xu via llvm-branch-commits 

Author: Chuanqi Xu
Date: 2023-09-18T15:05:07+08:00
New Revision: 2cfdebdb7e5e430471ea833b33eee72c1938eb98

URL: 
https://github.com/llvm/llvm-project/commit/2cfdebdb7e5e430471ea833b33eee72c1938eb98
DIFF: 
https://github.com/llvm/llvm-project/commit/2cfdebdb7e5e430471ea833b33eee72c1938eb98.diff

LOG: Revert "[NFC] [C++20] [Coroutines] Mention the side effect of a fix may 
bring regressions"

This reverts commit 6998ecd330f2b028bf4678edd4f53b5489c5e6df.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 62a3c841730b97d..8ab7e065d218412 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -715,9 +715,6 @@ Bug Fixes in This Version
   ``await_suspend`` could be misoptimized, including accesses to the awaiter
   object itself.
   (`#56301 `_)
-  The current solution may bring performance regressions if the awaiters have
-  non-static data members. See
-  `#64945 `_ for details.
 - 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.



___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits