[llvm] [PassTimingInfo] Handle nested timers in passes (PR #70165)

[Wei Wang via llvm-commits]

https://github.com/apolloww created https://github.com/llvm/llvm-project/pull/70165

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

>From 225b05e6c4e560d7f3b02a2ab46f62593bbaf40d Mon Sep 17 00:00:00 2001
From: Wei Wang <wangwei at meta.com>
Date: Tue, 24 Oct 2023 16:14:41 -0700
Subject: [PATCH] [PassTimingInfo] Handle nested timers in passes

---
 llvm/include/llvm/IR/PassTimingInfo.h |  5 +++--
 llvm/lib/IR/PassTimingInfo.cpp        | 24 ++++++++++++++++++------
 llvm/test/Other/time-passes.ll        | 15 +++++++++++++--
 3 files changed, 34 insertions(+), 10 deletions(-)

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)