[PATCH] D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added inline comments.


================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:409
 
+// This class implements --time-trace functionality for new pass manager.
+// It provides the pass-instrumentation callbacks that measure the pass
----------------
Could we use `///` here to get better doxygen appearances?


================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:413
+class TimeProfilingPassesHandler {
+
+public:
----------------
nit: can we move private members here (or using struct instead)


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1168
+    PassInstrumentationCallbacks &PIC) {
+  this->PIC = &PIC;
+  PIC.registerBeforeNonSkippedPassCallback(
----------------
nit: personally I would simply rename the local `PIC` rather than spelling out `this`


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1192
+void TimeProfilingPassesHandler::runAfterPass() {
+  if (getTimeTraceProfilerInstance() != nullptr) {
+    timeTraceProfilerEnd();
----------------
ditto the one above


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131960/new/

https://reviews.llvm.org/D131960