[llvm] [LLVM][Coroutines] Switch CoroAnnotationElidePass to a FunctionPass (PR #107897)

via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 9 11:29:52 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-coroutines

Author: Yuxuan Chen (yuxuanchen1997)

<details>
<summary>Changes</summary>

After landing https://github.com/llvm/llvm-project/pull/99285 we found that the call graph update was causing the following crash when expensive checks are turned on
```
llvm-project/llvm/lib/Analysis/CGSCCPassManager.cpp:982: LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(LazyCallGraph &, LazyCallGraph::SCC &, LazyCallGraph::Node &, CGSCCAnalysisManager &, CGSCCUpdateResult &, FunctionAnalysisManager &, bool): Assertion `(RC == &TargetRC || RC->isAncestorOf(Targe
tRC)) && "New call edge is not trivial!"' failed.                                                                                                                                                                                                                                                                               
```
This is because the call graph updater doesn't support inserting new edges yet. 

After reading the code in `CGSCCToFunctionPassAdaptor`, I am convinced that `CoroAnnotationElidePass` can be a FunctionPass and rely on the adaptor to update the call graph for us, so long as we properly invalidate the caller's analyses. 

After this patch, `llvm/test/Transforms/Coroutines/coro-transform-must-elide.ll` no longer fails under expensive checks. 

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


4 Files Affected:

- (modified) llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h (+3-7) 
- (modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+3-3) 
- (modified) llvm/lib/Passes/PassRegistry.def (+1-1) 
- (modified) llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp (+15-28) 


``````````diff
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h b/llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h
index 352c9e14526697..986a5dbd1ed0fe 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h
@@ -17,18 +17,14 @@
 #ifndef LLVM_TRANSFORMS_COROUTINES_COROANNOTATIONELIDE_H
 #define LLVM_TRANSFORMS_COROUTINES_COROANNOTATIONELIDE_H
 
-#include "llvm/Analysis/CGSCCPassManager.h"
-#include "llvm/Analysis/LazyCallGraph.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
 
-struct CoroAnnotationElidePass : PassInfoMixin<CoroAnnotationElidePass> {
-  CoroAnnotationElidePass() {}
-
-  PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM,
-                        LazyCallGraph &CG, CGSCCUpdateResult &UR);
+class Function;
 
+struct CoroAnnotationElidePass : PassInfoMixin<CoroAnnotationElidePass> {
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
   static bool isRequired() { return false; }
 };
 } // end namespace llvm
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 4e8e3dcdff4428..b063caa5279afa 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -976,7 +976,8 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
 
   if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
     MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));
-    MainCGPipeline.addPass(CoroAnnotationElidePass());
+    MainCGPipeline.addPass(
+        createCGSCCToFunctionPassAdaptor(CoroAnnotationElidePass()));
   }
 
   // Make sure we don't affect potential future NoRerun CGSCC adaptors.
@@ -1022,8 +1023,7 @@ PassBuilder::buildModuleInlinerPipeline(OptimizationLevel Level,
   if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
     MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(
         CoroSplitPass(Level != OptimizationLevel::O0)));
-    MPM.addPass(
-        createModuleToPostOrderCGSCCPassAdaptor(CoroAnnotationElidePass()));
+    MPM.addPass(createModuleToFunctionPassAdaptor(CoroAnnotationElidePass()));
   }
 
   return MPM;
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 4f5f680a6e9535..3dc7f185f330c5 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -243,7 +243,6 @@ CGSCC_PASS("attributor-light-cgscc", AttributorLightCGSCCPass())
 CGSCC_PASS("invalidate<all>", InvalidateAllAnalysesPass())
 CGSCC_PASS("no-op-cgscc", NoOpCGSCCPass())
 CGSCC_PASS("openmp-opt-cgscc", OpenMPOptCGSCCPass())
-CGSCC_PASS("coro-annotation-elide", CoroAnnotationElidePass())
 #undef CGSCC_PASS
 
 #ifndef CGSCC_PASS_WITH_PARAMS
@@ -343,6 +342,7 @@ FUNCTION_PASS("codegenprepare", CodeGenPreparePass(TM))
 FUNCTION_PASS("consthoist", ConstantHoistingPass())
 FUNCTION_PASS("constraint-elimination", ConstraintEliminationPass())
 FUNCTION_PASS("coro-elide", CoroElidePass())
+FUNCTION_PASS("coro-annotation-elide", CoroAnnotationElidePass())
 FUNCTION_PASS("correlated-propagation", CorrelatedValuePropagationPass())
 FUNCTION_PASS("count-visits", CountVisitsPass())
 FUNCTION_PASS("dce", DCEPass())
diff --git a/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp b/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
index 27a370a5d8fbf9..c3f258eb083dae 100644
--- a/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
@@ -16,7 +16,6 @@
 
 #include "llvm/Transforms/Coroutines/CoroAnnotationElide.h"
 
-#include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/LazyCallGraph.h"
 #include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/IR/Analysis.h"
@@ -96,24 +95,15 @@ static void processCall(CallBase *CB, Function *Caller, Function *NewCallee,
   CB->eraseFromParent();
 }
 
-PreservedAnalyses CoroAnnotationElidePass::run(LazyCallGraph::SCC &C,
-                                               CGSCCAnalysisManager &AM,
-                                               LazyCallGraph &CG,
-                                               CGSCCUpdateResult &UR) {
+PreservedAnalyses CoroAnnotationElidePass::run(Function &F,
+                                               FunctionAnalysisManager &FAM) {
   bool Changed = false;
-  CallGraphUpdater CGUpdater;
-  CGUpdater.initialize(CG, C, AM, UR);
-
-  auto &FAM =
-      AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, CG).getManager();
-
-  for (LazyCallGraph::Node &N : C) {
-    Function *Callee = &N.getFunction();
-    Function *NewCallee = Callee->getParent()->getFunction(
-        (Callee->getName() + ".noalloc").str());
-    if (!NewCallee) {
-      continue;
-    }
+
+  Function *NewCallee =
+      F.getParent()->getFunction((F.getName() + ".noalloc").str());
+  if (!NewCallee) {
+    return PreservedAnalyses::all();
+  }
 
     auto FramePtrArgPosition = NewCallee->arg_size() - 1;
     auto FrameSize =
@@ -122,34 +112,31 @@ PreservedAnalyses CoroAnnotationElidePass::run(LazyCallGraph::SCC &C,
         NewCallee->getParamAlign(FramePtrArgPosition).valueOrOne();
 
     SmallVector<CallBase *, 4> Users;
-    for (auto *U : Callee->users()) {
+    for (auto *U : F.users()) {
       if (auto *CB = dyn_cast<CallBase>(U)) {
-        if (CB->getCalledFunction() == Callee)
+        if (CB->getCalledFunction() == &F)
           Users.push_back(CB);
       }
     }
 
-    auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(*Callee);
+    auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
 
     for (auto *CB : Users) {
       auto *Caller = CB->getFunction();
       if (Caller && Caller->isPresplitCoroutine() &&
           CB->hasFnAttr(llvm::Attribute::CoroElideSafe)) {
 
-        auto *CallerN = CG.lookup(*Caller);
-        auto *CallerC = CG.lookupSCC(*CallerN);
         processCall(CB, Caller, NewCallee, FrameSize, FrameAlign);
 
         ORE.emit([&]() {
           return OptimizationRemark(DEBUG_TYPE, "CoroAnnotationElide", Caller)
-                 << "'" << ore::NV("callee", Callee->getName())
-                 << "' elided in '" << ore::NV("caller", Caller->getName());
+                 << "'" << ore::NV("callee", F.getName()) << "' elided in '"
+                 << ore::NV("caller", Caller->getName());
         });
+        FAM.invalidate(*Caller, PreservedAnalyses::none());
         Changed = true;
-        updateCGAndAnalysisManagerForCGSCCPass(CG, *CallerC, *CallerN, AM, UR,
-                                               FAM);
       }
     }
-  }
+
   return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/107897


More information about the llvm-commits mailing list