[NFC] [Pipelines] Hoist CoroCleanup as Module Pass (3/5)

[ChuanqiXu]

Summary

This is similar to previous patch https://reviews.llvm.org/D123925. It could also reduce the time we call declaresCoroCleanupIntrinsics. And it is helpful for further changes.

[aeubanks]

llvm/lib/Transforms/Coroutines/CoroCleanup.cpp

57

why are we removing Changed and always invalidating analyses? if we have a module where most functions don't call any of these intrinsics, we're going to unnecessarily invalidate a lot more analyses which will be bad for compile times.

[ChuanqiXu]

llvm/lib/Transforms/Coroutines/CoroCleanup.cpp

57

My thought is:
(1) We already checked if there is any coroutine intrinsic in the module before we start the pass.
(2) So there must be some changes for some functions.
(3) So the Changed variable would always be true. So I think it is redundant.

Here is the case that M owns the declaration of some coroutine intrinsics but not a function uses it actually. But I think it wouldn't happen in real cases.

[aeubanks]

llvm/lib/Transforms/Coroutines/CoroCleanup.cpp

57

oh sorry, I forgot this was now a module pass

llvm/include/llvm/Transforms/Coroutines/CoroCleanup.h

 
 #ifndef LLVM_TRANSFORMS_COROUTINES_COROCLEANUP_H
 #define LLVM_TRANSFORMS_COROUTINES_COROCLEANUP_H
 
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
 
-class Function;
+class Module;
 
 struct CoroCleanupPass : PassInfoMixin<CoroCleanupPass> {
-  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+  PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM);
   static bool isRequired() { return true; }
 };
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_COROUTINES_COROCLEANUP_H

llvm/lib/Passes/PassBuilderPipelines.cpp

PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   if (EnableSyntheticCounts && !PGOOpt)
     MPM.addPass(SyntheticCountsPropagation());
 
   if (EnableModuleInliner)
     MPM.addPass(buildModuleInlinerPipeline(Level, Phase));
   else
     MPM.addPass(buildInlinerPipeline(Level, Phase));
 
-  MPM.addPass(createModuleToFunctionPassAdaptor(CoroCleanupPass()));
+  MPM.addPass(CoroCleanupPass());
 
   if (EnableMemProfiler && Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
     MPM.addPass(createModuleToFunctionPassAdaptor(MemProfilerPass()));
     MPM.addPass(ModuleMemProfilerPass());
   }
 
   return MPM;
 }
if (!FPM.isEmpty())
       MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   }
 
   ModulePassManager CoroPM;
   CoroPM.addPass(CoroEarlyPass());
   CGSCCPassManager CGPM;
   CGPM.addPass(CoroSplitPass());
   CoroPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
-  CoroPM.addPass(createModuleToFunctionPassAdaptor(CoroCleanupPass()));
+  CoroPM.addPass(CoroCleanupPass());
   CoroPM.addPass(GlobalDCEPass());
   MPM.addPass(CoroConditionalWrapper(std::move(CoroPM)));
 
   for (auto &C : OptimizerLastEPCallbacks)
     C(MPM, Level);
 
   if (LTOPreLink)
     addRequiredLTOPreLinkPasses(MPM);

llvm/lib/Passes/PassRegistry.def

 MODULE_PASS("annotation2metadata", Annotation2MetadataPass())
 MODULE_PASS("openmp-opt", OpenMPOptPass())
 MODULE_PASS("called-value-propagation", CalledValuePropagationPass())
 MODULE_PASS("canonicalize-aliases", CanonicalizeAliasesPass())
 MODULE_PASS("cg-profile", CGProfilePass())
 MODULE_PASS("check-debugify", NewPMCheckDebugifyPass())
 MODULE_PASS("constmerge", ConstantMergePass())
 MODULE_PASS("coro-early", CoroEarlyPass())
+MODULE_PASS("coro-cleanup", CoroCleanupPass())
 MODULE_PASS("cross-dso-cfi", CrossDSOCFIPass())
 MODULE_PASS("deadargelim", DeadArgumentEliminationPass())
 MODULE_PASS("debugify", NewPMDebugifyPass())
 MODULE_PASS("dot-callgraph", CallGraphDOTPrinterPass())
 MODULE_PASS("elim-avail-extern", EliminateAvailableExternallyPass())
 MODULE_PASS("extract-blocks", BlockExtractorPass())
 MODULE_PASS("forceattrs", ForceFunctionAttrsPass())
 MODULE_PASS("function-import", FunctionImportPass())

 FUNCTION_PASS("bdce", BDCEPass())
 FUNCTION_PASS("bounds-checking", BoundsCheckingPass())
 FUNCTION_PASS("break-crit-edges", BreakCriticalEdgesPass())
 FUNCTION_PASS("callsite-splitting", CallSiteSplittingPass())
 FUNCTION_PASS("consthoist", ConstantHoistingPass())
 FUNCTION_PASS("constraint-elimination", ConstraintEliminationPass())
 FUNCTION_PASS("chr", ControlHeightReductionPass())
 FUNCTION_PASS("coro-elide", CoroElidePass())
-FUNCTION_PASS("coro-cleanup", CoroCleanupPass())
 FUNCTION_PASS("correlated-propagation", CorrelatedValuePropagationPass())
 FUNCTION_PASS("dce", DCEPass())
 FUNCTION_PASS("dfa-jump-threading", DFAJumpThreadingPass())
 FUNCTION_PASS("div-rem-pairs", DivRemPairsPass())
 FUNCTION_PASS("dse", DSEPass())
 FUNCTION_PASS("dot-cfg", CFGPrinterPass())
 FUNCTION_PASS("dot-cfg-only", CFGOnlyPrinterPass())
 FUNCTION_PASS("dot-dom", DomTreePrinterPass())

llvm/lib/Transforms/Coroutines/CoroCleanup.cpp

 
 #define DEBUG_TYPE "coro-cleanup"
 
 namespace {
 // Created on demand if CoroCleanup pass has work to do.
 struct Lowerer : coro::LowererBase {
   IRBuilder<> Builder;
   Lowerer(Module &M) : LowererBase(M), Builder(Context) {}
-  bool lowerRemainingCoroIntrinsics(Function &F);
+  void lower(Function &F);
 };
 }
 
 static void simplifyCFG(Function &F) {
   llvm::legacy::FunctionPassManager FPM(F.getParent());
   FPM.add(createCFGSimplificationPass());
 
   FPM.doInitialization();
static void lowerSubFn(IRBuilder<> &Builder, CoroSubFnInst *SubFn) {
   Builder.SetInsertPoint(SubFn);
   auto *FramePtr = Builder.CreateBitCast(FrameRaw, FramePtrTy);
   auto *Gep = Builder.CreateConstInBoundsGEP2_32(FrameTy, FramePtr, 0, Index);
   auto *Load = Builder.CreateLoad(FrameTy->getElementType(Index), Gep);
 
   SubFn->replaceAllUsesWith(Load);
 }
 
-bool Lowerer::lowerRemainingCoroIntrinsics(Function &F) {
-  bool Changed = false;
-
+void Lowerer::lower(Function &F) {
   bool IsPrivateAndUnprocessed =
       F.hasFnAttribute(CORO_PRESPLIT_ATTR) && F.hasLocalLinkage();
 
   for (Instruction &I : llvm::make_early_inc_range(instructions(F))) {
     if (auto *II = dyn_cast<IntrinsicInst>(&I)) {
       switch (II->getIntrinsicID()) {
       default:
         continue;
if (auto *II = dyn_cast<IntrinsicInst>(&I)) {
         }
         auto *TargetRelativeFunOffset = Target->getOperand(0);
         auto *NewFuncPtrStruct = ConstantStruct::get(
             Target->getType(), TargetRelativeFunOffset, SourceSize);
         Target->replaceAllUsesWith(NewFuncPtrStruct);
         break;
       }
       II->eraseFromParent();
-      Changed = true;
     }
   }
 
-  if (Changed) {
-    // After replacement were made we can cleanup the function body a little.
-    simplifyCFG(F);
-  }
 
-  return Changed;
-}
-
 static bool declaresCoroCleanupIntrinsics(const Module &M) {
   return coro::declaresIntrinsics(
       M, {"llvm.coro.alloc", "llvm.coro.begin", "llvm.coro.subfn.addr",
           "llvm.coro.free", "llvm.coro.id", "llvm.coro.id.retcon",
           "llvm.coro.id.retcon.once", "llvm.coro.async.size.replace",
           "llvm.coro.async.resume"});
 }
 
-PreservedAnalyses CoroCleanupPass::run(Function &F,
-                                       FunctionAnalysisManager &AM) {
-  auto &M = *F.getParent();
-  if (!declaresCoroCleanupIntrinsics(M) ||
-      !Lowerer(M).lowerRemainingCoroIntrinsics(F))
+PreservedAnalyses CoroCleanupPass::run(Module &M,
+                                       ModuleAnalysisManager &MAM) {
+  if (!declaresCoroCleanupIntrinsics(M))
     return PreservedAnalyses::all();
 
+  Lowerer L(M);
+  for (auto &F : M)
+    L.lower(F);
+
   return PreservedAnalyses::none();
 }

llvm/test/Transforms/Coroutines/smoketest.ll

 ; Test that all coroutine passes run in the correct order at all optimization
 ; levels adds coroutine passes to the pipeline.
 ;
 ; RUN: opt < %s -disable-output -passes='default<O0>' \
 ; RUN:     -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL
 ; RUN: opt < %s -disable-output -passes='default<O1>' \
 ; RUN:     -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -passes='default<O2>' \
 ; RUN:     -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -passes='default<O3>' \
 ; RUN:     -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -debug-pass-manager \
-; RUN:     -passes='module(coro-early),function(coro-elide),cgscc(coro-split),function(coro-cleanup)' 2>&1 \
+; RUN:     -passes='module(coro-early),function(coro-elide),cgscc(coro-split),module(coro-cleanup)' 2>&1 \
 ; RUN:     | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 
 ; note that we run CoroElidePass before CoroSplitPass. This is because CoroElidePass is part of
 ; function simplification pipeline, which runs before CoroSplitPass. And since @foo is not
 ; a coroutine, it won't be put back into the CGSCC, and hence won't trigger a CoroElidePass
 ; after CoroSplitPass.
 ; CHECK-ALL: CoroEarlyPass
 ; CHECK-OPT: CoroElidePass
 ; CHECK-ALL: CoroSplitPass
 ; CHECK-ALL: CoroCleanupPass
 
 declare token @llvm.coro.id(i32, i8*, i8*, i8*)
 
 define void @foo() {
   ret void
 }