[PATCH] D126826: [BOLT][NFC] Replace stdio with raw_ostream in CallGraph

[Amir Ayupov via Phabricator via llvm-commits]

Amir added a comment.

Please address the review comments and mark them as resolved.



================
Comment at: bolt/include/bolt/Passes/CallGraph.h:170
+  OS << "digraph g {\n";
+  for (NodeId F = 0; F < Nodes.size(); ++F) {
     if (Nodes[F].samples() == 0)
----------------
Let's keep iteration as is here and below


================
Comment at: bolt/include/bolt/Passes/CallGraph.h:173
       continue;
-    fprintf(File, "f%lu [label=\"%s\\nsamples=%u\\nsize=%u\"];\n", F,
-            GetLabel(F), Nodes[F].samples(), Nodes[F].size());
+    OS << "f" << (unsigned long)F << " [label=\"" << GetLabel(F)
+       << "\\nsamples=" << (unsigned int)(Nodes[F].samples())
----------------
Let's drop the explicit type cast here and below. 
I don't think it's needed given that 
- NodeId is an alias to `size_t`, 
- `Nodes[].samples()` returns `uint64_t`,
- `Nodes[].size()` returns `size_t`.


================
Comment at: bolt/include/bolt/Passes/CallGraph.h:177
   }
-  for (NodeId F = 0; F < Nodes.size(); F++) {
+
+  for (NodeId F = 0; F < Nodes.size(); ++F) {
----------------
Let's remove the added newline to make it easier to compare code side-by-side.


================
Comment at: bolt/include/bolt/Passes/CallGraph.h:190
+  OS << "}\n";
+  OS.close();
 }
----------------
No need to explicitly close OS as the object is local to the scope and the fd is closed by destructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126826