This is an archive of the discontinued LLVM Phabricator instance.

[NFC][PassTiming] factor out generic PassTimingInfo
ClosedPublic

Authored by fedor.sergeev on Aug 26 2018, 4:59 PM.

Details

Summary

Moving PassTimingInfo from legacy pass manager code into a separate header.
Making it suitable for both legacy and new pass manager.
Adding a test on -time-passes main functionality.

Diff Detail

Event Timeline

fedor.sergeev created this revision.Aug 26 2018, 4:59 PM
paquette accepted this revision.Aug 28 2018, 10:11 AM

Added some nits on comments. These could be done in a follow-up patch. I think a lot of these comments could use a style update.

LGTM otherwise, since this is mostly just code motion.

include/llvm/IR/PassTimingInfo.h
30–34

This can probably be simplified, and made a bit more Doxygen-ey.

Provides pass managers a generic interface for collecting pass timing information.

Legacy pass managers should provide a \p PassInfo* as \p PassInfoT.

New pass managers should provide a \p StringRef as \p PassInfoT.
40–41

Might make more sense to say something like

Default constructor. Use \p init() to for initialization.
47–49

Doxygen style, plus some style nits.

Initializes \p TheTimeInfo to a non-null value when -time-passes is enabled. Leaves it null otherwise.

This method may be called multiple times.
lib/IR/PassTimingInfo.cpp
53–59

Comments are a little redundant here. If we moved it all into the header comment it'd be a bit clearer.

E.g

Print out timing information and delete all timers, along with their \p TimerGroup.

The timing report is printed on deletion of the \p TimerGroup.
62–65

The init() in PassTimingInfo.h already has this comment. So, this one can be removed.

This revision is now accepted and ready to land.Aug 28 2018, 10:11 AM

updating comments

fedor.sergeev marked 5 inline comments as done.Aug 28 2018, 2:07 PM

Oh, well... it appears that this NFC was not exactly NFC after all :(

Before this change time information was aggregated per instance of Pass (Pass*), after this change time information is aggregated per name of Pass (P->getArgument()),
which causes information about multiple instances of the same pass to be aggregated into a single line.
Will do a followup that solves this problem.