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

Min-Yih Hsu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 19 21:09:31 PDT 2022


myhsu added a comment.

Thanks for the patch, lgtm for most of the parts. Please address my comment though (especially the `static` one)



================
Comment at: llvm/include/llvm/Analysis/DOTGraphTraitsPass.h:32
+template <typename GraphT>
+static void runViewerImpl(Function &F, GraphT &Graph, StringRef Name,
+                          bool IsSimple) {
----------------
I think `static` is redundant in this case.
Also, in LLVM, "XXXImpl" sounds more like a name for internal / implementation-specific code, but this function is global. I would recommend changing this (and runPrinterImpl) into something more general, like "viewGraphForFunction".


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

https://reviews.llvm.org/D123677



More information about the llvm-commits mailing list