Use global TimerGroups for both new pass manager and old pass manager timers #130375
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… timers Additionally, remove the behavior for both pass manager's timer manager classes (`PassTimingInfo` for the old pass manager and `TimePassesHandler` for the new pass manager) where these classes would print the values of their timers upon destruction. Currently, each pass manager manages their own `TimerGroup`s. This is problematic because of duplicate `TimerGroup`s (both pass managers have a `TimerGroup` for pass times with identical names and descriptions). The result is that in Clang, `-ftime-report` has two "Pass execution timing report" sections (one for the new pass manager which manages optimization passes, and one for the old pass manager which manages the backend). The result of this change is that Clang's `-ftime-report` now prints both optimization and backend pass timing info in a unified "Pass execution timing report" section. Moving the ownership of the `TimerGroups` to globals also makes it easier to implement JSON-formatted `-ftime-report`. This was not possible with the old structure because the two pass managers were created and destroyed in far parts of the codebase and outputting JSON requires the printing logic to be at the same place because of formatting. Previous discourse discussion: https://discourse.llvm.org/t/difficulties-with-implementing-json-formatted-ftime-report/84353
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-clang Author: Alan Zhao (alanzhao1) ChangesAdditionally, remove the behavior for both pass manager's timer manager classes ( Currently, each pass manager manages their own Moving the ownership of the Previous discourse discussion: https://discourse.llvm.org/t/difficulties-with-implementing-json-formatted-ftime-report/84353 Full diff: https://github.com/llvm/llvm-project/pull/130375.diff 6 Files Affected:
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..13947f7405245 100644
--- a/llvm/include/llvm/IR/PassTimingInfo.h
+++ b/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();
diff --git a/llvm/include/llvm/Support/Timer.h b/llvm/include/llvm/Support/Timer.h
index abe30451dd2f2..3e115df1500fe 100644
--- a/llvm/include/llvm/Support/Timer.h
+++ b/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
diff --git a/llvm/lib/IR/PassTimingInfo.cpp b/llvm/lib/IR/PassTimingInfo.cpp
index 46db2c74a5c76..0d133be890ce2 100644
--- a/llvm/lib/IR/PassTimingInfo.cpp
+++ b/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";
}
diff --git a/llvm/lib/Support/Timer.cpp b/llvm/lib/Support/Timer.cpp
index 089dae2886f22..0be1e5b47bee8 100644
--- a/llvm/lib/Support/Timer.cpp
+++ b/llvm/lib/Support/Timer.cpp
@@ -222,16 +222,27 @@ 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;
+ }
};
}
@@ -239,10 +250,23 @@ class Name2PairMap {
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
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());
|
aeubanks
reviewed
Mar 10, 2025
aeubanks
reviewed
Mar 10, 2025
MaskRay
approved these changes
Mar 11, 2025
aeubanks
reviewed
Mar 11, 2025
aeubanks
reviewed
Mar 11, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/14607 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/14604 Here is the relevant piece of the build log for the reference |
aeubanks
added a commit
that referenced
this pull request
Mar 13, 2025
… manager timers" (#131173) Reverts #130375 Causes breakages, e.g. https://lab.llvm.org/buildbot/#/builders/160/builds/14607
|
Reverted due to bot breakages. Also the precommit CI was already showing failures that are presumably related to this. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/7437 Here is the relevant piece of the build log for the reference |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Mar 13, 2025
…nd old pass manager timers" (#131173) Reverts llvm/llvm-project#130375 Causes breakages, e.g. https://lab.llvm.org/buildbot/#/builders/160/builds/14607
|
Hmm...the bot breakages look like LLD test failures. Will investigate further. |
Note to self: add |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/13013 Here is the relevant piece of the build log for the reference |
frederik-h
pushed a commit
to frederik-h/llvm-project
that referenced
this pull request
Mar 18, 2025
…d pass manager timers (llvm#130375) Additionally, remove the behavior for both pass manager's timer manager classes (`PassTimingInfo` for the old pass manager and `TimePassesHandler` for the new pass manager) where these classes would print the values of their timers upon destruction. Currently, each pass manager manages their own `TimerGroup`s. This is problematic because of duplicate `TimerGroup`s (both pass managers have a `TimerGroup` for pass times with identical names and descriptions). The result is that in Clang, `-ftime-report` has two "Pass execution timing report" sections (one for the new pass manager which manages optimization passes, and one for the old pass manager which manages the backend). The result of this change is that Clang's `-ftime-report` now prints both optimization and backend pass timing info in a unified "Pass execution timing report" section. Moving the ownership of the `TimerGroups` to globals also makes it easier to implement JSON-formatted `-ftime-report`. This was not possible with the old structure because the two pass managers were created and destroyed in far parts of the codebase and outputting JSON requires the printing logic to be at the same place because of formatting. Previous discourse discussion: https://discourse.llvm.org/t/difficulties-with-implementing-json-formatted-ftime-report/84353
frederik-h
pushed a commit
to frederik-h/llvm-project
that referenced
this pull request
Mar 18, 2025
… manager timers" (llvm#131173) Reverts llvm#130375 Causes breakages, e.g. https://lab.llvm.org/buildbot/#/builders/160/builds/14607
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Additionally, remove the behavior for both pass manager's timer manager classes (
PassTimingInfofor the old pass manager andTimePassesHandlerfor the new pass manager) where these classes would print the values of their timers upon destruction.Currently, each pass manager manages their own
TimerGroups. This is problematic because of duplicateTimerGroups (both pass managers have aTimerGroupfor pass times with identical names and descriptions). The result is that in Clang,-ftime-reporthas two "Pass execution timing report" sections (one for the new pass manager which manages optimization passes, and one for the old pass manager which manages the backend). The result of this change is that Clang's-ftime-reportnow prints both optimization and backend pass timing info in a unified "Pass execution timing report" section.Moving the ownership of the
TimerGroupsto globals also makes it easier to implement JSON-formatted-ftime-report. This was not possible with the old structure because the two pass managers were created and destroyed in far parts of the codebase and outputting JSON requires the printing logic to be at the same place because of formatting.Previous discourse discussion: https://discourse.llvm.org/t/difficulties-with-implementing-json-formatted-ftime-report/84353