[PATCH] D15212: [PGO] Value profiling text format reader/writer support

David Li via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 12 15:55:15 PST 2015


davidxl marked 5 inline comments as done.

================
Comment at: lib/ProfileData/InstrProfReader.cpp:107
@@ -106,3 +106,3 @@
   StringRef buffer = Buffer.getBufferStart();
-  return count == 0 || std::all_of(buffer.begin(), buffer.begin() + count,
-    [](char c) { return ::isprint(c) || ::isspace(c); });
+  return count == 0 ||
+         std::all_of(buffer.begin(), buffer.begin() + count,
----------------
vsk wrote:
> I'd prefer to keep format changes for separate NFC commits.
This was fixed already.

================
Comment at: lib/ProfileData/InstrProfReader.cpp:127
@@ +126,3 @@
+  Line++;
+  for (uint32_t VK = 0; VK < NumValueKinds; VK++) {
+    if (Line.is_at_end())
----------------
vsk wrote:
> This looks correct but a bit repetitive. What do you think of using a macro like:
> 
>     #define AdvanceAndReadInt(Line, Dst) \
>         if ((Line).is_at_end()) \
>             return error(instrprof_error::truncated); \
>         if (((Line)++)->getAsInteger(10, (Dst)))
>             return error(instrprof_error::truncated); \
> 
> You could move the various "uint32_t" definitions up, or define them within the macro.
good suggestion.

================
Comment at: test/tools/llvm-profdata/Inputs/vp-malform.proftext:43
@@ +42,3 @@
+2
+foo+100
+foo2:1000
----------------
vsk wrote:
> It's not obvious that this is the error. Please add notes to your tests which make it clear what's being tested.
Added a comment before the malformed line


http://reviews.llvm.org/D15212





More information about the llvm-commits mailing list