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

[Chandler Carruth via llvm-commits]

chandlerc requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: include/llvm/InitializePasses.h:118-126
@@ -117,18 +117,11 @@
 void initializeDomViewerPass(PassRegistry&);
-void initializeDominanceFrontierWrapperPassPass(PassRegistry&);
-void initializeDominatorTreeWrapperPassPass(PassRegistry&);
-void initializeEarlyIfConverterPass(PassRegistry&);
-void initializeEdgeBundlesPass(PassRegistry&);
-void initializeExpandPostRAPass(PassRegistry&);
+void initializeDominanceFrontierWrapperPassPass(PassRegistry &);
+void initializeDominatorTreeWrapperPassPass(PassRegistry &);
+void initializeEarlyIfConverterPass(PassRegistry &);
+void initializeEdgeBundlesPass(PassRegistry &);
+void initializeExpandPostRAPass(PassRegistry &);
 void initializeAAResultsWrapperPassPass(PassRegistry &);
-void initializeGCOVProfilerPass(PassRegistry&);
-void initializePGOInstrumentationGenPass(PassRegistry&);
-void initializePGOInstrumentationUsePass(PassRegistry&);
-void initializeInstrProfilingPass(PassRegistry&);
-void initializeAddressSanitizerPass(PassRegistry&);
-void initializeAddressSanitizerModulePass(PassRegistry&);
-void initializeMemorySanitizerPass(PassRegistry&);
-void initializeThreadSanitizerPass(PassRegistry&);
-void initializeSanitizerCoverageModulePass(PassRegistry&);
-void initializeDataFlowSanitizerPass(PassRegistry&);
-void initializeScalarizerPass(PassRegistry&);
+void initializeGCOVProfilerPass(PassRegistry &);
+void initializePGOInstrumentationGenPass(PassRegistry &);
+void initializePGOInstrumentationUsePass(PassRegistry &);
+void initializeInstrProfilingLegacyWrapperPassPass(PassRegistry &);
----------------
Please don't introduce lots of unrelated formatting changes.

================
Comment at: include/llvm/LinkAllPasses.h:85-93
@@ -84,20 +84,11 @@
       (void) llvm::createDeadStoreEliminationPass();
-      (void) llvm::createDependenceAnalysisPass();
-      (void) llvm::createDivergenceAnalysisPass();
-      (void) llvm::createDomOnlyPrinterPass();
-      (void) llvm::createDomPrinterPass();
-      (void) llvm::createDomOnlyViewerPass();
-      (void) llvm::createDomViewerPass();
-      (void) llvm::createGCOVProfilerPass();
-      (void) llvm::createPGOInstrumentationGenPass();
-      (void) llvm::createPGOInstrumentationUsePass();
-      (void) llvm::createInstrProfilingPass();
-      (void) llvm::createFunctionImportPass();
-      (void) llvm::createFunctionInliningPass();
-      (void) llvm::createAlwaysInlinerPass();
-      (void) llvm::createGlobalDCEPass();
-      (void) llvm::createGlobalOptimizerPass();
-      (void) llvm::createGlobalsAAWrapperPass();
-      (void) llvm::createIPConstantPropagationPass();
-      (void) llvm::createIPSCCPPass();
-      (void) llvm::createInductiveRangeCheckEliminationPass();
+      (void)llvm::createDependenceAnalysisPass();
+      (void)llvm::createDivergenceAnalysisPass();
+      (void)llvm::createDomOnlyPrinterPass();
+      (void)llvm::createDomPrinterPass();
+      (void)llvm::createDomOnlyViewerPass();
+      (void)llvm::createDomViewerPass();
+      (void)llvm::createGCOVProfilerPass();
+      (void)llvm::createPGOInstrumentationGenPass();
+      (void)llvm::createPGOInstrumentationUsePass();
+      (void)llvm::createInstrProfilingLegacyPass();
----------------
Here too.

================
Comment at: include/llvm/Transforms/Instrumentation.h:101
@@ -97,1 +100,3 @@
 
+class InstrProfiling {
+public:
----------------
I wouldn't put all of this into the common Instrumentation.h file. Instead, like with all the other pass ports, please add a header for this specific pass. That may also help address your concerns with the design.

================
Comment at: include/llvm/Transforms/Instrumentation.h:174-177
@@ +173,6 @@
+
+/// Instrumenation based profiling lowering pass. This pass lowers
+/// the profile instrumented code generated by FE or the IR based
+/// instrumentation pass.
+struct InstrProfilingPass : PassInfoMixin<InstrProfilingPass> {
+  InstrProfiling InstrProf;
----------------
My suggestion was not to keep these separate, but to actually have a single class which *is* the new pass manager pass, and can be embedded (like InstrProfiling) inside the legacy pass...

================
Comment at: lib/Passes/PassRegistry.def:41
@@ -40,2 +40,3 @@
 MODULE_PASS("invalidate<all>", InvalidateAllAnalysesPass())
+MODULE_PASS("instrprof", InstrProfilingPass())
 MODULE_PASS("no-op-module", NoOpModulePass())
----------------
Pretty sure this should be before "invalidate"?

================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:34
@@ -33,2 +33,3 @@
 
-class InstrProfiling : public ModulePass {
+class InstrProfilingLegacyWrapperPass : public ModulePass {
+  InstrProfiling InstrProf;
----------------
This isn't a wrapper pass. Those are passes that wrap an analysis result. This is just a transformation pass.


http://reviews.llvm.org/D18126