[PATCH] D18126: Port InstrProfiling pass to new pass manager

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

See below for how to thread options into a pass with the new pass manager. Note that the passes library that will replace the pass manager builder in the new pass manager may not have the same set of hooks that Clang is using, so there may need to be some new APIs introduced to make this fully functional, but that can be done in follow-up patches.


================
Comment at: include/llvm/Transforms/Instrumentation.h:99
@@ -97,1 +98,3 @@
 
+/// Instrumenation based profiling loewring pass.
+struct InstrProfilingPass : PassInfoMixin<InstrProfilingPass> {
----------------
typo: loewring -> lowering

Since you're lifting this into a header, it'd be good to provide some more expansive comments.

================
Comment at: lib/Passes/PassRegistry.def:43
@@ -42,2 +42,3 @@
 MODULE_PASS("print", PrintModulePass(dbgs()))
+MODULE_PASS("instrprof", InstrProfilingPass())
 MODULE_PASS("print-callgraph", CallGraphPrinterPass(dbgs()))
----------------
Keep this list sorted?

================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:34-37
@@ -33,14 +33,6 @@
 
-class InstrProfiling : public ModulePass {
+class InstrProfiling {
 public:
-  static char ID;
-
-  InstrProfiling() : ModulePass(ID) {}
-
-  InstrProfiling(const InstrProfOptions &Options)
-      : ModulePass(ID), Options(Options) {}
-
-  const char *getPassName() const override {
-    return "Frontend instrumentation-based coverage lowering";
-  }
+  InstrProfiling() {}
+  InstrProfiling(const InstrProfOptions &Options) : Options(Options) {}
 
----------------
Why not make this the actual new pass manager class? That's how most of the passes with stat work. You can see SROA and recently GVN for examples (admitedly a bit more complex).

This will also show the most basic part of how to parameterize it - the constructors here are exactly what you want to be able to construct this pass out of specific options.



http://reviews.llvm.org/D18126