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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:34-38
@@ -33,14 +33,7 @@
 
-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) {}
 
+  bool run(Module &M);
----------------
chandlerc wrote:
> If you want to create a per-run class that holds the state for each run, that's fine, but I think it would be better done as a separate change.
> 
> With the legacy pass manager the state for a single run is already kept inside the pass object and cleared between runs. My suggestion is to start off without changing that design, and if there is a desire to change it, do so as a follow-up change because they are two orthogonal aspects of the design.
If you insists this as a separate change, I can do it.

What you said about legacy pass manager is true, but the difference is that with new pass manager, I need to pull lots of implementation details into the common header file which should be avoided.  I am not sure about the implication on the design you mentioned here. 


http://reviews.llvm.org/D18126