[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