Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlgo][coro] Assign coro split-ed functions a FunctionLevel #68263

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Oct 4, 2023

The coroutine handle methods needed to be added to the FunctionLevels. We already had logic for their appearing between calls into the Inliner pass.

Fixes Issue #62616

The coroutine handle methods needed to be added to the `FunctionLevels`. We already had logic for their appearing between calls into the `Inliner` pass.
@llvmbot
Copy link
Member

llvmbot commented Oct 4, 2023

@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-mlgo

@llvm/pr-subscribers-llvm-transforms

Changes

The coroutine handle methods needed to be added to the FunctionLevels. We already had logic for their appearing between calls into the Inliner pass.


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/MLInlineAdvisor.cpp (+13-2)
  • (added) llvm/test/Transforms/Inline/ML/coro-split-func-levels.ll (+42)
diff --git a/llvm/lib/Analysis/MLInlineAdvisor.cpp b/llvm/lib/Analysis/MLInlineAdvisor.cpp
index 0660a9993b6dd7b..75eb8ece2e447e2 100644
--- a/llvm/lib/Analysis/MLInlineAdvisor.cpp
+++ b/llvm/lib/Analysis/MLInlineAdvisor.cpp
@@ -192,7 +192,9 @@ void MLInlineAdvisor::onPassEntry(LazyCallGraph::SCC *LastSCC) {
   // - in addition, if new Nodes were created by a pass (e.g. CoroSplit),
   // they'd be adjacent to Nodes in the last SCC. So we just need to check the
   // boundary of Nodes in NodesInLastSCC for Nodes we haven't seen. We don't
-  // care about the nature of the Edge (call or ref).
+  // care about the nature of the Edge (call or ref). `FunctionLevels`-wise, we
+  // record them at the same level as the original node (this is a choice, may
+  // need revisiting).
   NodeCount -= static_cast<int64_t>(NodesInLastSCC.size());
   while (!NodesInLastSCC.empty()) {
     const auto *N = *NodesInLastSCC.begin();
@@ -204,12 +206,15 @@ void MLInlineAdvisor::onPassEntry(LazyCallGraph::SCC *LastSCC) {
     }
     ++NodeCount;
     EdgeCount += getLocalCalls(N->getFunction());
+    const auto NLevel = FunctionLevels.at(N);
     for (const auto &E : *(*N)) {
       const auto *AdjNode = &E.getNode();
       assert(!AdjNode->isDead() && !AdjNode->getFunction().isDeclaration());
       auto I = AllNodes.insert(AdjNode);
-      if (I.second)
+      if (I.second) {
         NodesInLastSCC.insert(AdjNode);
+        FunctionLevels[AdjNode] = NLevel;
+      }
     }
   }
 
@@ -461,6 +466,12 @@ void MLInlineAdvisor::print(raw_ostream &OS) const {
     OS << "\n";
   }
   OS << "\n";
+  OS << "[MLInlineAdvisor] FuncLevels:\n";
+  for (auto I : FunctionLevels)
+    OS << (I.first->isDead() ? "<deleted>" : I.first->getFunction().getName())
+       << " : " << I.second << "\n";
+
+  OS << "\n";
 }
 
 MLInlineAdvice::MLInlineAdvice(MLInlineAdvisor *Advisor, CallBase &CB,
diff --git a/llvm/test/Transforms/Inline/ML/coro-split-func-levels.ll b/llvm/test/Transforms/Inline/ML/coro-split-func-levels.ll
new file mode 100644
index 000000000000000..b07536285dcec9b
--- /dev/null
+++ b/llvm/test/Transforms/Inline/ML/coro-split-func-levels.ll
@@ -0,0 +1,42 @@
+
+; REQUIRES: llvm_inliner_model_autogenerated
+; RUN: opt -S -passes='coro-early,scc-oz-module-inliner,print<inline-advisor>' \
+; RUN:  -enable-ml-inliner=release -keep-inline-advisor-for-printing < %s
+
+define void @_Z5get_sv() presplitcoroutine {
+  %1 = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
+  %2 = call ptr @llvm.coro.begin(token %1, ptr null)
+  %3 = call token @llvm.coro.save(ptr null)
+  %4 = call i8 @llvm.coro.suspend(token none, i1 false)
+  call void @_ZN1S12promise_typeD2Ev()
+  ret void
+}
+
+declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr)
+declare ptr @llvm.coro.begin(token, ptr writeonly)
+declare token @llvm.coro.save(ptr)
+declare i8 @llvm.coro.suspend(token, i1)
+
+declare void @__clang_call_terminate()
+
+define void @_ZN1S12promise_typeD2Ev() personality ptr null {
+  invoke void @_Z4funcv()
+          to label %1 unwind label %2
+
+1:                                                ; preds = %0
+  ret void
+
+2:                                                ; preds = %0
+  %3 = landingpad { ptr, i32 }
+          catch ptr null
+  call void @__clang_call_terminate()
+  unreachable
+}
+declare void @_Z4funcv()
+
+; CHECK:      [MLInlineAdvisor] FuncLevels:
+; CHECK-NEXT: _Z5get_sv : 1
+; CHECK-NEXT: _ZN1S12promise_typeD2Ev : 0
+; CHECK-NEXT: _Z5get_sv.resume : 1
+; CHECK-NEXT: _Z5get_sv.destroy : 1
+; CHECK-NEXT: _Z5get_sv.cleanup : 1
\ No newline at end of file

Copy link
Contributor

@aeubanks aeubanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make the title more descriptive and put the bug number in the description

@mtrofin mtrofin changed the title [mlgo][coro] Fix Issue #62616 [mlgo][coro] Assign coro split-ed functions a FunctionLevel Oct 4, 2023
@mtrofin mtrofin merged commit 1b3fc40 into llvm:main Oct 5, 2023
@mtrofin mtrofin deleted the inlfail branch October 5, 2023 04:29
mtrofin added a commit that referenced this pull request Oct 5, 2023
Post #68263, the inline advisor printer tries to print SCC Nodes' names,
but if we perform a full pipeline (like O1), there'll be some DCE-ing
happening and the Node pointers kept in the advisor for this (printing)
purpose are dangling. Using the more eager printer post each scc inline
pass is sufficient.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants