code review comments

Author: alanzhao1

llvm/include/llvm/IR/PassTimingInfo.h

@@ -65,7 +65,18 @@ class TimePassesHandler {
   bool Enabled;
   bool PerRun;
 
+  TimerGroup &PassTG =
+      NamedRegionTimer::getNamedTimerGroup(PassGroupName, PassGroupDesc);
+  TimerGroup &AnalysisTG = NamedRegionTimer::getNamedTimerGroup(
+      AnalysisGroupName, AnalysisGroupDesc);
+
 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);
 

llvm/include/llvm/Support/Timer.h

@@ -178,8 +178,8 @@ struct NamedRegionTimer : public TimeRegion {
 
   // Create or get a TimerGroup stored in the same global map owned by
   // NamedRegionTimer.
-  static TimerGroup &getNamedGroupTimerGroup(StringRef GroupName,
-                                             StringRef GroupDescription);
+  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

@@ -37,12 +37,6 @@ 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"));
@@ -109,7 +103,8 @@ void PassTimingInfo::init() {
 
 /// Prints out timing information and then resets the timers.
 void PassTimingInfo::print(raw_ostream *OutStream) {
-  NamedRegionTimer::getNamedGroupTimerGroup(PassGroupName, PassGroupDesc)
+  NamedRegionTimer::getNamedTimerGroup(TimePassesHandler::PassGroupName,
+                                       TimePassesHandler::PassGroupDesc)
       .print(OutStream ? *OutStream : *CreateInfoOutputFile(), true);
 }
 
@@ -119,8 +114,10 @@ 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 &NamedRegionTimer::getNamedGroupTimer(PassID, PassDescNumbered,
-                                               PassGroupName, PassGroupDesc);
+  return new Timer(
+      PassID, PassDescNumbered,
+      NamedRegionTimer::getNamedTimerGroup(TimePassesHandler::PassGroupName,
+                                           TimePassesHandler::PassGroupDesc));
 }
 
 Timer *PassTimingInfo::getPassTimer(Pass *P, PassInstanceID Pass) {
@@ -166,9 +163,7 @@ 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) {
-  StringRef TGName = IsPass ? PassGroupName : PassGroupDesc;
-  StringRef TGDesc = IsPass ? PassGroupDesc : AnalysisGroupDesc;
-  TimerGroup &TG = NamedRegionTimer::getNamedGroupTimerGroup(TGName, TGDesc);
+  TimerGroup &TG = IsPass ? PassTG : AnalysisTG;
   if (!PerRun) {
     TimerVector &Timers = TimingData[PassID];
     if (Timers.size() == 0)
@@ -212,10 +207,9 @@ void TimePassesHandler::print() {
     OS = &*MaybeCreated;
   }
 
-  NamedRegionTimer::getNamedGroupTimerGroup(PassGroupName, PassGroupDesc)
+  NamedRegionTimer::getNamedTimerGroup(PassGroupName, PassGroupDesc)
       .print(*OS, true);
-  NamedRegionTimer::getNamedGroupTimerGroup(AnalysisGroupName,
-                                            AnalysisGroupDesc)
+  NamedRegionTimer::getNamedTimerGroup(AnalysisGroupName, AnalysisGroupDesc)
       .print(*OS, true);
 }
 

llvm/lib/Support/Timer.cpp

@@ -250,21 +250,13 @@ class Name2PairMap {
 NamedRegionTimer::NamedRegionTimer(StringRef Name, StringRef Description,
                                    StringRef GroupName,
                                    StringRef GroupDescription, bool Enabled)
-    : TimeRegion(!Enabled ? nullptr
-                          : &getNamedGroupTimer(Name, Description, GroupName,
-                                                GroupDescription)) {}
+    : TimeRegion(!Enabled
+                     ? nullptr
+                     : &namedGroupedTimers().get(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) {
+TimerGroup &NamedRegionTimer::getNamedTimerGroup(StringRef GroupName,
+                                                 StringRef GroupDescription) {
   return namedGroupedTimers().getTimerGroup(GroupName, GroupDescription);
 }