[PATCH] D14894: [PGO] Add --text-format option for llvm-profdata show|merge commands

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 22 23:35:53 PST 2015


vsk added a comment.

Looks good with minor nits


================
Comment at: docs/CommandGuide/llvm-profdata.rst:132
@@ -121,2 +131,3 @@
 
- Specify that the input profile is an instrumentation-based profile.
+ Specify that the input profile is an instrumentation-based profile. When
+ instrumentation-based profile is specified, the profile counters can be
----------------
Please move your description under the `.. option:: -text-format` heading. The rest of the descriptions here start with an imperative, so maybe we can use "Enables ..."?

================
Comment at: lib/ProfileData/InstrProfWriter.cpp:177
@@ +176,3 @@
+  OS << Func.Name << "\n" << Func.Hash << "\n" << Func.Counts.size() << "\n";
+  for (size_t I = 0, E = Func.Counts.size(); I < E; ++I)
+    OS << Func.Counts[I] << "\n";
----------------
`for (uint64_t Count : Func.Counts)`?

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:35
@@ -35,1 +34,3 @@
+
+static void exitWithError(const Twine &Message, StringRef Whence = "",
                           StringRef Hint = "") {
----------------
Could you handle this in a follow-up commit?


http://reviews.llvm.org/D14894





More information about the llvm-commits mailing list