[clang] [llvm] Reapply "Use global TimerGroups for both new pass manager and old pass manager timers" (#131173) (PR #131217)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 13 14:05:16 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Alan Zhao (alanzhao1)
<details>
<summary>Changes</summary>
This reverts commit 31ebe6647b7f1fc7f6778a5438175b12f82357ae.
The reason for the test failure is likely due to
`Name2PairMap::getTimerGroup(...)` not holding a lock.
---
Full diff: https://github.com/llvm/llvm-project/pull/131217.diff
6 Files Affected:
- (modified) clang/test/Misc/time-passes.c (-1)
- (modified) llvm/include/llvm/IR/PassTimingInfo.h (+11-7)
- (modified) llvm/include/llvm/Support/Timer.h (+5)
- (modified) llvm/lib/IR/PassTimingInfo.cpp (+9-21)
- (modified) llvm/lib/Support/Timer.cpp (+22-5)
- (modified) llvm/unittests/IR/TimePassesTest.cpp (+2-2)
``````````diff
diff --git a/clang/test/Misc/time-passes.c b/clang/test/Misc/time-passes.c
index 395da216aad42..c1669826b2268 100644
--- a/clang/test/Misc/time-passes.c
+++ b/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; }
diff --git a/llvm/include/llvm/IR/PassTimingInfo.h b/llvm/include/llvm/IR/PassTimingInfo.h
index 1148399943186..b47ba7f16ef37 100644
--- a/llvm/include/llvm/IR/PassTimingInfo.h
+++ b/llvm/include/llvm/IR/PassTimingInfo.h
@@ -39,8 +39,7 @@ 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
@@ -48,8 +47,10 @@ class TimePassesHandler {
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();
diff --git a/llvm/include/llvm/Support/Timer.h b/llvm/include/llvm/Support/Timer.h
index abe30451dd2f2..5a5082b6718ed 100644
--- a/llvm/include/llvm/Support/Timer.h
+++ b/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
diff --git a/llvm/lib/IR/PassTimingInfo.cpp b/llvm/lib/IR/PassTimingInfo.cpp
index 46db2c74a5c76..4e27086e97ac5 100644
--- a/llvm/lib/IR/PassTimingInfo.cpp
+++ b/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) {}
diff --git a/llvm/lib/Support/Timer.cpp b/llvm/lib/Support/Timer.cpp
index eca726828c697..69a1846fec29b 100644
--- a/llvm/lib/Support/Timer.cpp
+++ b/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
//===----------------------------------------------------------------------===//
diff --git a/llvm/unittests/IR/TimePassesTest.cpp b/llvm/unittests/IR/TimePassesTest.cpp
index 33f8e00b377d5..85986132103ca 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());
- // 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());
``````````
</details>
https://github.com/llvm/llvm-project/pull/131217
More information about the llvm-commits
mailing list