[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 397835.
ChuanqiXu added a comment.
Herald added a reviewer: aaron.ballman.

Address comments:

- Update warning message as Mathias's suggestion.
- Update document.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115867/new/

https://reviews.llvm.org/D115867

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/SemaCXX/coroutines.cpp


Index: clang/test/SemaCXX/coroutines.cpp
===================================================================
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -1460,3 +1460,13 @@
   co_await missing_await_suspend{}; // expected-error {{no member named 'await_suspend' in 'missing_await_suspend'}}
   co_await missing_await_resume{}; // expected-error {{no member named 'await_resume' in 'missing_await_resume'}}
 }
+
+__attribute__((__always_inline__))
+void warn_always_inline() { // expected-warning {{a coroutine may be split into pieces and not every piece of the coroutine can be guaranteed to be inlined}}
+  co_await suspend_always{};
+}
+
+[[gnu::always_inline]]
+void warn_gnu_always_inline() { // expected-warning {{a coroutine may be split into pieces and not every piece of the coroutine can be guaranteed to be inlined}}
+  co_await suspend_always{};
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1081,6 +1081,15 @@
     return;
   }
 
+  // The coroutine marked always inline might not be inlined properly in current
+  // compiler support. Since the coroutine would be splitted into pieces. And
+  // the call to resume() and destroy() functions might be indirect call. Also
+  // the ramp function wouldn't get inlined under O0 due to pipeline ordering
+  // problems. It might be different to what users expects to. Emit a warning to
+  // tell it.
+  if (FD->hasAttr<AlwaysInlineAttr>())
+    Diag(FD->getLocation(), diag::warn_always_inline_coroutine);
+
   // [stmt.return.coroutine]p1:
   //   A coroutine shall not enclose a return statement ([stmt.return]).
   if (Fn->FirstReturnLoc.isValid()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11105,6 +11105,10 @@
 def note_coroutine_function_declare_noexcept : Note<
   "must be declared with 'noexcept'"
 >;
+def warn_always_inline_coroutine : Warning<
+  "a coroutine may be split into pieces and not every piece of the coroutine can be guaranteed to be inlined"
+  >,
+  InGroup<AlwaysInlineCoroutine>;
 } // end of coroutines issue category
 
 let CategoryName = "Documentation Issue" in {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -58,7 +58,9 @@
   DiagGroup<"deprecated-experimental-coroutine">;
 def DeprecatedCoroutine :
   DiagGroup<"deprecated-coroutine", [DeprecatedExperimentalCoroutine]>;
-def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException, DeprecatedCoroutine]>;
+def AlwaysInlineCoroutine :
+  DiagGroup<"always-inline-coroutine">;
+def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException, DeprecatedCoroutine, AlwaysInlineCoroutine]>;
 def ObjCBoolConstantConversion : DiagGroup<"objc-bool-constant-conversion">;
 def ConstantConversion : DiagGroup<"constant-conversion",
                                    [BitFieldConstantConversion,
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -6137,6 +6137,10 @@
 
 Does not guarantee that inline substitution actually occurs.
 
+Note that a coroutine function wouldn't get inlined in O0 if it is marked with "always_inline".
+In other optimization levels, only the first part of the coroutine - "ramp function"<https://llvm.org/docs/Coroutines.html>`_
+can be guaranteed to be inlined.
+
 See also `the Microsoft Docs on Inline Functions`_, `the GCC Common Function
 Attribute docs`_, and `the GCC Inline docs`_.
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115867.397835.patch
Type: text/x-patch
Size: 3986 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220106/43086937/attachment.bin>