Skip to content

[PassTimingInfo] Handle nested timers in passes #70165

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

Merged
merged 2 commits into from
Oct 27, 2023
Merged

Conversation

apolloww
Copy link
Contributor

Some pass can call other passes, such as CoroCleanupPass. Make pass timers work with nested passes.

@llvmbot
Copy link
Member

llvmbot commented Oct 25, 2023

@llvm/pr-subscribers-llvm-ir

Author: Wei Wang (apolloww)

Changes

Some pass can call other passes, such as CoroCleanupPass. Make pass timers work with nested passes.


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

3 Files Affected:

  • (modified) llvm/include/llvm/IR/PassTimingInfo.h (+3-2)
  • (modified) llvm/lib/IR/PassTimingInfo.cpp (+18-6)
  • (modified) llvm/test/Other/time-passes.ll (+13-2)
diff --git a/llvm/include/llvm/IR/PassTimingInfo.h b/llvm/include/llvm/IR/PassTimingInfo.h
index d464b998fa706af..076e27b9b80185f 100644
--- a/llvm/include/llvm/IR/PassTimingInfo.h
+++ b/llvm/include/llvm/IR/PassTimingInfo.h
@@ -55,8 +55,9 @@ class TimePassesHandler {
   /// Map of timers for pass invocations
   StringMap<TimerVector> TimingData;
 
-  /// Currently active pass timer.
-  Timer *ActivePassTimer = nullptr;
+  /// Stack of currently active pass timers. Passes can request other
+  /// passes.
+  SmallVector<Timer *, 8> PassActiveTimerStack;
   /// Stack of currently active analysis timers. Analyses can request other
   /// analyses.
   SmallVector<Timer *, 8> AnalysisActiveTimerStack;
diff --git a/llvm/lib/IR/PassTimingInfo.cpp b/llvm/lib/IR/PassTimingInfo.cpp
index cfd27bf78793746..3816eff5c0f22da 100644
--- a/llvm/lib/IR/PassTimingInfo.cpp
+++ b/llvm/lib/IR/PassTimingInfo.cpp
@@ -252,9 +252,14 @@ static bool shouldIgnorePass(StringRef PassID) {
 void TimePassesHandler::startPassTimer(StringRef PassID) {
   if (shouldIgnorePass(PassID))
     return;
-  assert(!ActivePassTimer && "should only have one pass timer at a time");
+  // Stop the previous pass timer to prevent double counting when a
+  // pass requests another pass.
+  if (!PassActiveTimerStack.empty()) {
+    assert(PassActiveTimerStack.back()->isRunning());
+    PassActiveTimerStack.back()->stopTimer();
+  }
   Timer &MyTimer = getPassTimer(PassID, /*IsPass*/ true);
-  ActivePassTimer = &MyTimer;
+  PassActiveTimerStack.push_back(&MyTimer);
   assert(!MyTimer.isRunning());
   MyTimer.startTimer();
 }
@@ -262,10 +267,17 @@ void TimePassesHandler::startPassTimer(StringRef PassID) {
 void TimePassesHandler::stopPassTimer(StringRef PassID) {
   if (shouldIgnorePass(PassID))
     return;
-  assert(ActivePassTimer);
-  assert(ActivePassTimer->isRunning());
-  ActivePassTimer->stopTimer();
-  ActivePassTimer = nullptr;
+  assert(!PassActiveTimerStack.empty() && "empty stack in popTimer");
+  Timer *MyTimer = PassActiveTimerStack.pop_back_val();
+  assert(MyTimer && "timer should be present");
+  assert(MyTimer->isRunning());
+  MyTimer->stopTimer();
+
+  // Restart the previously stopped timer.
+  if (!PassActiveTimerStack.empty()) {
+    assert(!PassActiveTimerStack.back()->isRunning());
+    PassActiveTimerStack.back()->startTimer();
+  }
 }
 
 void TimePassesHandler::startAnalysisTimer(StringRef PassID) {
diff --git a/llvm/test/Other/time-passes.ll b/llvm/test/Other/time-passes.ll
index f7a0851a69c140a..fd73fe2fd243d40 100644
--- a/llvm/test/Other/time-passes.ll
+++ b/llvm/test/Other/time-passes.ll
@@ -1,11 +1,11 @@
 ; RUN: opt < %s -disable-output -passes='default<O2>' -time-passes 2>&1 | FileCheck %s --check-prefix=TIME
 ;
 ; For new pass manager, check that -time-passes-per-run emit one report for each pass run.
-; RUN: opt < %s -disable-output -passes='instcombine,instcombine,loop-mssa(licm)' -time-passes-per-run 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-PER-RUN
+; RUN: opt < %s -disable-output -passes='coro-cleanup,function(instcombine,instcombine,loop-mssa(licm))' -time-passes-per-run 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-PER-RUN --check-prefix=TIME-PER-RUN-CORO
 ; RUN: opt < %s -disable-output -passes='instcombine,loop-mssa(licm),instcombine,loop-mssa(licm)' -time-passes-per-run 2>&1 | FileCheck %s --check-prefix=TIME --check-prefix=TIME-PER-RUN -check-prefix=TIME-DOUBLE-LICM
 ;
 ; For new pass manager, check that -time-passes emit one report for each pass.
-; RUN: opt < %s -disable-output -passes='instcombine,instcombine,loop-mssa(licm)' -time-passes 2>&1 | FileCheck %s --check-prefixes=TIME,TIME-PER-PASS
+; RUN: opt < %s -disable-output -passes='coro-cleanup,function(instcombine,instcombine,loop-mssa(licm))' -time-passes 2>&1 | FileCheck %s --check-prefixes=TIME,TIME-PER-PASS,TIME-PER-PASS-CORO
 ; RUN: opt < %s -disable-output -passes='instcombine,loop-mssa(licm),instcombine,loop-mssa(licm)' -time-passes 2>&1 | FileCheck %s --check-prefixes=TIME,TIME-PER-PASS
 ;
 ; The following 2 test runs verify -info-output-file interaction (default goes to stderr, '-' goes to stdout).
@@ -30,11 +30,15 @@
 ; TIME-PER-RUN-DAG:      LCSSAPass
 ; TIME-PER-RUN-DAG:      LoopSimplifyPass
 ; TIME-PER-RUN-DAG:      VerifierPass
+; TIME-PER-RUN-CORO-DAG:     SimplifyCFGPass #1
+; TIME-PER-RUN-CORO-DAG:     CoroCleanupPass #1
 ; TIME-PER-PASS-DAG:   InstCombinePass
 ; TIME-PER-PASS-DAG:   LICMPass
 ; TIME-PER-PASS-DAG:   LCSSAPass
 ; TIME-PER-PASS-DAG:   LoopSimplifyPass
 ; TIME-PER-PASS-DAG:   VerifierPass
+; TIME-PER-PASS-CORO-DAG:    SimplifyCFGPass
+; TIME-PER-PASS-CORO-DAG:    CoroCleanupPass
 ; TIME-PER-PASS-NOT:   InstCombinePass #
 ; TIME-PER-PASS-NOT:   LICMPass #
 ; TIME-PER-PASS-NOT:   LCSSAPass #
@@ -78,3 +82,10 @@ end:
   ret void
 
 }
+
+define void @baz_coro() {
+  %unused = call ptr @llvm.coro.begin(token none, ptr null)
+  ret void
+}
+
+declare ptr @llvm.coro.begin(token, ptr)

@apolloww apolloww requested a review from aeubanks October 25, 2023 05:24
@apolloww apolloww merged commit b00c8b9 into llvm:main Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants