[Coroutines] Remove CoroElide from O0 pipeline

[lxfind]

Summary

CoroElide pass works only when a post-split coroutine is inlined into another post-split coroutine.
In O0, there is no inlining after CoroSplit, and hence no CoroElide can happen.
It's useless to put CoroElide pass in the O0 pipeline and it will never be triggered (unless I miss anything).

[lxfind]

update tests

[ChuanqiXu]

On O0, it is possible to inline if the user marked the function with always_inline.
Since CoroElide is kind of optimization, it should be OK to skip in O0.
Out of curiosity, what's the reason that you want to remove it?

[lxfind]

In D105066#2845958, @ChuanqiXu wrote:

On O0, it is possible to inline if the user marked the function with always_inline.
Since CoroElide is kind of optimization, it should be OK to skip in O0.
Out of curiosity, what's the reason that you want to remove it?

Coroutine functions cannot be inlined before splitting, even if it's marked "always_inline" (in fact, we should make it illegal to mark a coroutine "always_inline", because there is no guarantee that a coroutine can be fully inlined, GCC does that).

[ChuanqiXu]

In D105066#2845978, @lxfind wrote:

Coroutine functions cannot be inlined before splitting, even if it's marked "always_inline"

Yeah, but it may be inlined after splitting, which could trigger coro elide.

in fact, we should make it illegal to mark a coroutine "always_inline", because there is no guarantee that a coroutine can be fully inlined, GCC does that

To my understanding, it looks like that we shouldn't inline it since we couldn't inline all parts of the function. Is this what you want to say?
I think it may be a problem that we can't inline the full coroutine. But it's not the reason to forbid it.

---

Coro Elide is not defined in the standard (although it comes up in the proposal). So it should be a compiler optimization. In this way, it should be OK to remove it in O0.

[lxfind]

Yeah, but it may be inlined after splitting, which could trigger coro elide.

In O0, there is no inliner pass (after CoroSplit), so inlining should never happen.

in fact, we should make it illegal to mark a coroutine "always_inline", because there is no guarantee that a coroutine can be fully inlined, GCC does that

To my understanding, it looks like that we shouldn't inline it since we couldn't inline all parts of the function. Is this what you want to say?
I think it may be a problem that we can't inline the full coroutine. But it's not the reason to forbid it.

That's a separate topic though. Let's agree on this diff first and then I can explain more about the always_inline issue.

---

Coro Elide is not defined in the standard (although it comes up in the proposal). So it should be a compiler optimization. In this way, it should be OK to remove it in O0.

[ChuanqiXu]

That's a separate topic though. Let's agree on this diff first and then I can explain more about the always_inline issue.

Yeah, maybe we should discuss it on the llvm-dev thread.

clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp

 // Tests that coroutine passes are added to and run by the new pass manager
 // pipeline, at -O0 and above.
 
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null \
 // RUN:   -fexperimental-new-pass-manager -fdebug-pass-manager -fcoroutines-ts \
-// RUN:   -O0 %s 2>&1 | FileCheck %s
+// RUN:   -O0 %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null \
 // RUN:   -fexperimental-new-pass-manager -fdebug-pass-manager -fcoroutines-ts \
-// RUN:   -O1 %s 2>&1 | FileCheck %s
+// RUN:   -O1 %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 //
-// CHECK: Running pass:{{.*}}CoroEarlyPass
+// CHECK-ALL: Running pass:{{.*}}CoroEarlyPass
 //
 // The first coro-split pass enqueues a second run of the entire CGSCC pipeline.
-// CHECK: Running pass: CoroSplitPass on (_Z3foov)
-// CHECK: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
+// CHECK-ALL: Running pass: CoroSplitPass on (_Z3foov)
+// CHECK-OPT: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
 //
 // The second coro-split pass splits coroutine 'foo' into funclets
 // 'foo.resume', 'foo.destroy', and 'foo.cleanup'.
-// CHECK: Running pass: CoroSplitPass on (_Z3foov)
-// CHECK: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
+// CHECK-ALL: Running pass: CoroSplitPass on (_Z3foov)
+// CHECK-OPT: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
 //
-// CHECK: Running pass:{{.*}}CoroCleanupPass
+// CHECK-ALL: Running pass:{{.*}}CoroCleanupPass
 
 namespace std {
 namespace experimental {
 
 struct handle {};
 
 struct awaitable {
   bool await_ready() noexcept { return true; }

llvm/lib/Passes/PassBuilder.cpp

if (!FPM.isEmpty())
       MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   }
 
   if (PTO.Coroutines) {
     MPM.addPass(createModuleToFunctionPassAdaptor(CoroEarlyPass()));
 
     CGSCCPassManager CGPM;
     CGPM.addPass(CoroSplitPass());
-    CGPM.addPass(createCGSCCToFunctionPassAdaptor(CoroElidePass()));
     MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
 
     MPM.addPass(createModuleToFunctionPassAdaptor(CoroCleanupPass()));
   }
 
   for (auto &C : OptimizerLastEPCallbacks)
     C(MPM, Level);
 

llvm/test/Transforms/Coroutines/smoketest.ll

 ; Test that all coroutine passes run in the correct order at all optimization
 ; levels and -enable-coroutines adds coroutine passes to the pipeline.
 ;
 ; RUN: opt < %s -disable-output -passes='default<O0>' -enable-coroutines \
-; RUN:     -debug-pass-manager 2>&1 | FileCheck %s
+; RUN:     -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL
 ; RUN: opt < %s -disable-output -passes='default<O1>' -enable-coroutines \
-; RUN:     -debug-pass-manager 2>&1 | FileCheck %s
+; RUN:     -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -passes='default<O2>' -enable-coroutines \
-; RUN:     -debug-pass-manager 2>&1 | FileCheck %s
+; RUN:     -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -passes='default<O3>' -enable-coroutines \
-; RUN:     -debug-pass-manager 2>&1 | FileCheck %s
+; RUN:     -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -debug-pass-manager \
 ; RUN:     -passes='function(coro-early),cgscc(coro-split),function(coro-elide,coro-cleanup)' 2>&1 \
-; RUN:     | FileCheck %s
+; RUN:     | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 
-; CHECK: CoroEarlyPass
-; CHECK: CoroSplitPass
-; CHECK: CoroElidePass
-; CHECK: CoroCleanupPass
+; CHECK-ALL: CoroEarlyPass
+; CHECK-ALL: CoroSplitPass
+; CHECK-OPT: CoroElidePass
+; CHECK-ALL: CoroCleanupPass
 
 define void @foo() {
   ret void
 }