[PATCH] D123677: [PassManager] Implement DOTGraphTraitsViewer under NPM

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

The usual scheme for legacy pass names is usually to append either `WrapperPass` (such as `DominatorTreeWrapperPass`) or `LegacyPass` (such as `CFGPrinterLegacyPass`). Could you use one of those conventions?

Could you upload the patch with context (`git diff -U999999`, see https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)

`DOTGraphTraitsPrinter` is not used in this patch. Do you plan to also introduce NPM passes for `DomPrinter` etc that derive from `DOTGraphTraitsViewer`?



================
Comment at: llvm/include/llvm/Analysis/DOTGraphTraitsPass.h:92-112
+    auto &Result = FAM.getResult<AnalysisT>(F);
+    if (!processFunction(F, Result))
+      return PreservedAnalyses::all();
+
+    GraphT Graph = AnalysisGraphTraitsT::getGraph(Result);
+    std::string Filename = Name + "." + F.getName().str() + ".dot";
+    std::error_code EC;
----------------
The function is now duplicated with `LegacyDOTGraphTraitsPrinter::runOnFunction`. You could either move it out into a common function, are call this NPM run function from the legacy passes' `runOnFunction`. See e.g. `InstSimplifyLegacyPass::runOnFunction` how this was was done.


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

https://reviews.llvm.org/D123677