ChuanqiXu updated this revision to Diff 403121.
ChuanqiXu added a comment.
Address comments
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 {{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{};
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1081,6 +1081,14 @@
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()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11110,6 +11110,10 @@
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 {
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
@@ -6202,6 +6202,10 @@
Does not guarantee that inline substitution actually occurs.
+A coroutine function will not be inlined at -O0, even if it is marked with
this attribute.
+At higher optimization levels, only the first part of the coroutine - the
"ramp function"<https://llvm.org/docs/Coroutines.html>`_
+is guaranteed to be inlined.
+
See also `the Microsoft Docs on Inline Functions`_, `the GCC Common Function
Attribute docs`_, and `the GCC Inline docs`_.
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 {{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{};
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1081,6 +1081,14 @@
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()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11110,6 +11110,10 @@
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 {
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
@@ -6202,6 +6202,10 @@
Does not guarantee that inline substitution actually occurs.
+A coroutine function will not be inlined at -O0, even if it is marked with this attribute.
+At higher optimization levels, only the first part of the coroutine - the "ramp function"<https://llvm.org/docs/Coroutines.html>`_
+is guaranteed to be inlined.
+
See also `the Microsoft Docs on Inline Functions`_, `the GCC Common Function
Attribute docs`_, and `the GCC Inline docs`_.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
