[PATCH] D80711: Utility to dump .dot representation of SelectionDAG without firing viewer

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SelectionDAG.h:442
+  /// Error will be emitted if the path is insane.
+  void dumpDotGraph(const std::string &FileName, const Twine &Title);
+
----------------
I would expect the filename to be a StringRef (I guess debuggers struggle more with this, and the others are using std::string)


================
Comment at: llvm/include/llvm/Support/GraphWriter.h:344-345
       return "";
+    } else {
+      errs() << "writing to the newly created file " << Filename << "\n";
     }
----------------
The existing done I think was sufficient


================
Comment at: llvm/include/llvm/Support/GraphWriter.h:366-369
+  std::string Filename = llvm::WriteGraph(G, Name, ShortNames, Title, FileName);
+
+  if (Filename.empty())
+    return;
----------------
Don't see the point of getting the return string and check if it's empty?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp:171
+// This code is only for debugging!
+#ifndef NDEBUG
+  DumpDotGraph(this, FileName, Title);
----------------
Should this fully remove the code depending on LLVM_ENABLE_DUMP? Also the declaration should get LLVM_DUMP_METHOD?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80711