[PATCH] D15100: [PGO] Add support for reading multiple versions of indexed profile format profile data
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 1 11:06:44 PST 2015
vsk added a comment.
I like this approach. Some comments below --
================
Comment at: include/llvm/ProfileData/InstrProfReader.h:263
@@ +262,3 @@
+struct InstrProfReaderIndexBase {
+ // Read all the pofile records with the same key pointed to the current
+ // iterator.
----------------
nit, profile
================
Comment at: include/llvm/ProfileData/InstrProfReader.h:278
@@ +277,3 @@
+
+template <typename OnDiskHashTableImpl>
+class InstrProfReaderIndex : public InstrProfReaderIndexBase {
----------------
It'd make for less typing if we use `typename IndexType` and keep `std::unique_ptr<IndexType> Index` where you have `HashTable`.
================
Comment at: lib/ProfileData/InstrProfReader.cpp:538
@@ -534,3 +537,3 @@
// The rest of the file is an on disk hash table.
- Index.Init(Start + HashOffset, Cur, Start, HashType, FormatVersion);
-
+ InstrProfReaderIndexBase *IndexPtr = 0;
+ if (FormatVersion <= 2)
----------------
`nullptr`?
================
Comment at: lib/ProfileData/InstrProfReader.cpp:542
@@ +541,3 @@
+ Start + HashOffset, Cur, Start, HashType, FormatVersion);
+ else
+ llvm_unreachable("Only version 2 and below is supported");
----------------
I expected this to be an NFC commit, but this looks like breakage. InstrProfWriter uses IndexedInstrProf::Version (3).
http://reviews.llvm.org/D15100
More information about the llvm-commits
mailing list