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,14 @@ 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;
-
-  using TimerVector = llvm::SmallVector<std::unique_ptr<Timer>, 4>;
+  using TimerVector = llvm::SmallVector<Timer *, 4>;
   /// Map of timers for pass invocations
   StringMap<TimerVector> TimingData;
 
@@ -74,8 +69,7 @@ class TimePassesHandler {
   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,17 @@ struct NamedRegionTimer : public TimeRegion {
   explicit NamedRegionTimer(StringRef Name, StringRef Description,
                             StringRef GroupName,
                             StringRef GroupDescription, bool Enabled = true);
+
+  // Create or get a timer stored in the same global map as other timers owned
+  // by NamedRegionTimer.
+  static Timer &getNamedGroupTimer(StringRef Name, StringRef Description,
+                                   StringRef GroupName,
+                                   StringRef GroupDescription);
+
+  // Create or get a TimerGroup stored in the same global map owned by
+  // NamedRegionTimer.
+  static TimerGroup &getNamedGroupTimerGroup(StringRef GroupName,
+                                             StringRef GroupDescription);
 };
 
 /// The TimerGroup class is used to group together related timers into a single

llvm/lib/IR/PassTimingInfo.cpp

@@ -37,6 +37,12 @@ namespace llvm {
 bool TimePassesIsEnabled = false;
 bool TimePassesPerRun = false;
 
+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";
+
 static cl::opt<bool, true> EnableTiming(
     "time-passes", cl::location(TimePassesIsEnabled), cl::Hidden,
     cl::desc("Time each pass, printing elapsed time for each on exit"));
@@ -62,16 +68,12 @@ 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
 
 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 +96,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;
@@ -115,7 +109,8 @@ void PassTimingInfo::init() {
 
 /// Prints out timing information and then resets the timers.
 void PassTimingInfo::print(raw_ostream *OutStream) {
-  TG.print(OutStream ? *OutStream : *CreateInfoOutputFile(), true);
+  NamedRegionTimer::getNamedGroupTimerGroup(PassGroupName, PassGroupDesc)
+      .print(OutStream ? *OutStream : *CreateInfoOutputFile(), true);
 }
 
 Timer *PassTimingInfo::newPassTimer(StringRef PassID, StringRef PassDesc) {
@@ -124,7 +119,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);
+  return &NamedRegionTimer::getNamedGroupTimer(PassID, PassDescNumbered,
+                                               PassGroupName, PassGroupDesc);
 }
 
 Timer *PassTimingInfo::getPassTimer(Pass *P, PassInstanceID Pass) {
@@ -133,16 +129,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;
@@ -170,11 +166,13 @@ void reportAndResetTimings(raw_ostream *OutStream) {
 /// Returns the timer for the specified pass invocation of \p PassID.
 /// Each time it creates a new timer.
 Timer &TimePassesHandler::getPassTimer(StringRef PassID, bool IsPass) {
-  TimerGroup &TG = IsPass ? PassTG : AnalysisTG;
+  StringRef TGName = IsPass ? PassGroupName : PassGroupDesc;
+  StringRef TGDesc = IsPass ? PassGroupDesc : AnalysisGroupDesc;
+  TimerGroup &TG = NamedRegionTimer::getNamedGroupTimerGroup(TGName, TGDesc);
   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 +184,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,8 +211,12 @@ void TimePassesHandler::print() {
     MaybeCreated = CreateInfoOutputFile();
     OS = &*MaybeCreated;
   }
-  PassTG.print(*OS, true);
-  AnalysisTG.print(*OS, true);
+
+  NamedRegionTimer::getNamedGroupTimerGroup(PassGroupName, PassGroupDesc)
+      .print(*OS, true);
+  NamedRegionTimer::getNamedGroupTimerGroup(AnalysisGroupName,
+                                            AnalysisGroupDesc)
+      .print(*OS, true);
 }
 
 LLVM_DUMP_METHOD void TimePassesHandler::dump() const {
@@ -226,7 +226,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 +236,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,27 +222,51 @@ 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;
+  }
 };
 
 }
 
 NamedRegionTimer::NamedRegionTimer(StringRef Name, StringRef Description,
                                    StringRef GroupName,
                                    StringRef GroupDescription, bool Enabled)
-    : TimeRegion(!Enabled
-                     ? nullptr
-                     : &namedGroupedTimers().get(Name, Description, GroupName,
-                                                 GroupDescription)) {}
+    : TimeRegion(!Enabled ? nullptr
+                          : &getNamedGroupTimer(Name, Description, GroupName,
+                                                GroupDescription)) {}
+
+Timer &NamedRegionTimer::getNamedGroupTimer(StringRef Name,
+                                            StringRef Description,
+                                            StringRef GroupName,
+                                            StringRef GroupDescription) {
+  return namedGroupedTimers().get(Name, Description, GroupName,
+                                  GroupDescription);
+}
+
+TimerGroup &
+NamedRegionTimer::getNamedGroupTimerGroup(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());