[PATCH] D51274: [NFC][PassTiming] factor out generic PassTimingInfo

[Jessica Paquette via Phabricator via llvm-commits]

paquette accepted this revision.
paquette added a comment.
This revision is now accepted and ready to land.

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.



================
Comment at: include/llvm/IR/PassTimingInfo.h:29-33
+/// This template class provides an generic interface for collection of pass
+/// timing info for use in pass managers. Current implementation provides
+/// specializations for two kinds of PassInfoT:
+///   PassInfo* - for legacy pass manager
+///   StringRef - for new pass manager
----------------
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.
```


================
Comment at: include/llvm/IR/PassTimingInfo.h:39-40
+public:
+  /// Constructing default time info.
+  /// Use 'init' member to activate this.
+  PassTimingInfo();
----------------
Might make more sense to say something like

```
Default constructor. Use \p init() to for initialization.
```


================
Comment at: include/llvm/IR/PassTimingInfo.h:46-48
+  /// This method either initializes the TheTimeInfo pointer
+  /// to a non-null value (if the -time-passes option is enabled) or it leaves
+  /// it null.  It may be called multiple times.
----------------
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.
```


================
Comment at: lib/IR/PassTimingInfo.cpp:52-58
+// TimingDtor - Print out information about timing information
+template <typename PassT> PassTimingInfo<PassT>::~PassTimingInfo() {
+  // Delete all of the timers, which accumulate their info into the
+  // TimerGroup.
+  for (auto &I : TimingData)
+    delete I.getValue();
+  // TimerGroup is deleted next, printing the report.
----------------
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.
```


================
Comment at: lib/IR/PassTimingInfo.cpp:61-64
+/// This method either initializes the TheTimeInfo pointer to a non-null value
+/// (if the -time-passes option is enabled) or it leaves it null.
+///
+/// It may be called multiple times.
----------------
The `init()` in PassTimingInfo.h already has this comment. So, this one can be removed.


Repository:
  rL LLVM

https://reviews.llvm.org/D51274