[clang] [llvm] Revert "Use global TimerGroups for both new pass manager and old pass manager timers" (PR #131173)
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 13 10:28:59 PDT 2025
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/131173
Reverts llvm/llvm-project#130375
Causes breakages, e.g. https://lab.llvm.org/buildbot/#/builders/160/builds/14607
>From 747535b6fd8b3bbee0c8a34a7d92fd47a89d65f5 Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <ayueubanks at gmail.com>
Date: Thu, 13 Mar 2025 10:28:36 -0700
Subject: [PATCH] =?UTF-8?q?Revert=20"[llvm][Timer]=20Use=20global=20TimerG?=
=?UTF-8?q?roups=20for=20both=20new=20pass=20manager=20and=20ol=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit 09d8e442ac2884aabe4cdfb01d1444b54cff7147.
---
clang/test/Misc/time-passes.c | 1 +
llvm/include/llvm/IR/PassTimingInfo.h | 18 +++++++---------
llvm/include/llvm/Support/Timer.h | 5 -----
llvm/lib/IR/PassTimingInfo.cpp | 30 +++++++++++++++++++--------
llvm/lib/Support/Timer.cpp | 26 +++++------------------
llvm/unittests/IR/TimePassesTest.cpp | 4 ++--
6 files changed, 36 insertions(+), 48 deletions(-)
diff --git a/clang/test/Misc/time-passes.c b/clang/test/Misc/time-passes.c
index c1669826b2268..395da216aad42 100644
--- a/clang/test/Misc/time-passes.c
+++ b/clang/test/Misc/time-passes.c
@@ -19,5 +19,6 @@
// NPM: InstCombinePass{{$}}
// NPM-NOT: InstCombinePass #
// TIME: Total{{$}}
+// NPM: Pass execution timing report
int foo(int x, int y) { return x + y; }
diff --git a/llvm/include/llvm/IR/PassTimingInfo.h b/llvm/include/llvm/IR/PassTimingInfo.h
index b47ba7f16ef37..1148399943186 100644
--- a/llvm/include/llvm/IR/PassTimingInfo.h
+++ b/llvm/include/llvm/IR/PassTimingInfo.h
@@ -39,7 +39,8 @@ 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.
+/// passes are being run. At the end of its life-time it prints the resulting
+/// timing report.
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
@@ -47,10 +48,8 @@ class TimePassesHandler {
using PassInvocationID = std::pair<StringRef, unsigned>;
/// Groups of timers for passes and analyses.
- TimerGroup &PassTG =
- NamedRegionTimer::getNamedTimerGroup(PassGroupName, PassGroupDesc);
- TimerGroup &AnalysisTG = NamedRegionTimer::getNamedTimerGroup(
- AnalysisGroupName, AnalysisGroupDesc);
+ TimerGroup PassTG;
+ TimerGroup AnalysisTG;
using TimerVector = llvm::SmallVector<std::unique_ptr<Timer>, 4>;
/// Map of timers for pass invocations
@@ -72,15 +71,12 @@ 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();
diff --git a/llvm/include/llvm/Support/Timer.h b/llvm/include/llvm/Support/Timer.h
index 5a5082b6718ed..abe30451dd2f2 100644
--- a/llvm/include/llvm/Support/Timer.h
+++ b/llvm/include/llvm/Support/Timer.h
@@ -169,11 +169,6 @@ 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
diff --git a/llvm/lib/IR/PassTimingInfo.cpp b/llvm/lib/IR/PassTimingInfo.cpp
index 4e27086e97ac5..46db2c74a5c76 100644
--- a/llvm/lib/IR/PassTimingInfo.cpp
+++ b/llvm/lib/IR/PassTimingInfo.cpp
@@ -63,9 +63,16 @@ class PassTimingInfo {
private:
StringMap<unsigned> PassIDCountMap; ///< Map that counts instances of passes
DenseMap<PassInstanceID, std::unique_ptr<Timer>> TimingData; ///< timers for pass instances
- TimerGroup *PassTG = nullptr;
+ TimerGroup TG;
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.
///
@@ -87,6 +94,14 @@ 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;
@@ -95,16 +110,12 @@ 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) {
- assert(PassTG && "PassTG is null, did you call PassTimingInfo::Init()?");
- PassTG->print(OutStream ? *OutStream : *CreateInfoOutputFile(), true);
+ TG.print(OutStream ? *OutStream : *CreateInfoOutputFile(), true);
}
Timer *PassTimingInfo::newPassTimer(StringRef PassID, StringRef PassDesc) {
@@ -113,8 +124,7 @@ 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();
- assert(PassTG && "PassTG is null, did you call PassTimingInfo::Init()?");
- return new Timer(PassID, PassDescNumbered, *PassTG);
+ return new Timer(PassID, PassDescNumbered, TG);
}
Timer *PassTimingInfo::getPassTimer(Pass *P, PassInstanceID Pass) {
@@ -183,7 +193,9 @@ Timer &TimePassesHandler::getPassTimer(StringRef PassID, bool IsPass) {
}
TimePassesHandler::TimePassesHandler(bool Enabled, bool PerRun)
- : Enabled(Enabled), PerRun(PerRun) {}
+ : PassTG("pass", "Pass execution timing report"),
+ AnalysisTG("analysis", "Analysis execution timing report"),
+ Enabled(Enabled), PerRun(PerRun) {}
TimePassesHandler::TimePassesHandler()
: TimePassesHandler(TimePassesIsEnabled, TimePassesPerRun) {}
diff --git a/llvm/lib/Support/Timer.cpp b/llvm/lib/Support/Timer.cpp
index 4d3b4f7481edf..eca726828c697 100644
--- a/llvm/lib/Support/Timer.cpp
+++ b/llvm/lib/Support/Timer.cpp
@@ -222,26 +222,15 @@ class Name2PairMap {
StringRef GroupDescription) {
sys::SmartScopedLock<true> L(timerLock());
- 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;
- }
+ std::pair<TimerGroup*, Name2TimerMap> &GroupEntry = Map[GroupName];
-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;
+ Timer &T = GroupEntry.second[Name];
+ if (!T.isInitialized())
+ T.init(Name, Description, *GroupEntry.first);
+ return T;
}
};
@@ -255,11 +244,6 @@ 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
//===----------------------------------------------------------------------===//
diff --git a/llvm/unittests/IR/TimePassesTest.cpp b/llvm/unittests/IR/TimePassesTest.cpp
index 85986132103ca..33f8e00b377d5 100644
--- a/llvm/unittests/IR/TimePassesTest.cpp
+++ b/llvm/unittests/IR/TimePassesTest.cpp
@@ -161,8 +161,8 @@ TEST(TimePassesTest, CustomOut) {
PI.runBeforePass(Pass2, M);
PI.runAfterPass(Pass2, M, PreservedAnalyses::all());
- // Clear and generate report again.
- TimePasses->print();
+ // Generate report by deleting the handler.
+ TimePasses.reset();
// There should be Pass2 in this report and no Pass1.
EXPECT_FALSE(TimePassesStr.str().empty());
More information about the llvm-commits
mailing list