[PATCH] D126826: [BOLT][NFC] Replacing stdio functions with raw_ostream functions

[Amir Ayupov via Phabricator via llvm-commits]

Amir added a comment.

Looking good. Let's address some issues:

1. Please retitle the diff as "[BOLT][NFC] Replace stdio with raw_ostream in CallGraph". The commit title should be a declarative statement, and explicit about where the change is made.
2. While we're on it, let's add an option to use `printDot` method. Please use an example of `PrintCFG` option.
3. Inline comments.



================
Comment at: bolt/include/bolt/Passes/CallGraph.h:19
+#include <iomanip>
+#include <string>
 #include <unordered_set>
----------------
Are all of the new includes here really needed? Can you please prune the added includes as much as possible?


================
Comment at: bolt/include/bolt/Passes/CallGraph.h:169
+  std::error_code EC;
+  raw_fd_ostream OS(std::string(FileName), EC, llvm::sys::fs::OF_None);
+  if (EC)
----------------



================
Comment at: bolt/include/bolt/Passes/CallGraph.h:174
+  OS << "digraph g {\n";
+  for (auto F = 0; F < Nodes.size(); ++F) {
     if (Nodes[F].samples() == 0)
----------------
Please keep the type as is - there has been an effort to reduce the number of `auto`'s in the codebase. LLVM coding style advises to be explicit about type in most cases: https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable


================
Comment at: bolt/include/bolt/Passes/CallGraph.h:187
       ArcConstIterator Arc = findArc(F, Dst);
-      fprintf(
-          File,
-          "f%lu -> f%u [label=\"normWgt=%.3lf,weight=%.0lf,callOffset=%.1lf\"];"
-          "\n",
-          F, Dst, Arc->normalizedWeight(), Arc->weight(), Arc->avgCallOffset());
+      OS << "f" << (unsigned long)F << " -> f" << (unsigned)Dst
+         << " [label=\"normWgt=" << (double)(Arc->normalizedWeight())
----------------
You can preserve the requested precision using the `format` method described here: https://llvm.org/doxygen/Format_8h_source.html (see comment on the top).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126826