Use global TimerGroups for both new pass manager and old pass manager timers

clang/test/Misc/time-passes.c

@@ -19,6 +19,5 @@
 // NPM:       InstCombinePass{{$}}
 // NPM-NOT:   InstCombinePass #
 // TIME: Total{{$}}
-// NPM: Pass execution timing report
 
 int foo(int x, int y) { return x + y; }

llvm/include/llvm/IR/PassTimingInfo.h

@@ -39,19 +39,20 @@ Timer *getPassTimer(Pass *);
 /// This class implements -time-passes functionality for new pass manager.
 /// It provides the pass-instrumentation callbacks that measure the pass
 /// execution time. They collect timing info into individual timers as
-/// passes are being run. At the end of its life-time it prints the resulting
-/// timing report.
+/// passes are being run.
 class TimePassesHandler {
   /// Value of this type is capable of uniquely identifying pass invocations.
   /// It is a pair of string Pass-Identifier (which for now is common
   /// to all the instance of a given pass) + sequential invocation counter.
   using PassInvocationID = std::pair<StringRef, unsigned>;
 
   /// Groups of timers for passes and analyses.
-  TimerGroup PassTG;
-  TimerGroup AnalysisTG;
+  TimerGroup &PassTG =
+      NamedRegionTimer::getNamedTimerGroup(PassGroupName, PassGroupDesc);
+  TimerGroup &AnalysisTG = NamedRegionTimer::getNamedTimerGroup(
+      AnalysisGroupName, AnalysisGroupDesc);
 
-  using TimerVector = llvm::SmallVector<std::unique_ptr<Timer>, 4>;
+  using TimerVector = llvm::SmallVector<Timer *, 4>;
   /// Map of timers for pass invocations
   StringMap<TimerVector> TimingData;
 
@@ -71,11 +72,16 @@ class TimePassesHandler {
   bool PerRun;
 
 public:
+  static constexpr StringRef PassGroupName = "pass";
+  static constexpr StringRef AnalysisGroupName = "analysis";
+  static constexpr StringRef PassGroupDesc = "Pass execution timing report";
+  static constexpr StringRef AnalysisGroupDesc =
+      "Analysis execution timing report";
+
   TimePassesHandler();
   TimePassesHandler(bool Enabled, bool PerRun = false);
 
-  /// Destructor handles the print action if it has not been handled before.
-  ~TimePassesHandler() { print(); }
+  ~TimePassesHandler() = default;
 
   /// Prints out timing information and then resets the timers.
   void print();

llvm/include/llvm/Support/Timer.h

@@ -169,6 +169,11 @@ struct NamedRegionTimer : public TimeRegion {
   explicit NamedRegionTimer(StringRef Name, StringRef Description,
                             StringRef GroupName,
                             StringRef GroupDescription, bool Enabled = true);
+
+  // Create or get a TimerGroup stored in the same global map owned by
+  // NamedRegionTimer.
+  static TimerGroup &getNamedTimerGroup(StringRef GroupName,
+                                        StringRef GroupDescription);
 };
 
 /// The TimerGroup class is used to group together related timers into a single

llvm/lib/IR/PassTimingInfo.cpp

@@ -62,16 +62,13 @@ class PassTimingInfo {
 
 private:
   StringMap<unsigned> PassIDCountMap; ///< Map that counts instances of passes
-  DenseMap<PassInstanceID, std::unique_ptr<Timer>> TimingData; ///< timers for pass instances
-  TimerGroup TG;
+  DenseMap<PassInstanceID, Timer *> TimingData; ///< timers for pass instances
+  TimerGroup *PassTG = nullptr;
 
 public:
-  /// Default constructor for yet-inactive timeinfo.
-  /// Use \p init() to activate it.
-  PassTimingInfo();
+  PassTimingInfo() = default;
 
-  /// Print out timing information and release timers.
-  ~PassTimingInfo();
+  ~PassTimingInfo() = default;
 
   /// Initializes the static \p TheTimeInfo member to a non-null value when
   /// -time-passes is enabled. Leaves it null otherwise.
@@ -94,14 +91,6 @@ class PassTimingInfo {
 
 static ManagedStatic<sys::SmartMutex<true>> TimingInfoMutex;
 
-PassTimingInfo::PassTimingInfo() : TG("pass", "Pass execution timing report") {}
-
-PassTimingInfo::~PassTimingInfo() {
-  // Deleting the timers accumulates their info into the TG member.
-  // Then TG member is (implicitly) deleted, actually printing the report.
-  TimingData.clear();
-}
-
 void PassTimingInfo::init() {
   if (TheTimeInfo || !TimePassesIsEnabled)
     return;
@@ -110,12 +99,16 @@ void PassTimingInfo::init() {
   // This guarantees that the object will be constructed after static globals,
   // thus it will be destroyed before them.
   static ManagedStatic<PassTimingInfo> TTI;
+  if (!TTI->PassTG)
+    TTI->PassTG = &NamedRegionTimer::getNamedTimerGroup(
+        TimePassesHandler::PassGroupName, TimePassesHandler::PassGroupDesc);
   TheTimeInfo = &*TTI;
 }
 
 /// Prints out timing information and then resets the timers.
 void PassTimingInfo::print(raw_ostream *OutStream) {
-  TG.print(OutStream ? *OutStream : *CreateInfoOutputFile(), true);
+  assert(PassTG && "PassTG is null, did you call PassTimingInfo::Init()?");
+  PassTG->print(OutStream ? *OutStream : *CreateInfoOutputFile(), true);
 }
 
 Timer *PassTimingInfo::newPassTimer(StringRef PassID, StringRef PassDesc) {
@@ -124,7 +117,8 @@ Timer *PassTimingInfo::newPassTimer(StringRef PassID, StringRef PassDesc) {
   // Appending description with a pass-instance number for all but the first one
   std::string PassDescNumbered =
       num <= 1 ? PassDesc.str() : formatv("{0} #{1}", PassDesc, num).str();
-  return new Timer(PassID, PassDescNumbered, TG);
+  assert(PassTG && "PassTG is null, did you call PassTimingInfo::Init()?");
+  return new Timer(PassID, PassDescNumbered, *PassTG);
 }
 
 Timer *PassTimingInfo::getPassTimer(Pass *P, PassInstanceID Pass) {
@@ -133,16 +127,16 @@ Timer *PassTimingInfo::getPassTimer(Pass *P, PassInstanceID Pass) {
 
   init();
   sys::SmartScopedLock<true> Lock(*TimingInfoMutex);
-  std::unique_ptr<Timer> &T = TimingData[Pass];
+  Timer *&T = TimingData[Pass];
 
   if (!T) {
     StringRef PassName = P->getPassName();
     StringRef PassArgument;
     if (const PassInfo *PI = Pass::lookupPassInfo(P->getPassID()))
       PassArgument = PI->getPassArgument();
-    T.reset(newPassTimer(PassArgument.empty() ? PassName : PassArgument, PassName));
+    T = newPassTimer(PassArgument.empty() ? PassName : PassArgument, PassName);
   }
-  return T.get();
+  return T;
 }
 
 PassTimingInfo *PassTimingInfo::TheTimeInfo;
@@ -174,7 +168,7 @@ Timer &TimePassesHandler::getPassTimer(StringRef PassID, bool IsPass) {
   if (!PerRun) {
     TimerVector &Timers = TimingData[PassID];
     if (Timers.size() == 0)
-      Timers.emplace_back(new Timer(PassID, PassID, TG));
+      Timers.push_back(new Timer(PassID, PassID, TG));
     return *Timers.front();
   }
 
@@ -186,16 +180,14 @@ Timer &TimePassesHandler::getPassTimer(StringRef PassID, bool IsPass) {
   std::string FullDesc = formatv("{0} #{1}", PassID, Count).str();
 
   Timer *T = new Timer(PassID, FullDesc, TG);
-  Timers.emplace_back(T);
+  Timers.push_back(T);
   assert(Count == Timers.size() && "Timers vector not adjusted correctly.");
 
   return *T;
 }
 
 TimePassesHandler::TimePassesHandler(bool Enabled, bool PerRun)
-    : PassTG("pass", "Pass execution timing report"),
-      AnalysisTG("analysis", "Analysis execution timing report"),
-      Enabled(Enabled), PerRun(PerRun) {}
+    : Enabled(Enabled), PerRun(PerRun) {}
 
 TimePassesHandler::TimePassesHandler()
     : TimePassesHandler(TimePassesIsEnabled, TimePassesPerRun) {}
@@ -215,6 +207,7 @@ void TimePassesHandler::print() {
     MaybeCreated = CreateInfoOutputFile();
     OS = &*MaybeCreated;
   }
+
   PassTG.print(*OS, true);
   AnalysisTG.print(*OS, true);
 }
@@ -226,7 +219,7 @@ LLVM_DUMP_METHOD void TimePassesHandler::dump() const {
     StringRef PassID = I.getKey();
     const TimerVector& MyTimers = I.getValue();
     for (unsigned idx = 0; idx < MyTimers.size(); idx++) {
-      const Timer* MyTimer = MyTimers[idx].get();
+      const Timer *MyTimer = MyTimers[idx];
       if (MyTimer && MyTimer->isRunning())
         dbgs() << "\tTimer " << MyTimer << " for pass " << PassID << "(" << idx << ")\n";
     }
@@ -236,7 +229,7 @@ LLVM_DUMP_METHOD void TimePassesHandler::dump() const {
     StringRef PassID = I.getKey();
     const TimerVector& MyTimers = I.getValue();
     for (unsigned idx = 0; idx < MyTimers.size(); idx++) {
-      const Timer* MyTimer = MyTimers[idx].get();
+      const Timer *MyTimer = MyTimers[idx];
       if (MyTimer && MyTimer->hasTriggered() && !MyTimer->isRunning())
         dbgs() << "\tTimer " << MyTimer << " for pass " << PassID << "(" << idx << ")\n";
     }

llvm/lib/Support/Timer.cpp

@@ -222,16 +222,27 @@ class Name2PairMap {
              StringRef GroupDescription) {
     sys::SmartScopedLock<true> L(timerLock());
 
-    std::pair<TimerGroup*, Name2TimerMap> &GroupEntry = Map[GroupName];
-
-    if (!GroupEntry.first)
-      GroupEntry.first = new TimerGroup(GroupName, GroupDescription);
-
+    std::pair<TimerGroup *, Name2TimerMap> &GroupEntry =
+        getGroupEntry(GroupName, GroupDescription);
     Timer &T = GroupEntry.second[Name];
     if (!T.isInitialized())
       T.init(Name, Description, *GroupEntry.first);
     return T;
   }
+
+  TimerGroup &getTimerGroup(StringRef GroupName, StringRef GroupDescription) {
+    return *getGroupEntry(GroupName, GroupDescription).first;
+  }
+
+private:
+  std::pair<TimerGroup *, Name2TimerMap> &
+  getGroupEntry(StringRef GroupName, StringRef GroupDescription) {
+    std::pair<TimerGroup *, Name2TimerMap> &GroupEntry = Map[GroupName];
+    if (!GroupEntry.first)
+      GroupEntry.first = new TimerGroup(GroupName, GroupDescription);
+
+    return GroupEntry;
+  }
 };
 
 }
@@ -244,6 +255,11 @@ NamedRegionTimer::NamedRegionTimer(StringRef Name, StringRef Description,
                      : &namedGroupedTimers().get(Name, Description, GroupName,
                                                  GroupDescription)) {}
 
+TimerGroup &NamedRegionTimer::getNamedTimerGroup(StringRef GroupName,
+                                                 StringRef GroupDescription) {
+  return namedGroupedTimers().getTimerGroup(GroupName, GroupDescription);
+}
+
 //===----------------------------------------------------------------------===//
 //   TimerGroup Implementation
 //===----------------------------------------------------------------------===//

llvm/unittests/IR/TimePassesTest.cpp

@@ -161,8 +161,8 @@ TEST(TimePassesTest, CustomOut) {
   PI.runBeforePass(Pass2, M);
   PI.runAfterPass(Pass2, M, PreservedAnalyses::all());
 
-  // Generate report by deleting the handler.
-  TimePasses.reset();
+  // Clear and generate report again.
+  TimePasses->print();
 
   // There should be Pass2 in this report and no Pass1.
   EXPECT_FALSE(TimePassesStr.str().empty());