Author: Chuanqi Xu Date: 2022-02-08T11:43:42+08:00 New Revision: e39ba0461757339f7172f6bc3882f41116bb7c13
URL: https://github.com/llvm/llvm-project/commit/e39ba0461757339f7172f6bc3882f41116bb7c13 DIFF: https://github.com/llvm/llvm-project/commit/e39ba0461757339f7172f6bc3882f41116bb7c13.diff LOG: [C++20] [Coroutines] Warning for always_inline coroutine See the discussion in https://reviews.llvm.org/D100282. 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. This is what GCC does too: https://godbolt.org/z/7eajb1Gf8 Reviewed By: Quuxplusone Differential Revision: https://reviews.llvm.org/D115867 Added: Modified: 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 Removed: ################################################################################ diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index e12a0b259cc2e..1682a71eb992e 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -6205,6 +6205,10 @@ optimization level. Does not guarantee that inline substitution actually occurs. +<ins>Note: applying this attribute to a coroutine at the `-O0` optimization level +has no effect; other optimization levels may only partially inline and result in a +diagnostic.</ins> + See also `the Microsoft Docs on Inline Functions`_, `the GCC Common Function Attribute docs`_, and `the GCC Inline docs`_. diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 608e16147b1cd..3f14fa32597d9 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -58,7 +58,9 @@ def DeprecatedExperimentalCoroutine : 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, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f9bfd343ac8d1..08cd8c8991ff9 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11122,6 +11122,10 @@ def err_coroutine_promise_final_suspend_requires_nothrow : Error< def note_coroutine_function_declare_noexcept : Note< "must be declared with 'noexcept'" >; +def warn_always_inline_coroutine : Warning< + "this coroutine may be split into pieces; not every piece is guaranteed to be inlined" + >, + InGroup<AlwaysInlineCoroutine>; } // end of coroutines issue category let CategoryName = "Documentation Issue" in { diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index cd3ae62ebbe26..aae90c403f0f2 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -1081,6 +1081,14 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { return; } + // The always_inline attribute doesn't reliably apply to a coroutine, + // because the coroutine will be split into pieces and some pieces + // might be called indirectly, as in a virtual call. Even the ramp + // function cannot be inlined at -O0, due to pipeline ordering + // problems (see https://llvm.org/PR53413). Tell the user about 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()) { diff --git a/clang/test/SemaCXX/coroutines.cpp b/clang/test/SemaCXX/coroutines.cpp index b461c881355b5..c81385bcc0595 100644 --- a/clang/test/SemaCXX/coroutines.cpp +++ b/clang/test/SemaCXX/coroutines.cpp @@ -1460,3 +1460,13 @@ void test_missing_awaitable_members() { 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 {{this coroutine may be split into pieces; not every piece is guaranteed to be inlined}} + co_await suspend_always{}; +} + +[[gnu::always_inline]] +void warn_gnu_always_inline() { // expected-warning {{this coroutine may be split into pieces; not every piece is guaranteed to be inlined}} + co_await suspend_always{}; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
