[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