[PGO] Make indexed value profile data more compact and add structural definitions for the data format

[davidxl]

Summary

The patch include the following changes:

• Make indexed value profile data more compact by peeling out the per-site value count field into its own smaller sized array.
• Introduced formal data structure definitions to specify value profile data layout in indexed format. Previously the layout of the data is only assumed in the client code (scattered in three different places : size computation, EmitData, and ReadData) which can be out of sync.
• The bulk of reading/writing is the implementation of serialization/deserialization of the value profile data: interfaces that convert between on-disk ValueProfData and InstrProfRecord class.
• The new data structure also serves as a central place for layout documentation.

[davidxl]

Force llvm style when using clang-format.

[betulb]

It looks fine to me overall. I'll need to do the testing when I'm rebasing my pending CL's.

[davidxl]

thanks. Let me update the patch first -- there were some conflicting format
changes.

David

[davidxl]

Rebase patch.

[betulb]

+1

[vsk]

Thanks! I have a few inline comments.

As I understand it, we need to maintain backwards compatibility for the indexed data format. Could you commit a binary blob which uses the old indexed format (in which value site counts are right next to value kinds), and show that we can still read it?

include/llvm/ProfileData/InstrProf.h

415

recordfor -> record for

419

SiteCountArray has length NumValueSites, but it's declared here with length 1. Your patch also stores uint32_t into this array, which is an array of uint8_t.

I was confused by this when I first read through your patch. Could you change this to uint32_t SiteCountArray[]? If you mean for this to be an array of uint8_t, please change the return type of getNumValueDataForSite.

421

I think the convention is to use #if 0 for this in llvm.

469

Same nit as above.

543

Do you have any plans for pulling in this definition into compiler-rt, and removing the duplicate definition there?

lib/ProfileData/InstrProfReader.cpp

25

I think this line would have to change to sizeof(ValueProfRecord) + sizeof(uint8_t) * NumValueSites if you take my earlier suggestion. I think it would be cleaner this way.

[davidxl]

There is no need for binary blob testing -- the runtime and raw format change is still pending, so this is no need to keep the backward compatibility. Besides the patch basically removes the old format support, it will break with the test.

include/llvm/ProfileData/InstrProf.h

419

uint8_t is an implementation detail (where the packing comes from). In practice, most of the value sites are cold with zero value tracked and large number of tracked values will be useless for compiler and is a waste of space, this is why the runtime will guarantee (change by Betul under review) the value count for each site is limited.

However I think the interface should still use uint32_t. Also note that the SiteCountArray is variable length, thus it is declared with element == 1.

543

Yes -- there is a plan for that.

lib/ProfileData/InstrProfReader.cpp

25

There was a bug there that may waste space. Changed to use offsetof.

[davidxl]

Update the patch according to comments.

[vsk]

IIUC, when the raw format and runtime changes land, we will need a backwards compatibility test for the indexed format?

A few more inline comments --

include/llvm/ProfileData/InstrProf.h

419

Ah ok.

When Betul's patch lands, we should really consider being consistent about using uint8_t for site counts everywhere, as it's a bit more accurate.

I see that SiteCountArray is variable length, but that's not obvious from a superficial reading of the code. It's declared as an array of length 1, which is surprising.

Declaring it as [] conveys the meaning more directly. Alternatively, you could define a method getSiteCountArray() { return ((uint8_t *)this) + sizeof(ValueProfRecord); }, and move the SiteCountArray[] declaration into the #if 0 block.

lib/ProfileData/InstrProfReader.cpp

121

The use of sizeof(uint64_t) in this function isn't self-explanatory. Afaict sizeof(ValueProfData) would work at least as well, and is a lot clearer.

126

Can you remove this union, or lift it into a more descriptive comment?

133

This would be much easier to understand as if (TotalSize % sizeof(ValueProfData)), which gets optimized properly later.

[davidxl]

include/llvm/ProfileData/InstrProf.h

419

The comment says the number of element is NumValueSites. Declaring the array with 1 element serves the purpose of indicating that there is at least one site profiled. Just added a comment that there is one element guaranteed.

lib/ProfileData/InstrProfReader.cpp

126

Added a comment.

133

changed to TotalSize % sizeof(uint64_t) and added comment.

[davidxl]

yes -- binary blob testing can be added once the feature is turned on (with all pieces ready and possibly after a major public clang release with the feature is announced) -- until then, we still have some flexibility to tune the layout without the need to worry about incompatible changes.

[davidxl]

Update the patch according to Vedant's comments.

[vsk]

It'd be nice to have blob testing done before a major release, but that sounds like a discussion for llvm-dev. Lgtm with a tiny nit.

include/llvm/ProfileData/InstrProf.h

420

nit: inthe -> in the

[davidxl]

1. addressed Justin's comments
2. consolidate byte swap code for reader/writer
3. added byte swap related interfaces purely for the purpose of testing

4. increased test coverage by added a new unittest that streams in/out value profile data in big endian order and force byte swapping to happen.

[davidxl]

Update the patch:

1. Add tags in doxygen comments
2. Move ValueProfData implementation from InstrProfReader.cpp to InstrProf.cpp as it is a shared structure between reader and writer.

include/llvm/ProfileData/InstrProf.h

 
 #ifndef LLVM_PROFILEDATA_INSTRPROF_H_
 #define LLVM_PROFILEDATA_INSTRPROF_H_
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MD5.h"
 #include <cstdint>
 #include <list>
 #include <system_error>
 #include <vector>
 
 namespace llvm {
 

 
 uint32_t InstrProfRecord::getNumValueKinds() const {
   uint32_t NumValueKinds = 0;
   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
     NumValueKinds += !(getValueSitesForKind(Kind).empty());
   return NumValueKinds;
 }
 
+uint32_t InstrProfRecord::getNumValueData(uint32_t ValueKind) const {
+  uint32_t N = 0;
+  const std::vector<InstrProfValueSiteRecord> &SiteRecords =
+      getValueSitesForKind(ValueKind);
+  for (auto &SR : SiteRecords) {
+    N += SR.ValueData.size();
+  }
+  return N;
+}
+
 uint32_t InstrProfRecord::getNumValueSites(uint32_t ValueKind) const {
   return getValueSitesForKind(ValueKind).size();
 }
 
 uint32_t InstrProfRecord::getNumValueDataForSite(uint32_t ValueKind,
                                                  uint32_t Site) const {
   return getValueSitesForKind(ValueKind)[Site].ValueData.size();
 }
static inline uint64_t ComputeHash(HashT Type, StringRef K) {
   }
   llvm_unreachable("Unhandled hash type");
 }
 
 const uint64_t Magic = 0x8169666f72706cff;  // "\xfflprofi\x81"
 const uint64_t Version = 3;
 const HashT HashType = HashT::MD5;
 
+// This structure defines the file header of the LLVM profile
+// data file in indexed-format.
 struct Header {
   uint64_t Magic;
   uint64_t Version;
   uint64_t MaxFunctionCount;
   uint64_t HashType;
   uint64_t HashOffset;
 };
 
+// This is the header of the data structure that defines the on-disk
+// layout of the value profile data of a particular kind for one function.
+struct ValueProfRecord {
+  // The kind of the value profile record.
+  uint32_t Kind;
+  // The number of value profile sites. It is guaranteed to be non-zero;
+  // otherwise the recordfor this kind won't be emitted.
+  uint32_t NumValueSites;
+  // The first element of the array that stores the number of profiled
+  // values for each value site. The size of the array is NumValueSites.
+  uint8_t SiteCountArray[1];
+
+#if DO_NOT_DEFINE_ME
+  // The fake declaration is for documentation purpose only.
+  // Align the start of next field to be on 8 byte boundaries.
+  uint8_t Padding[X];
+
+  // The array of value profile data. The size of the array is the sum
+  // of all elements in SiteCountArray[].
+  InstrProfValueData ValueData[];
+#endif
+
+  /// Return the ValueProfRecord header size including the padding bytes.
+  static uint32_t getHeaderSize(uint32_t NumValueSites);
+  /// Return the total size of the value profile record including the
+  /// header and the value data.
+  static uint32_t getSize(uint32_t NumValueSites, uint32_t NumValueData);
+  /// Return the total size of the value profile record including the
+  /// header and the value data.
+  uint32_t getSize() const { return getSize(NumValueSites, getNumValueData()); }
+  /// Use this method to advance to the next ValueProfRecord.
+  ValueProfRecord *getNext();
+  /// Return the pointer to the first value profile data.
+  InstrProfValueData *getValueData();
+  /// Return the number of value sites.
+  uint32_t getNumValueSites() const { return NumValueSites; }
+  /// Return the number of value data.
+  uint32_t getNumValueData() const;
+  /// Read data from this record and save it to Record.
+  void deserializeTo(InstrProfRecord &Record,
+                     InstrProfRecord::ValueMapType *VMap);
+  /// Extract data from Record and serialize into this instance.
+  void serializeFrom(const InstrProfRecord &Record, uint32_t ValueKind,
+                     uint32_t NumValueSites);
+};
+
+// Per-function header/control data structure for value profiling
+// data in indexed-format.
+struct ValueProfData {
+  // Total size in bytes including this field. It must be a multiple
+  // of sizeof(uint64_t).
+  uint32_t TotalSize;
+  // The number of value profile kinds that has value profile data.
+  // In this implementation, a value profile kind is considered to
+  // have profile data if the number of value profile sites for the
+  // kind is not zero. More aggressively, the implemnetation can
+  // choose to check the actual data value: if none of the value sites
+  // has any profiled values, the kind can be skipped.
+  uint32_t NumValueKinds;
+
+#if DO_NOT_DEFINE_ME
+  // Following are a sequence of variable length records. The prefix/header
+  // of each record is defined by ValueProfRecord type. The number of
+  // records is NumValueKinds.
+  ValueProfRecord Record_1;
+  ..ValueProfRecord Record_N;
+#endif
+
+  /// Return the total size in bytes of the on-disk value profile data
+  /// given the data stored in Record.
+  static uint32_t getSize(const InstrProfRecord &Record);
+  /// Return a pointer to ValueProfData instance ready to be streamed.
+  static std::unique_ptr<ValueProfData> serializeFrom(
+      const InstrProfRecord &Record);
+  /// Return a pointer to ValueProfileData instance ready to be read.
+  /// All data in the instance are properly swapped.
+  static ErrorOr<std::unique_ptr<ValueProfData>> swapBytes(
+      const unsigned char *D, const unsigned char *const BufferEnd);
+
+  /// Return the total size of the ValueProfileData.
+  uint32_t getSize() const { return TotalSize; }
+  /// Read data from this data and save it to Record.
+  void deserializeTo(InstrProfRecord &Record,
+                     InstrProfRecord::ValueMapType *VMap);
+  /// Return the first ValueProfRecord instance.
+  ValueProfRecord *getFirstValueProfRecord();
+};
+
 }  // end namespace IndexedInstrProf
 
 namespace RawInstrProf {
 
 const uint64_t Version = 1;
 
 // Magic number to detect file format and endianness.
 // Use 255 at one end, since no UTF-8 file can use that character.  Avoid 0,

 
 template <>
 inline uint64_t getMagic<uint32_t>() {
   return uint64_t(255) << 56 | uint64_t('l') << 48 | uint64_t('p') << 40 |
          uint64_t('r') << 32 | uint64_t('o') << 24 | uint64_t('f') << 16 |
          uint64_t('R') << 8 | uint64_t(129);
 }
 
+// Per-function profile data header/control structure.
 // The definition should match the structure defined in
 // compiler-rt/lib/profile/InstrProfiling.h.
 // It should also match the synthesized type in
 // Transforms/Instrumentation/InstrProfiling.cpp:getOrCreateRegionCounters.
-
 template <class IntPtrT>
 struct ProfileData {
   #define INSTR_PROF_DATA(Type, LLVMType, Name, Init) Type Name;
   #include "llvm/ProfileData/InstrProfData.inc"
 };
 
+// File header structure of the LLVM profile data in raw format.
 // The definition should match the header referenced in
 // compiler-rt/lib/profile/InstrProfilingFile.c  and
 // InstrProfilingBuffer.c.
-
 struct Header {
   const uint64_t Magic;
   const uint64_t Version;
   const uint64_t DataSize;
   const uint64_t CountersSize;
   const uint64_t NamesSize;
   const uint64_t CountersDelta;
   const uint64_t NamesDelta;
 };
 
 }  // end namespace RawInstrProf
 
 namespace coverage {
 
+// Profile coverage map has the following layout:
+// [CoverageMapFileHeader]
+// [ArrayStart]
+//  [CovMapFunctionRecord]
+//  [CovMapFunctionRecord]
+//  ...
+// [ArrayEnd]
+// [Encoded Region Mapping Data]
 LLVM_PACKED_START
 template <class IntPtrT>
 struct CovMapFunctionRecord {
   #define COVMAP_FUNC_RECORD(Type, LLVMType, Name, Init) Type Name;
   #include "llvm/ProfileData/InstrProfData.inc"
 };
 LLVM_PACKED_END
 

lib/ProfileData/InstrProfReader.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/ProfileData/InstrProfReader.h"
 #include "llvm/ADT/STLExtras.h"
 #include <cassert>
 
 using namespace llvm;
 
+namespace llvm {
+namespace IndexedInstrProf {
+
+uint32_t ValueProfRecord::getHeaderSize(uint32_t NumValueSites) {
+  size_t Size = sizeof(ValueProfRecord) + sizeof(uint8_t) * (NumValueSites - 1);
+  // Round the size to multiple of 8 bytes.
+  Size = (Size + 7) & ~7;
+  return Size;
+}
+
+uint32_t ValueProfRecord::getSize(uint32_t NumValueSites,
+                                  uint32_t NumValueData) {
+  return getHeaderSize(NumValueSites) +
+         sizeof(InstrProfValueData) * NumValueData;
+}
+
+void ValueProfRecord::deserializeTo(InstrProfRecord &Record,
+                                    InstrProfRecord::ValueMapType *VMap) {
+  Record.reserveSites(Kind, NumValueSites);
+
+  InstrProfValueData *ValueData = this->getValueData();
+  for (uint64_t VSite = 0; VSite < NumValueSites; ++VSite) {
+    uint8_t ValueDataCount = this->SiteCountArray[VSite];
+    Record.addValueData(Kind, VSite, ValueData, ValueDataCount, VMap);
+    ValueData += ValueDataCount;
+  }
+}
+
+void ValueProfRecord::serializeFrom(const InstrProfRecord &Record,
+                                    uint32_t ValueKind,
+                                    uint32_t NumValueSites) {
+  Kind = ValueKind;
+  this->NumValueSites = NumValueSites;
+  InstrProfValueData *DstVD = getValueData();
+  for (uint32_t S = 0; S < NumValueSites; S++) {
+    uint32_t ND = Record.getNumValueDataForSite(ValueKind, S);
+    SiteCountArray[S] = ND;
+    std::unique_ptr<InstrProfValueData[]> SrcVD =
+        Record.getValueForSite(ValueKind, S);
+    for (uint32_t I = 0; I < ND; I++) {
+      DstVD[I] = SrcVD[I];
+      if (ValueKind == IPVK_IndirectCallTarget)
+        DstVD[I].Value = ComputeHash(HashType, (const char *)DstVD[I].Value);
+    }
+    DstVD += ND;
+  }
+}
+
+uint32_t ValueProfData::getSize(const InstrProfRecord &Record) {
+  uint32_t TotalSize = sizeof(ValueProfData);
+  uint32_t NumValueKinds = Record.getNumValueKinds();
+  if (NumValueKinds == 0) return TotalSize;
+
+  for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; Kind++) {
+    uint32_t NumValueSites = Record.getNumValueSites(Kind);
+    if (!NumValueSites) continue;
+    TotalSize +=
+        ValueProfRecord::getSize(NumValueSites, Record.getNumValueData(Kind));
+  }
+  return TotalSize;
+}
+
+void ValueProfData::deserializeTo(InstrProfRecord &Record,
+                                  InstrProfRecord::ValueMapType *VMap) {
+  if (NumValueKinds == 0) return;
+
+  ValueProfRecord *VR = getFirstValueProfRecord();
+  for (uint32_t K = 0; K < NumValueKinds; K++) {
+    VR->deserializeTo(Record, VMap);
+    VR = VR->getNext();
+  }
+}
+
+std::unique_ptr<ValueProfData> ValueProfData::serializeFrom(
+    const InstrProfRecord &Record) {
+  uint32_t TotalSize = getSize(Record);
+  std::unique_ptr<ValueProfData> VPD(
+      reinterpret_cast<ValueProfData *>(new char[TotalSize]));
+
+  VPD->TotalSize = TotalSize;
+  VPD->NumValueKinds = Record.getNumValueKinds();
+  ValueProfRecord *VR = VPD->getFirstValueProfRecord();
+  for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; Kind++) {
+    uint32_t NumValueSites = Record.getNumValueSites(Kind);
+    if (!NumValueSites) continue;
+    VR->serializeFrom(Record, Kind, NumValueSites);
+    VR = VR->getNext();
+  }
+  return VPD;
+}
+
+ErrorOr<std::unique_ptr<ValueProfData>> ValueProfData::swapBytes(
+    const unsigned char *D, const unsigned char *const BufferEnd) {
+  using namespace support;
+  if (D + sizeof(uint64_t) > BufferEnd) return instrprof_error::truncated;
+  union {
+    ValueProfData Header;
+    uint64_t RawHeader;
+  };
+  RawHeader = endian::byte_swap<uint64_t, little>(*D);
+  uint32_t TotalSize = Header.TotalSize;
+  if (D + TotalSize > BufferEnd) return instrprof_error::too_large;
+  if (Header.NumValueKinds > IPVK_Last) return instrprof_error::malformed;
+  if (TotalSize & (sizeof(uint64_t) - 1)) return instrprof_error::malformed;
+
+  std::unique_ptr<ValueProfData> VPD(
+      reinterpret_cast<ValueProfData *>(new char[TotalSize]));
+
+  for (uint32_t I = 0; I < TotalSize / sizeof(uint64_t); I++)
+    reinterpret_cast<uint64_t *>(VPD.get())[I] =
+        endian::byte_swap<uint64_t, little>(
+            reinterpret_cast<const uint64_t *>(D)[I]);
+
+  // Data integrety check:
+  ValueProfRecord *VR = VPD->getFirstValueProfRecord();
+  for (uint32_t K = 0; K < VPD->NumValueKinds; K++) {
+    if (VR->Kind > IPVK_Last) return instrprof_error::malformed;
+    VR = VR->getNext();
+    if ((char *)VR - (char *)VPD.get() > TotalSize)
+      return instrprof_error::malformed;
+  }
+
+  return std::move(VPD);
+}
+
+ValueProfRecord *ValueProfData::getFirstValueProfRecord() {
+  return reinterpret_cast<ValueProfRecord *>((char *)this +
+                                             sizeof(ValueProfData));
+}
+
+uint32_t ValueProfRecord::getNumValueData() const {
+  uint32_t NumValueData = 0;
+  for (uint32_t I = 0; I < NumValueSites; I++)
+    NumValueData += SiteCountArray[I];
+  return NumValueData;
+}
+
+ValueProfRecord *ValueProfRecord::getNext() {
+  return reinterpret_cast<ValueProfRecord *>((char *)this + getSize());
+}
+
+InstrProfValueData *ValueProfRecord::getValueData() {
+  return reinterpret_cast<InstrProfValueData *>((char *)this +
+                                                getHeaderSize(NumValueSites));
+}
+
+}  // End of IndexedInstrProf namespace.
+}  // End of llvm namespace.
+
 static ErrorOr<std::unique_ptr<MemoryBuffer>>
 setupMemoryBuffer(std::string Path) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
       MemoryBuffer::getFileOrSTDIN(Path);
   if (std::error_code EC = BufferOrErr.getError())
     return EC;
   return std::move(BufferOrErr.get());
 }
InstrProfLookupTrait::ComputeHash(StringRef K) {
   return IndexedInstrProf::ComputeHash(HashType, K);
 }
 
 typedef InstrProfLookupTrait::data_type data_type;
 typedef InstrProfLookupTrait::offset_type offset_type;
 
 bool InstrProfLookupTrait::ReadValueProfilingData(
     const unsigned char *&D, const unsigned char *const End) {
+  ErrorOr<std::unique_ptr<IndexedInstrProf::ValueProfData>> VDataPtrOrErr =
+      IndexedInstrProf::ValueProfData::swapBytes(D, End);
 
-  using namespace support;
-  // Read number of value kinds with value sites.
-  if (D + sizeof(uint64_t) > End)
-    return false;
-  uint64_t ValueKindCount = endian::readNext<uint64_t, little, unaligned>(D);
-
-  InstrProfRecord &ProfRecord = DataBuffer.back();
-  for (uint32_t Kind = 0; Kind < ValueKindCount; ++Kind) {
-
-    // Read value kind and number of value sites for kind.
-    if (D + 2 * sizeof(uint64_t) > End)
-      return false;
-
-    uint64_t ValueKind = endian::readNext<uint64_t, little, unaligned>(D);
-    uint64_t ValueSiteCount = endian::readNext<uint64_t, little, unaligned>(D);
-
-    ProfRecord.reserveSites(ValueKind, ValueSiteCount);
-
-    for (uint64_t VSite = 0; VSite < ValueSiteCount; ++VSite) {
-      // Read number of value data pairs at value site.
-      if (D + sizeof(uint64_t) > End)
-        return false;
+  if (VDataPtrOrErr.getError()) return false;
 
-      uint64_t ValueDataCount =
-          endian::readNext<uint64_t, little, unaligned>(D);
-
-      // Check if there are as many ValueDataPairs as ValueDataCount in memory.
-      if (D + (ValueDataCount << 1) * sizeof(uint64_t) > End)
-        return false;
+  VDataPtrOrErr.get()->deserializeTo(DataBuffer.back(), &HashKeys);
+  D += VDataPtrOrErr.get()->TotalSize;
 
-      std::unique_ptr<InstrProfValueData[]> VDataPtr(
-          ValueDataCount == 0 ? nullptr
-                              : new InstrProfValueData[ValueDataCount]);
-
-      for (uint64_t VCount = 0; VCount < ValueDataCount; ++VCount) {
-        VDataPtr[VCount].Value =
-            endian::readNext<uint64_t, little, unaligned>(D);
-        VDataPtr[VCount].Count =
-            endian::readNext<uint64_t, little, unaligned>(D);
-      }
-      ProfRecord.addValueData(ValueKind, VSite, VDataPtr.get(), ValueDataCount,
-                              &HashKeys);
-    }
-  }
   return true;
 }
 
 data_type InstrProfLookupTrait::ReadData(StringRef K, const unsigned char *D,
                                          offset_type N) {
   // Check if the data is corrupt. If so, don't try to read it.
   if (N % sizeof(uint64_t))
     return data_type();

lib/ProfileData/InstrProfWriter.cpp

EmitKeyDataLength(raw_ostream &Out, key_type_ref K, data_type_ref V) {
     offset_type M = 0;
     for (const auto &ProfileData : *V) {
       const InstrProfRecord &ProfRecord = ProfileData.second;
       M += sizeof(uint64_t); // The function hash
       M += sizeof(uint64_t); // The size of the Counts vector
       M += ProfRecord.Counts.size() * sizeof(uint64_t);
 
       // Value data
-      M += sizeof(uint64_t); // Number of value kinds with value sites.
-      for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) {
-        uint32_t NumValueSites = ProfRecord.getNumValueSites(Kind);
-        if (NumValueSites == 0) continue;
-        M += sizeof(uint64_t); // Value kind
-        M += sizeof(uint64_t); // The number of value sites for given value kind
-        for (uint32_t I = 0; I < NumValueSites; I++) {
-          M += sizeof(uint64_t); // Number of value data pairs at a value site
-          uint64_t NumValueDataForSite =
-              ProfRecord.getNumValueDataForSite(Kind, I);
-          M += 2 * sizeof(uint64_t) * NumValueDataForSite;  // Value data pairs
-        }
-      }
+      M += IndexedInstrProf::ValueProfData::getSize(ProfileData.second);
     }
     LE.write<offset_type>(M);
 
     return std::make_pair(N, M);
   }
 
   static void EmitKey(raw_ostream &Out, key_type_ref K, offset_type N){
     Out.write(K.data(), N);
   }
 
   static void EmitData(raw_ostream &Out, key_type_ref, data_type_ref V,
                        offset_type) {
     using namespace llvm::support;
     endian::Writer<little> LE(Out);
     for (const auto &ProfileData : *V) {
       const InstrProfRecord &ProfRecord = ProfileData.second;
 
       LE.write<uint64_t>(ProfileData.first); // Function hash
       LE.write<uint64_t>(ProfRecord.Counts.size());
       for (uint64_t I : ProfRecord.Counts) LE.write<uint64_t>(I);
 
-      // Compute the number of value kinds with value sites.
-      uint64_t NumValueKinds = ProfRecord.getNumValueKinds();
-      LE.write<uint64_t>(NumValueKinds);
-
       // Write value data
-      for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) {
-        uint32_t NumValueSites = ProfRecord.getNumValueSites(Kind);
-        if (NumValueSites == 0) continue;
-        LE.write<uint64_t>(Kind); // Write value kind
-        // Write number of value sites for current value kind
-        LE.write<uint64_t>(NumValueSites);
-
-        for (uint32_t I = 0; I < NumValueSites; I++) {
-          // Write number of value data pairs at this value site
-          uint64_t NumValueDataForSite =
-              ProfRecord.getNumValueDataForSite(Kind, I);
-          LE.write<uint64_t>(NumValueDataForSite);
-          std::unique_ptr<InstrProfValueData[]> VD =
-              ProfRecord.getValueForSite(Kind, I);
-
-          for (uint32_t V = 0; V < NumValueDataForSite; V++) {
-            if (Kind == IPVK_IndirectCallTarget)
-              LE.write<uint64_t>(ComputeHash((const char *)VD[V].Value));
-            else
-              LE.write<uint64_t>(VD[V].Value);
-            LE.write<uint64_t>(VD[V].Count);
-          }
-        }
-      }
+      std::unique_ptr<IndexedInstrProf::ValueProfData> VDataPtr =
+          IndexedInstrProf::ValueProfData::serializeFrom(ProfileData.second);
+
+      uint64_t *RawPtr = reinterpret_cast<uint64_t *>(VDataPtr.get());
+      for (uint32_t I = 0; I < VDataPtr->TotalSize / sizeof(uint64_t); I++)
+        LE.write<uint64_t>(RawPtr[I]);
       }
     }
 };
 }
 
 static std::error_code combineInstrProfRecords(InstrProfRecord &Dest,
                                                InstrProfRecord &Source,
                                                uint64_t &MaxFunctionCount) {
   // If the number of counters doesn't match we either have bad data
   // or a hash collision.