[LTO][Pipelines][Coro] De-duplicate Coro passes

[vitalybuka]

if (!isLTOPostLink(Phase))
    CoroPM.addPass(CoroEarlyPass());
if (!isLTOPreLink(Phase))
    // Other Coro passes

Followup to #126168.

[llvmbot]

@llvm/pr-subscribers-lto

Author: Vitaly Buka (vitalybuka)

Changes

if (!isLTOPostLink(Phase))
    CoroPM.addPass(CoroEarlyPass());
if (!isLTOPreLink(Phase))
    // Other Coro passes

Followup to #126168.

---

Full diff: https://github.com/llvm/llvm-project/pull/128654.diff

4 Files Affected:

• (modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+14-13)
• (modified) llvm/test/LTO/X86/coro.ll (+6-2)
• (modified) llvm/test/Other/new-pm-defaults.ll (+3-3)
• (modified) llvm/test/Other/new-pm-lto-defaults.ll (-1)

diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 6ca2f90aa0668..38f4ff5fab7e1 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -421,12 +421,15 @@ static bool isLTOPostLink(ThinOrFullLTOPhase Phase) {
 static CoroConditionalWrapper buildCoroWrapper(ThinOrFullLTOPhase Phase) {
   // TODO: Skip passes according to Phase.
   ModulePassManager CoroPM;
-  CoroPM.addPass(CoroEarlyPass());
-  CGSCCPassManager CGPM;
-  CGPM.addPass(CoroSplitPass());
-  CoroPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
-  CoroPM.addPass(CoroCleanupPass());
-  CoroPM.addPass(GlobalDCEPass());
+  if (!isLTOPostLink(Phase))
+    CoroPM.addPass(CoroEarlyPass());
+  if (!isLTOPreLink(Phase)) {
+    CGSCCPassManager CGPM;
+    CGPM.addPass(CoroSplitPass());
+    CoroPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
+    CoroPM.addPass(CoroCleanupPass());
+    CoroPM.addPass(GlobalDCEPass());
+  }
   return CoroConditionalWrapper(std::move(CoroPM));
 }
 
@@ -1010,7 +1013,7 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
   MainCGPipeline.addPass(createCGSCCToFunctionPassAdaptor(
       RequireAnalysisPass<ShouldNotRunFunctionPassesAnalysis, Function>()));
 
-  if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
+  if (!isLTOPreLink(Phase)) {
     MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));
     MainCGPipeline.addPass(CoroAnnotationElidePass());
   }
@@ -1060,7 +1063,7 @@ PassBuilder::buildModuleInlinerPipeline(OptimizationLevel Level,
       buildFunctionSimplificationPipeline(Level, Phase),
       PTO.EagerlyInvalidateAnalyses));
 
-  if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
+  if (!isLTOPreLink(Phase)) {
     MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(
         CoroSplitPass(Level != OptimizationLevel::O0)));
     MPM.addPass(
@@ -1120,7 +1123,8 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
     // Do basic inference of function attributes from known properties of system
     // libraries and other oracles.
     MPM.addPass(InferFunctionAttrsPass());
-    MPM.addPass(CoroEarlyPass());
+    if (!isLTOPostLink(Phase))
+      MPM.addPass(CoroEarlyPass());
 
     FunctionPassManager EarlyFPM;
     EarlyFPM.addPass(EntryExitInstrumenterPass(/*PostInlining=*/false));
@@ -1283,7 +1287,7 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // and argument promotion.
   MPM.addPass(DeadArgumentEliminationPass());
 
-  if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink)
+  if (!isLTOPreLink(Phase))
     MPM.addPass(CoroCleanupPass());
 
   // Optimize globals now that functions are fully simplified.
@@ -1948,9 +1952,6 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
     return MPM;
   }
 
-  // TODO: Skip to match buildCoroWrapper.
-  MPM.addPass(CoroEarlyPass());
-
   // Optimize globals to try and fold them into constants.
   MPM.addPass(GlobalOptPass());
 
diff --git a/llvm/test/LTO/X86/coro.ll b/llvm/test/LTO/X86/coro.ll
index cde398dd76d85..f9830d964bc69 100644
--- a/llvm/test/LTO/X86/coro.ll
+++ b/llvm/test/LTO/X86/coro.ll
@@ -1,4 +1,6 @@
-; RUN: llvm-as %s -o %t1.bc
+; RUN: opt %s -passes='lto-pre-link<O0>' -S -o %t1.ll
+; RUN: FileCheck %s --check-prefixes=CHECK,PRELINK --implicit-check-not="call void @llvm.coro" --input-file=%t1.ll
+; RUN: llvm-as %t1.ll -o %t1.bc
 ; RUN: llvm-lto2 run %t1.bc -o %t2.o -r=%t1.bc,test,plx -r=%t1.bc,extern_func,plx -save-temps
 ; RUN: llvm-dis %t2.o.0.5.precodegen.bc -o - | FileCheck %s --implicit-check-not="call void @llvm.coro"
 
@@ -7,7 +9,9 @@ target triple = "x86_64-unknown-fuchsia"
 
 declare void @extern_func()
 
-; CHECK:      define {{.*}} void @test(
+; CHECK: define{{.*}} void @test(
+; PRELINK: call ptr @llvm.coro.subfn.addr
+; PRELINK: call ptr @llvm.coro.subfn.addr
 define void @test(ptr %hdl) {
   call void @llvm.coro.resume(ptr %hdl)
   call void @llvm.coro.destroy(ptr %hdl)
diff --git a/llvm/test/Other/new-pm-defaults.ll b/llvm/test/Other/new-pm-defaults.ll
index c554fdbf4c799..30ff1a5879df2 100644
--- a/llvm/test/Other/new-pm-defaults.ll
+++ b/llvm/test/Other/new-pm-defaults.ll
@@ -230,13 +230,13 @@
 ; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass
 ; CHECK-O-NEXT: Running pass: RequireAnalysisPass<{{.*}}ShouldNotRunFunctionPassesAnalysis
 ; CHECK-O-NEXT: Running analysis: ShouldNotRunFunctionPassesAnalysis
-; CHECK-O-NEXT: Running pass: CoroSplitPass
-; CHECK-O-NEXT: Running pass: CoroAnnotationElidePass
+; CHECK-DEFAULT-NEXT: Running pass: CoroSplitPass
+; CHECK-DEFAULT-NEXT: Running pass: CoroAnnotationElidePass
 ; CHECK-O-NEXT: Running pass: InvalidateAnalysisPass<{{.*}}ShouldNotRunFunctionPassesAnalysis
 ; CHECK-O-NEXT: Invalidating analysis: ShouldNotRunFunctionPassesAnalysis
 ; CHECK-O-NEXT: Invalidating analysis: InlineAdvisorAnalysis
 ; CHECK-O-NEXT: Running pass: DeadArgumentEliminationPass
-; CHECK-O-NEXT: Running pass: CoroCleanupPass
+; CHECK-DEFAULT-NEXT: Running pass: CoroCleanupPass
 ; CHECK-O-NEXT: Running pass: GlobalOptPass
 ; CHECK-O-NEXT: Running pass: GlobalDCEPass
 ; CHECK-DEFAULT-NEXT: Running pass: EliminateAvailableExternallyPass
diff --git a/llvm/test/Other/new-pm-lto-defaults.ll b/llvm/test/Other/new-pm-lto-defaults.ll
index 3aea0f2061f3e..e4320bb619533 100644
--- a/llvm/test/Other/new-pm-lto-defaults.ll
+++ b/llvm/test/Other/new-pm-lto-defaults.ll
@@ -67,7 +67,6 @@
 ; CHECK-O1-NEXT: Running analysis: TargetLibraryAnalysis
 ; CHECK-O-NEXT: Running pass: GlobalSplitPass
 ; CHECK-O-NEXT: Running pass: WholeProgramDevirtPass
-; CHECK-O23SZ-NEXT: Running pass: CoroEarlyPass
 ; CHECK-O1-NEXT: Running pass: LowerTypeTestsPass
 ; CHECK-O23SZ-NEXT: Running pass: GlobalOptPass
 ; CHECK-O23SZ-NEXT: Running pass: PromotePass

[thevinster]

@vitalybuka @aeubanks This change appears to be breaking builds that use FatLTO. See godbolt for a reproducer. When -ffat-lto-objects isn't enabled, the compilation works. Is it possible that the de-duplication has removed critical passes ran during the FatLTO pipeline?

[vitalybuka]

@vitalybuka @aeubanks This change appears to be breaking builds that use FatLTO. See godbolt for a reproducer. When -ffat-lto-objects isn't enabled, the compilation works. Is it possible that the de-duplication has removed critical passes ran during the FatLTO pipeline?

Sorry, reverting.
Thanks for report!