ChuanqiXu created this revision.
ChuanqiXu added reviewers: rjmccall, lxfind, aeubanks.
ChuanqiXu added a project: clang.
Herald added subscribers: ormris, steven_wu, hiraditya.
ChuanqiXu requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits.
Herald added a project: LLVM.
See https://reviews.llvm.org/D100282 and
https://github.com/llvm/llvm-project/issues/49264 for the background.
Simply, coroutine shouldn't be inlined before CoroSplit. And the marker for
pre-splited coroutine is created in CoroEarly pass, which ran after
AlwaysInliner Pass in O0 pipeline. So that the AlwaysInliner couldn't detect it
shouldn't inline a coroutine. So here is the error.
In D100282 <https://reviews.llvm.org/D100282>, we have discussed how should we
fix this bug. Should we move the CoroEarly pass in front of AlwaysInliner? Or
should we make it in the frontend. According to @lxfind , it would be better to
move the CoroEarly pass.
And I have another reason to not implement it in the frontend. Since I found
that if we make it in the frontend, we need to edit the `Coroutines.rst`
document to tell that the frontend should emit a `"coroutine.presplit"`
attribute (at least for switch-resumed ABI), it is weird. Since it is required
all the time, it should be done automatically in the middle end. So I think it
would be better to move the CoroEarly pass.
BTW, according to the discuss in D100282 <https://reviews.llvm.org/D100282>, it
is suggested that we should add a warning for marking a coroutine as
`always_inline`. I implement it in this patch. But I am willing to split it if
required.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D115790
Files:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCoroutine.cpp
clang/test/CodeGen/lto-newpm-pipeline.c
clang/test/CodeGenCoroutines/coro-always-inline.cpp
clang/test/SemaCXX/coroutines.cpp
llvm/lib/Passes/PassBuilderPipelines.cpp
llvm/test/Other/new-pass-manager.ll
llvm/test/Other/new-pm-O0-defaults.ll
Index: llvm/test/Other/new-pm-O0-defaults.ll
===================================================================
--- llvm/test/Other/new-pm-O0-defaults.ll
+++ llvm/test/Other/new-pm-O0-defaults.ll
@@ -31,14 +31,13 @@
; CHECK-DIS: Running analysis: InnerAnalysisManagerProxy
; CHECK-DIS-NEXT: Running pass: AddDiscriminatorsPass
+; CHECK-CORO: Running pass: CoroEarlyPass
; CHECK-DIS-NEXT: Running pass: AlwaysInlinerPass
; CHECK-DIS-NEXT: Running analysis: ProfileSummaryAnalysis
-; CHECK-DEFAULT: Running pass: AlwaysInlinerPass
-; CHECK-DEFAULT-NEXT: Running analysis: InnerAnalysisManagerProxy
+; CHECK-DEFAULT-NEXT: Running pass: AlwaysInlinerPass
; CHECK-DEFAULT-NEXT: Running analysis: ProfileSummaryAnalysis
; CHECK-MATRIX: Running pass: LowerMatrixIntrinsicsPass
; CHECK-MATRIX-NEXT: Running analysis: TargetIRAnalysis
-; CHECK-CORO-NEXT: Running pass: CoroEarlyPass
; CHECK-CORO-NEXT: Running analysis: InnerAnalysisManagerProxy
; CHECK-CORO-NEXT: Running analysis: LazyCallGraphAnalysis
; CHECK-CORO-NEXT: Running analysis: TargetLibraryAnalysis
Index: llvm/test/Other/new-pass-manager.ll
===================================================================
--- llvm/test/Other/new-pass-manager.ll
+++ llvm/test/Other/new-pass-manager.ll
@@ -292,8 +292,9 @@
; RUN: -passes='default<O0>' %s 2>&1 \
; RUN: | FileCheck %s --check-prefix=CHECK-O0 --check-prefix=%llvmcheckext
; CHECK-O0: Running pass: AlwaysInlinerPass
-; CHECK-O0-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}>
; CHECK-O0-NEXT: Running analysis: ProfileSummaryAnalysis
+; CHECK-O0-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}>
+; CHECK-O0-NEXT: Running analysis: LazyCallGraphAnalysis
; CHECK-EXT-NEXT: Running pass: {{.*}}Bye
; We don't have checks for CHECK-NOEXT here, but this simplifies the test, while
; avoiding FileCheck complaining about the unused prefix.
Index: llvm/lib/Passes/PassBuilderPipelines.cpp
===================================================================
--- llvm/lib/Passes/PassBuilderPipelines.cpp
+++ llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1717,6 +1717,8 @@
for (auto &C : PipelineEarlySimplificationEPCallbacks)
C(MPM, Level);
+ MPM.addPass(createModuleToFunctionPassAdaptor(CoroEarlyPass()));
+
// Build a minimal pipeline based on the semantics required by LLVM,
// which is just that always inlining occurs. Further, disable generating
// lifetime intrinsics to avoid enabling further optimizations during
@@ -1771,7 +1773,6 @@
MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
}
- MPM.addPass(createModuleToFunctionPassAdaptor(CoroEarlyPass()));
CGSCCPassManager CGPM;
CGPM.addPass(CoroSplitPass());
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
Index: clang/test/SemaCXX/coroutines.cpp
===================================================================
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -1443,3 +1443,7 @@
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 marked always_inline might not be inlined properly}}
+ co_await suspend_always{};
+}
Index: clang/test/CodeGenCoroutines/coro-always-inline.cpp
===================================================================
--- clang/test/CodeGenCoroutines/coro-always-inline.cpp
+++ clang/test/CodeGenCoroutines/coro-always-inline.cpp
@@ -48,3 +48,15 @@
// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::awaitable"* %ref.tmp{{.*}} to i8*
// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 1, i8* [[CAST3]])
void foo() { co_return; }
+
+// check that bar is not inlined even it's marked as always_inline
+
+// CHECK-LABEL: define {{.*}} void @_Z3bazv()
+// CHECK: call void @_Z3barv(
+__attribute__((__always_inline__)) void bar() {
+ co_return;
+}
+void baz() {
+ bar();
+ co_return;
+}
Index: clang/test/CodeGen/lto-newpm-pipeline.c
===================================================================
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -26,8 +26,9 @@
// RUN: -check-prefix=CHECK-THIN-OPTIMIZED
// CHECK-FULL-O0: Running pass: AlwaysInlinerPass
-// CHECK-FULL-O0-NEXT: Running analysis: InnerAnalysisManagerProxy
// CHECK-FULL-O0-NEXT: Running analysis: ProfileSummaryAnalysis
+// CHECK-FULL-O0-NEXT: Running analysis: InnerAnalysisManagerProxy
+// CHECK-FULL-O0-NEXT: Running analysis: LazyCallGraphAnalysis
// CHECK-FULL-O0: Running pass: CoroSplitPass
// CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
// CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
@@ -38,8 +39,9 @@
// CHECK-FULL-O0-NEXT: Running pass: BitcodeWriterPass
// CHECK-THIN-O0: Running pass: AlwaysInlinerPass
-// CHECK-THIN-O0-NEXT: Running analysis: InnerAnalysisManagerProxy
// CHECK-THIN-O0-NEXT: Running analysis: ProfileSummaryAnalysis
+// CHECK-THIN-O0-NEXT: Running analysis: InnerAnalysisManagerProxy
+// CHECK-THIN-O0-NEXT: Running analysis: LazyCallGraphAnalysis
// CHECK-THIN-O0: Running pass: CoroCleanupPass
// CHECK-THIN-O0-NEXT: Running pass: CanonicalizeAliasesPass
// CHECK-THIN-O0-NEXT: Running pass: NameAnonGlobalPass
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1062,6 +1062,12 @@
return;
}
+ // The coroutine marked always inline might not be inlined properly in current
+ // compiler support. Only the ramp function is guarantted to be inlined. 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);
+
// Coroutines [stmt.return]p1:
// A return statement shall not appear in a coroutine.
if (Fn->FirstReturnLoc.isValid()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11099,6 +11099,10 @@
def note_coroutine_function_declare_noexcept : Note<
"must be declared with 'noexcept'"
>;
+def warn_always_inline_coroutine : Warning<
+ "A coroutine marked always_inline might not be inlined properly."
+ >,
+ 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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
