[PATCH] D47858: [New PM] Introducing PassInstrumentation framework

[Philip Pfaffe via Phabricator via llvm-commits]

philip.pfaffe added a comment.

I like how simple this is now!



================
Comment at: include/llvm/IR/PassInstrumentation.h:199
+private:
+  mutable PassExecutionCounter PC;
+
----------------
Is `mutable` necessary now?


================
Comment at: include/llvm/IR/PassManager.h:919
                << "\n";
+
       AnalysisResultListT &ResultList = AnalysisResultLists[&IR];
----------------
Empty line?


================
Comment at: include/llvm/IR/PassManager.h:1209
+/// being processed.
+template <typename IRUnitT>
+class PassInstrumentationProxy
----------------
No need to make this a template class! Since `PassInstrumentation` isn't, anymore, this doesn't need to be either.


================
Comment at: include/llvm/Passes/PassBuilder.h:568
+      const std::function<void(PassInstrumentation &)> &C) {
+    assert(PI &&
+           "PassInstrumentation object is not available for registration");
----------------
I don't think this should assert. This can be called from a plugin context, and the plugin has no chance of knowing whether `PassInstrumentation` is available. So in that case, just do nothing I guess. 


Repository:
  rL LLVM

https://reviews.llvm.org/D47858