Reapply "Use global TimerGroups for both new pass manager and old pass manager timers" (#131173) (#131217)

This reverts commit 31ebe66.

The reason for the test failure is likely due to
`Name2PairMap::getTimerGroup(...)` not holding a lock.

Author: alanzhao1

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,17 +39,18 @@ 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>;
   /// Map of timers for pass invocations
@@ -71,12 +72,15 @@ 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(); }
-
   /// 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

@@ -63,16 +63,9 @@ 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;
+  TimerGroup *PassTG = nullptr;
 
 public:
-  /// Default constructor for yet-inactive timeinfo.
-  /// Use \p init() to activate it.
-  PassTimingInfo();
-
-  /// Print out timing information and release timers.
-  ~PassTimingInfo();
-
   /// Initializes the static \p TheTimeInfo member to a non-null value when
   /// -time-passes is enabled. Leaves it null otherwise.
   ///
@@ -94,14 +87,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 +95,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 +113,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) {
@@ -193,9 +183,7 @@ Timer &TimePassesHandler::getPassTimer(StringRef PassID, bool IsPass) {
 }
 
 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) {}

llvm/lib/Support/Timer.cpp

@@ -222,16 +222,28 @@ 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) {
+    sys::SmartScopedLock<true> L(timerLock());
+    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 +256,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());