[PATCH] D123678: [polly] migrate -polly-show to the new pass manager

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/include/llvm/Analysis/RegionPrinter.h:30
 
+  // this class has been implemented in LLVM RegionPrinter.cpp
+  template <>
----------------
Nothing unusual of a declaration in `RegionPrinter.h` to be implemented in `RegionPrinter.cpp`.


================
Comment at: polly/include/polly/ScopGraphPrinter.h:33
+template <>
+struct GraphTraits<ScopDetection> : public GraphTraits<RegionInfo *> {
+  static NodeRef getEntryNode(const ScopDetection &SD) {
----------------
Looking at other specializations, all GraphTraits of an analysis are a pointer of the the analysis (`GraphTraits<ScopDetection>`). Why the change? I fear that GraphTraits users may accidentally try to copy the object.


================
Comment at: polly/include/polly/ScopGraphPrinter.h:62
+
+  static std::string escapeString(std::string String);
+
----------------
Consider using `llvm::StringRef`


================
Comment at: polly/include/polly/ScopGraphPrinter.h:64-65
+
+  // Print the cluster of the subregions. This groups the single basic blocks
+  // and adds a different background color for each group.
+  static void printRegionCluster(const ScopDetection &SD, const Region *R,
----------------
Use doxygen comments for API documentation.


================
Comment at: polly/lib/Analysis/ScopGraphPrinter.cpp:95
 
-  // Print the cluster of the subregions. This groups the single basic blocks
-  // and adds a different background color for each group.
-  static void printRegionCluster(ScopDetection *SD, const Region *R,
-                                 raw_ostream &O, unsigned depth = 0) {
-    O.indent(2 * depth) << "subgraph cluster_" << static_cast<const void *>(R)
-                        << " {\n";
-    unsigned LineBegin, LineEnd;
-    std::string FileName;
+  if (const_cast<ScopDetection &>(SD).isMaxRegionInScop(*R)) {
+    O.indent(2 * (depth + 1)) << "style = filled;\n";
----------------
Consider `DOTGraphTraits<ScopDetection*>`


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

https://reviews.llvm.org/D123678