[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