[profile] Add binary ids into indexed profiles

[gulfem]

Summary

This patch adds support for including binary ids in an indexed profile.
It adds a new field into the header that points to the offset of the
binary id section. The binary id section consists of a size of the
section, and a list of binary ids (if they are present) that consist
of two parts: length and data.

This patch guarantees that indexed profile is backwards compatible
after adding binary ids.

[mysterymath]

llvm/include/llvm/ProfileData/InstrProfReader.h

102

Can we use ArrayRef<uint8_t> in lieu of std::pair<uint64_t, const uint8_t *>? Lengths larger than size_t won't work on 32-bit platforms any, since the data is in memory.
This would allow us to use the convenience ArrayRef internally, then convert to 64-bit sizes at the (de)serialization edge.

llvm/lib/ProfileData/InstrProfReader.cpp

78

nit: This is alignToPowerOf2 in MathExtras.h.

llvm/lib/ProfileData/InstrProfWriter.cpp

379–383

nit: 'HashOffset', 'MemProfOffset', and 'BinaryIdOffset'.

500

It's a bit odd that binary ID parsing logic is inlined here into the writer.
It seems like this should be a responsibility of the reader, such that it would present and interface for iterating over each binary ID semantically.

[gulfem]

Provide readBinaryIds() function and address feedback.

[gulfem]

Fixed the issue about error reporting in llvm-profdata that caused invalid text format tests failed.

[mysterymath]

llvm/include/llvm/ProfileData/InstrProf.h

1053

nit: IDs

llvm/lib/ProfileData/InstrProfReader.cpp

114

nit: Just 8. The comment says "aligned to 8 bytes", so that's the origin of 8, rather than it being the size of a uint64_t.

llvm/lib/ProfileData/InstrProfWriter.cpp

284

ArrayRef<llvm:object::BuildIDRef>; that way, any vector-like collection can be used here. Also saves a copy.

[mysterymath]

Also, just a heads up, but there looks to be a reliable prof-data-related crash in the Windows presubmit tests.

[gulfem]

1. Assign 0 for BinaryIdsSize by default to fix the llvm-profdata

test failures happened in Windows, where the indexed profiles are
generated with an earlier version of indexed profile.

2. Addressed further feedback

[gulfem]

llvm/include/llvm/ProfileData/InstrProf.h

1053

I consistently use Id instead of an ID throughout the implementation.

llvm/lib/ProfileData/InstrProfWriter.cpp

284

I overlooked this, and thanks for pointing that out. It should at least be a reference. I changed that to ArrayRef to make it more flexible.

[mysterymath]

LGTM from my POV (general build IDisms, usability for debuginfod).

llvm/lib/ProfileData/InstrProfWriter.cpp

285–286

llvm::append_range(BinaryIds, BIs);

[gulfem]

Use llvm::append_range

[gulfem]

@davidxl, @vsk and @ellis
We are planning to implement support for the debuginfod protocol in llvm-cov, which will be used by the source-based code coverage in Fuchsia.
Supporting build ids in indexed profiles is the first step towards that goal. Could you please take a look at that?

[davidxl]

What is the use case for the feature?

[ellis]

llvm/tools/llvm-profdata/llvm-profdata.cpp

344

SmallVector<T, 0> seems to be preferred over std::vector<T> even if the number of elements is large.

https://www.llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h

[gulfem]

In D135929#3886182, @davidxl wrote:

What is the use case for the feature?

Our current coverage pipeline roughly has the following steps:

1. Read binary-id in each raw profile (.profraw) via llvm-profdata
2. Fetch all the binaries with the corresponding binary ids
3. Merge all the raw profiles, and generate an indexed profile (.profdata)
4. Generate a coverage report via llvm-cov by providing the indexed profile and all the fetched binaries.

We would like to simplify our coverage pipeline by using debuginfod in llvm-cov. When we include binary ids into indexed profiles and start using debuginfod in llvm-cov, our coverage pipeline can look like this:

1. Merge all the raw profiles, and generate an indexed profile (.profdata)
2. Generate a coverage report via llvm-cov by providing only an indexed profile. llvm-cov is going to read all the binary ids from an indexed profile, and fetch the associated binaries from the debuginfod server.

[alanphipps]

In D135929#3886921, @gulfem wrote:

We would like to simplify our coverage pipeline by using debuginfod in llvm-cov. When we include binary ids into indexed profiles and start using debuginfod in llvm-cov

Thanks for sharing! Our downstream coverage flow is very similar to yours, and so I suspect we could also benefit from this improvement.

[davidxl]

In D135929#3886921, @gulfem wrote:

In D135929#3886182, @davidxl wrote:

What is the use case for the feature?

Our current coverage pipeline has the following steps:

1. Read binary-id in each raw profile (.profraw) via using llvm-profdata
2. Fetch all the binaries with the corresponding binary id
3. Merge all the raw profiles, and generate an indexed profile (.profdata)
4. Generate a coverage report via using llvm-cov by providing all the fetched binaries and indexed profile.

We would like to simplify our coverage pipeline by using debuginfod in llvm-cov. When we include binary ids into indexed profiles and start using debuginfod in llvm-cov, our coverage pipeline can look like this:

1. Merge all the raw profiles, and generate an indexed profile (.profdata)
2. Generate a coverage report via using llvm-cov by providing only an indexed profile. llvm-cov is going to read all the binary ids from an indexed profile, and fetch the associated binaries from the debuginfod server.

thanks.

[gulfem]

In D135929#3888682, @alanphipps wrote:

In D135929#3886921, @gulfem wrote:

We would like to simplify our coverage pipeline by using debuginfod in llvm-cov. When we include binary ids into indexed profiles and start using debuginfod in llvm-cov

Thanks for sharing! Our downstream coverage flow is very similar to yours, and so I suspect we could also benefit from this improvement.

It looks like coverage users share a similar flow, so we expect that this work can be leveraged by other coverage users. We have custom tools/scripts that handle the workflow that I described above in Fuchsia.
If we can move some of the workflow to llvm tools like llvm-cov, the customization is going to be smaller. Please feel free to review the code.

[gulfem]

llvm/tools/llvm-profdata/llvm-profdata.cpp

344

The way I interpret Programmers Manual on SmallVector is that it good for the containers that have small number of elements:
"This is good for vectors that are “usually small” (e.g. the number of predecessors/successors of a block is usually less than 8)."
This container might include thousands of elements when all the profiles are merged together while collecting coverage in a big project.

[gulfem]

Does anybody have any more feedback, questions or concerns? If not, I'm planning to land this patch.

[gulfem]

https://reviews.llvm.org/D136702 is the WIP patch that adds support using debuginfod into llvm-cov, which relies on embedding binary ids into indexed profiles.

[ellis]

Seems good to me! I just have some minor nits.

llvm/lib/ProfileData/InstrProfReader.cpp

98

Can we use endian::readNext() here and remove BI += sizeof(BILen) later?

llvm/lib/ProfileData/InstrProfWriter.cpp

485–486

NIT: Can we combine these two lines into one?

[gulfem]

Fix a few things after testing on Fuchsia and rebase

1. Merge binary ids when writer contexts from different threads are merged
2. Remove duplicated binary ids
3. Use object::BuildID instead of object::BuildIDRef

[ellis]

llvm/lib/ProfileData/InstrProfWriter.cpp

298–301

I'm not sure if this early return makes sense here. If we fail to merge the "frame mappings" then does that mean we should also skip merging the binary ids? I think we should either move this to its own function or move merging the binary ids before this.

[mysterymath]

llvm/lib/ProfileData/InstrProfWriter.cpp

309

Shouldn't this be reserve(BinaryIds.size() + IPW.BinaryIDs.size())?

[gulfem]

Addressed feedback about merging binary ids.

[gulfem]

llvm/lib/ProfileData/InstrProfWriter.cpp

298–301

Thanks for pointing that out. I moved it earlier.

[gulfem]

There were two issues when I landed the change:

1. Failed on big endian powerpc64 bot

https://lab.llvm.org/buildbot#builders/231/builds/6229
The issue is that we use the host-endian format in raw profiles,
but we always use little-endian format in indexed profiles.
readBinaryIdsInternal() is shared between raw and indexed profiles,
so I extended the function to take endian argument.

2. Failed on some profile tests

Some tests use different flags while instrumenting the code, and
compare the generated profiles via using diff. After adding
binary ids into profiles, the generated profiles have different
binary ids when they are compiled with different flags.
So, I updated the tests to compare the llvm-profdata show
result instead of directly comparing the profiles.

[gulfem]

Use Endian instead of Endiannness to be consistent.

[mingmingl]

Hi, I noticed that the binary id section is padded so its section size is a power of 2. So the actual bytes written could be approx. 2x of binary id content size (e.g, 17 is padded to 32). May I know what's the motivation? If the section is not padded to a power of 2 (e.g., pad up so the section size is a multiple of 8), instr prof reader could be changed accordingly to read binary id correctly.

compiler-rt/include/profile/InstrProfData.inc

 #define INSTR_PROF_RAW_MAGIC_32 (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
 
 /* FIXME: Please remedy the fixme in the header before bumping the version. */
 /* Raw profile format version (start from 1). */
 #define INSTR_PROF_RAW_VERSION 8
 /* Indexed profile format version (start from 1). */
-#define INSTR_PROF_INDEX_VERSION 8
+#define INSTR_PROF_INDEX_VERSION 9
 /* Coverage mapping format version (start from 0). */
 #define INSTR_PROF_COVMAP_VERSION 5
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
  * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentation
  * generated profile, and 0 if this is a Clang FE generated profile.
  * 1 in bit 57 indicates there are context-sensitive records in the profile.

compiler-rt/test/profile/Linux/binary-id.c

 // REQUIRES: linux
 // RUN: %clang_profgen -Wl,--build-id=none -O2 -o %t %s
 // RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
-// RUN: llvm-profdata show --binary-ids  %t.profraw > %t.out
+// RUN: llvm-profdata show --binary-ids %t.profraw > %t.out
 // RUN: FileCheck %s --check-prefix=NO-BINARY-ID < %t.out
 // RUN: llvm-profdata merge -o %t.profdata %t.profraw
 
 // RUN: %clang_profgen -Wl,--build-id -O2 -o %t %s
 // RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
-// RUN: llvm-profdata show --binary-ids  %t.profraw > %t.profraw.out
+// RUN: llvm-profdata show --binary-ids %t.profraw > %t.profraw.out
 // RUN: FileCheck %s --check-prefix=BINARY-ID-RAW-PROF < %t.profraw.out
 
 // RUN: rm -rf %t.profdir
 // RUN: env LLVM_PROFILE_FILE=%t.profdir/default_%m.profraw %run %t
 // RUN: env LLVM_PROFILE_FILE=%t.profdir/default_%m.profraw %run %t
 // RUN: env LLVM_PROFILE_FILE=%t.profdir/default_%m.profraw %run %t
 // RUN: llvm-profdata show --binary-ids  %t.profdir/default_*.profraw > %t.profraw.out
 // RUN: FileCheck %s --check-prefix=BINARY-ID-MERGE-PROF < %t.profraw.out
 
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw %t.profraw
+// RUN: llvm-profdata show --binary-ids %t.profdata > %t.profdata.out
+// RUN: FileCheck %s --check-prefix=BINARY-ID-INDEXED-PROF < %t.profraw.out
+
 void foo() {
 }
 
 void bar() {
 }
 
 int main() {
   foo();

 // BINARY-ID-RAW-PROF-NEXT: {{[0-9a-f]+}}
 
 // BINARY-ID-MERGE-PROF: Instrumentation level: Front-end
 // BINARY-ID-MERGE-PROF-NEXT: Total functions: 3
 // BINARY-ID-MERGE-PROF-NEXT: Maximum function count: 3
 // BINARY-ID-MERGE-PROF-NEXT: Maximum internal block count: 0
 // BINARY-ID-MERGE-PROF-NEXT: Binary IDs:
 // BINARY-ID-MERGE-PROF-NEXT: {{[0-9a-f]+}}
+
+// BINARY-ID-INDEXED-PROF: Instrumentation level: Front-end
+// BINARY-ID-INDEXED-PROF-NEXT: Total functions: 3
+// BINARY-ID-INDEXED-PROF-NEXT: Maximum function count: 3
+// BINARY-ID-INDEXED-PROF-NEXT: Maximum internal block count: 0
+// BINARY-ID-INDEXED-PROF-NEXT: Binary IDs:
+// BINARY-ID-INDEXED-PROF-NEXT: {{[0-9a-f]+}}

llvm/include/llvm/ProfileData/InstrProf.h

enum ProfVersion {
   Version5 = 5,
   // In this version, the frontend PGO stable hash algorithm got fixed and
   // may produce hashes different from Version5.
   Version6 = 6,
   // An additional counter is added around logical operators.
   Version7 = 7,
   // An additional (optional) memory profile type is added.
   Version8 = 8,
-  // The current version is 8.
+  // Binary ids are added.
+  Version9 = 9,
+  // The current version is 9.
   CurrentVersion = INSTR_PROF_INDEX_VERSION
 };
 const uint64_t Version = ProfVersion::CurrentVersion;
 
 const HashT HashType = HashT::MD5;
 
 inline uint64_t ComputeHash(StringRef K) { return ComputeHash(HashType, K); }
 
 // 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 Unused; // Becomes unused since version 4
   uint64_t HashType;
   uint64_t HashOffset;
   uint64_t MemProfOffset;
+  uint64_t BinaryIdOffset;
   // New fields should only be added at the end to ensure that the size
   // computation is correct. The methods below need to be updated to ensure that
   // the new field is read correctly.
 
   // Reads a header struct from the buffer.
   static Expected<Header> readFromBuffer(const unsigned char *Buffer);
 
   // Returns the size of the header in bytes for all valid fields based on the

llvm/include/llvm/ProfileData/InstrProfData.inc

 #define INSTR_PROF_RAW_MAGIC_32 (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
 
 /* FIXME: Please remedy the fixme in the header before bumping the version. */
 /* Raw profile format version (start from 1). */
 #define INSTR_PROF_RAW_VERSION 8
 /* Indexed profile format version (start from 1). */
-#define INSTR_PROF_INDEX_VERSION 8
+#define INSTR_PROF_INDEX_VERSION 9
 /* Coverage mapping format version (start from 0). */
 #define INSTR_PROF_COVMAP_VERSION 5
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
  * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentation
  * generated profile, and 0 if this is a Clang FE generated profile.
  * 1 in bit 57 indicates there are context-sensitive records in the profile.

llvm/include/llvm/ProfileData/InstrProfReader.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_PROFILEDATA_INSTRPROFREADER_H
 #define LLVM_PROFILEDATA_INSTRPROFREADER_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/ProfileSummary.h"
+#include "llvm/Object/BuildID.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/ProfileData/InstrProfCorrelator.h"
 #include "llvm/ProfileData/MemProf.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/OnDiskHashTable.h"
 #include "llvm/Support/SwapByteOrder.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <cstdint>
 #include <iterator>
public:
   virtual ~InstrProfReader() = default;
 
   /// Read the header.  Required before reading first record.
   virtual Error readHeader() = 0;
 
   /// Read a single record.
   virtual Error readNextRecord(NamedInstrProfRecord &Record) = 0;
 
-  /// Print binary ids on stream OS.
+  /// Read a list of binary ids.
+  virtual Error
+  readBinaryIds(std::vector<llvm::object::BuildIDRef> &BinaryIds) {
+    return success();
+  }
+
+  /// Print binary ids.
   virtual Error printBinaryIds(raw_ostream &OS) { return success(); };
 
   /// Iterator over profile data.
   InstrProfIterator<> begin() { return InstrProfIterator<>(this); }
   InstrProfIterator<> end() { return InstrProfIterator<>(); }
 
   /// Return the profile version.
   virtual uint64_t getVersion() const = 0;
RawInstrProfReader(std::unique_ptr<MemoryBuffer> DataBuffer,
         Correlator(dyn_cast_or_null<const InstrProfCorrelatorImpl<IntPtrT>>(
             Correlator)) {}
   RawInstrProfReader(const RawInstrProfReader &) = delete;
   RawInstrProfReader &operator=(const RawInstrProfReader &) = delete;
 
   static bool hasFormat(const MemoryBuffer &DataBuffer);
   Error readHeader() override;
   Error readNextRecord(NamedInstrProfRecord &Record) override;
+  Error
+  readBinaryIds(std::vector<llvm::object::BuildIDRef> &BinaryIds) override;
   Error printBinaryIds(raw_ostream &OS) override;
 
   uint64_t getVersion() const override { return Version; }
 
   bool isIRLevelProfile() const override {
     return (Version & VARIANT_MASK_IR_PROF) != 0;
   }
 
private:
   /// MemProf record profile data on-disk indexed via llvm::md5(FunctionName).
   std::unique_ptr<MemProfRecordHashTable> MemProfRecordTable;
   /// MemProf frame profile data on-disk indexed via frame id.
   std::unique_ptr<MemProfFrameHashTable> MemProfFrameTable;
 
   // Index to the current record in the record array.
   unsigned RecordIndex;
 
+  uint64_t BinaryIdsSize;
+  const uint8_t *BinaryIdsStart;
+
   // Read the profile summary. Return a pointer pointing to one byte past the
   // end of the summary data if it exists or the input \c Cur.
   // \c UseCS indicates whether to use the context-sensitive profile summary.
   const unsigned char *readSummary(IndexedInstrProf::ProfVersion Version,
                                    const unsigned char *Cur, bool UseCS);
 
 public:
   IndexedInstrProfReader(
ProfileSummary &getSummary(bool UseCS) {
     if (UseCS) {
       assert(CS_Summary && "No context sensitive summary");
       return *(CS_Summary.get());
     } else {
       assert(Summary && "No profile summary");
       return *(Summary.get());
     }
   }
+
+  Error
+  readBinaryIds(std::vector<llvm::object::BuildIDRef> &BinaryIds) override;
+  Error printBinaryIds(raw_ostream &OS) override;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_PROFILEDATA_INSTRPROFREADER_H

llvm/include/llvm/ProfileData/InstrProfWriter.h

 
 #ifndef LLVM_PROFILEDATA_INSTRPROFWRITER_H
 #define LLVM_PROFILEDATA_INSTRPROFWRITER_H
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/IR/GlobalValue.h"
+#include "llvm/Object/BuildID.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/ProfileData/MemProf.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include <cstdint>
 #include <memory>
 
 namespace llvm {
private:
   // convert IndexedMemProfRecord to MemProfRecords with frame information
   // inline.
   llvm::MapVector<memprof::FrameId, memprof::Frame> MemProfFrameData;
 
   // An enum describing the attributes of the profile.
   InstrProfKind ProfileKind = InstrProfKind::Unknown;
   // Use raw pointer here for the incomplete type object.
   InstrProfRecordWriterTrait *InfoObj;
+  // List of binary ids.
+  std::vector<llvm::object::BuildIDRef> BinaryIds;
 
 public:
   InstrProfWriter(bool Sparse = false);
   ~InstrProfWriter();
 
   StringMap<ProfilingData> &getProfileData() { return FunctionData; }
 
   /// Add function counts for the given function. If there are already counts
public:
   void addMemProfRecord(const GlobalValue::GUID Id,
                         const memprof::IndexedMemProfRecord &Record);
 
   /// Add a memprof frame identified by the hash of the contents of the frame in
   /// \p FrameId.
   bool addMemProfFrame(const memprof::FrameId, const memprof::Frame &F,
                        function_ref<void(Error)> Warn);
 
+  // Add a binary ids to the binary ids list.
+  void addBinaryIds(std::vector<llvm::object::BuildIDRef> BinaryIds);
+
   /// Merge existing function counts from the given writer.
   void mergeRecordsFromWriter(InstrProfWriter &&IPW,
                               function_ref<void(Error)> Warn);
 
   /// Write the profile to \c OS
   Error write(raw_fd_ostream &OS);
 
   /// Write the profile in text format to \c OS

llvm/lib/ProfileData/InstrProf.cpp

Expected<Header> Header::readFromBuffer(const unsigned char *Buffer) {
   if (GET_VERSION(H.formatVersion()) >
       IndexedInstrProf::ProfVersion::CurrentVersion)
     return make_error<InstrProfError>(instrprof_error::unsupported_version);
 
   switch (GET_VERSION(H.formatVersion())) {
     // When a new field is added in the header add a case statement here to
     // populate it.
     static_assert(
-        IndexedInstrProf::ProfVersion::CurrentVersion == Version8,
+        IndexedInstrProf::ProfVersion::CurrentVersion == Version9,
         "Please update the reading code below if a new field has been added, "
         "if not add a case statement to fall through to the latest version.");
+  case 9ull:
+    H.BinaryIdOffset = read(Buffer, offsetOf(&Header::BinaryIdOffset));
+    [[fallthrough]];
   case 8ull:
     H.MemProfOffset = read(Buffer, offsetOf(&Header::MemProfOffset));
     [[fallthrough]];
   default: // Version7 (when the backwards compatible header was introduced).
     H.HashType = read(Buffer, offsetOf(&Header::HashType));
     H.HashOffset = read(Buffer, offsetOf(&Header::HashOffset));
   }
 
   return H;
 }
 
 size_t Header::size() const {
   switch (GET_VERSION(formatVersion())) {
     // When a new field is added to the header add a case statement here to
     // compute the size as offset of the new field + size of the new field. This
     // relies on the field being added to the end of the list.
-    static_assert(IndexedInstrProf::ProfVersion::CurrentVersion == Version8,
+    static_assert(IndexedInstrProf::ProfVersion::CurrentVersion == Version9,
                   "Please update the size computation below if a new field has "
                   "been added to the header, if not add a case statement to "
                   "fall through to the latest version.");
+  case 9ull:
+    return offsetOf(&Header::BinaryIdOffset) + sizeof(Header::BinaryIdOffset);
   case 8ull:
     return offsetOf(&Header::MemProfOffset) + sizeof(Header::MemProfOffset);
   default: // Version7 (when the backwards compatible header was introduced).
     return offsetOf(&Header::HashOffset) + sizeof(Header::HashOffset);
   }
 }
 
 } // namespace IndexedInstrProf
 
 } // end namespace llvm

llvm/lib/ProfileData/InstrProfReader.cpp

if (std::error_code EC = BufferOrErr.getError())
     return errorCodeToError(EC);
   return std::move(BufferOrErr.get());
 }
 
 static Error initializeReader(InstrProfReader &Reader) {
   return Reader.readHeader();
 }
 
+static Error
+readBinaryIdsInternal(const MemoryBuffer &DataBuffer, uint64_t BinaryIdsSize,
+                      const uint8_t *BinaryIdsStart,
+                      std::vector<llvm::object::BuildIDRef> &BinaryIds) {
+  using namespace support;
+
+  if (BinaryIdsSize == 0)
+    return Error::success();
+
+  const uint8_t *BI = BinaryIdsStart;
+  const uint8_t *BIEnd = BinaryIdsStart + BinaryIdsSize;
+  while (BI < BIEnd) {
+    size_t Remaining = BIEnd - BI;
+    // There should be enough left to read the binary id length.
+    if (Remaining < sizeof(uint64_t))
+      return make_error<InstrProfError>(
+          instrprof_error::malformed,
+          "not enough data to read binary id length");
+
+    // Read binary id length.
+    uint64_t BILen = endian::byte_swap<uint64_t, little>(
+        *reinterpret_cast<const uint64_t *>(BI));
+    // There should be enough left to read the binary id length and its data.
+    if (Remaining < sizeof(BILen) + BILen)
+      return make_error<InstrProfError>(
+          instrprof_error::malformed, "not enough data to read binary id data");
+
+    // Increment by binary id length data type size.
+    BI += sizeof(BILen);
+    if (BI > (const uint8_t *)DataBuffer.getBufferEnd())
+      return make_error<InstrProfError>(
+          instrprof_error::malformed,
+          "binary id that is read is bigger than buffer size");
+    BinaryIds.push_back(llvm::object::BuildIDRef(BI, BILen));
+
+    // Increment by binary id data length, aligned to 8 bytes.
+    BI += alignToPowerOf2(BILen, sizeof(uint64_t));
+    if (BI > (const uint8_t *)DataBuffer.getBufferEnd())
+      return make_error<InstrProfError>(instrprof_error::malformed);
+  }
+
+  return Error::success();
+}
+
+static Error printBinaryIdsInternal(raw_ostream &OS,
+                                    const MemoryBuffer &DataBuffer,
+                                    uint64_t BinaryIdsSize,
+                                    const uint8_t *BinaryIdsStart) {
+  if (BinaryIdsSize == 0)
+    return Error::success();
+
+  std::vector<llvm::object::BuildIDRef> BinaryIds;
+  if (Error E = readBinaryIdsInternal(DataBuffer, BinaryIdsSize, BinaryIdsStart,
+                                      BinaryIds))
+    return E;
+
+  OS << "Binary IDs: \n";
+  for (auto BI : BinaryIds) {
+    for (uint64_t I = 0; I < BI.size(); I++)
+      OS << format("%02x", BI[I]);
+    OS << "\n";
+  }
+
+  return Error::success();
+}
+
 Expected<std::unique_ptr<InstrProfReader>>
 InstrProfReader::create(const Twine &Path,
                         const InstrProfCorrelator *Correlator) {
   // Set up the buffer to read.
   auto BufferOrError = setupMemoryBuffer(Path);
   if (Error E = BufferOrError.takeError())
     return std::move(E);
   return InstrProfReader::create(std::move(BufferOrError.get()), Correlator);
Error RawInstrProfReader<IntPtrT>::readNextRecord(NamedInstrProfRecord &Record) {
   if (Error E = readValueProfilingData(Record))
     return error(std::move(E));
 
   // Iterate.
   advanceData();
   return success();
 }
 
-static size_t RoundUp(size_t size, size_t align) {
-  return (size + align - 1) & ~(align - 1);
+template <class IntPtrT>
+Error RawInstrProfReader<IntPtrT>::readBinaryIds(
+    std::vector<llvm::object::BuildIDRef> &BinaryIds) {
+  return readBinaryIdsInternal(*DataBuffer, BinaryIdsSize, BinaryIdsStart,
+                               BinaryIds);
 }
 
 template <class IntPtrT>
 Error RawInstrProfReader<IntPtrT>::printBinaryIds(raw_ostream &OS) {
-  if (BinaryIdsSize == 0)
-    return success();
-
-  OS << "Binary IDs: \n";
-  const uint8_t *BI = BinaryIdsStart;
-  const uint8_t *BIEnd = BinaryIdsStart + BinaryIdsSize;
-  while (BI < BIEnd) {
-    size_t Remaining = BIEnd - BI;
-
-    // There should be enough left to read the binary ID size field.
-    if (Remaining < sizeof(uint64_t))
-      return make_error<InstrProfError>(
-          instrprof_error::malformed,
-          "not enough data to read binary id length");
-
-    uint64_t BinaryIdLen = swap(*reinterpret_cast<const uint64_t *>(BI));
-
-    // There should be enough left to read the binary ID size field, and the
-    // binary ID.
-    if (Remaining < sizeof(BinaryIdLen) + BinaryIdLen)
-      return make_error<InstrProfError>(
-          instrprof_error::malformed, "not enough data to read binary id data");
-
-    // Increment by binary id length data type size.
-    BI += sizeof(BinaryIdLen);
-    if (BI > (const uint8_t *)DataBuffer->getBufferEnd())
-      return make_error<InstrProfError>(
-          instrprof_error::malformed,
-          "binary id that is read is bigger than buffer size");
-
-    for (uint64_t I = 0; I < BinaryIdLen; I++)
-      OS << format("%02x", BI[I]);
-    OS << "\n";
-
-    // Increment by binary id data length, rounded to the next 8 bytes. This
-    // accounts for the zero-padding after each build ID.
-    BI += RoundUp(BinaryIdLen, sizeof(uint64_t));
-    if (BI > (const uint8_t *)DataBuffer->getBufferEnd())
-      return make_error<InstrProfError>(instrprof_error::malformed);
-  }
-
-  return success();
+  return printBinaryIdsInternal(OS, *DataBuffer, BinaryIdsSize, BinaryIdsStart);
 }
 
 namespace llvm {
 
 template class RawInstrProfReader<uint32_t>;
 template class RawInstrProfReader<uint64_t>;
 
 } // end namespace llvm
if (!HeaderOr)
     return HeaderOr.takeError();
 
   const IndexedInstrProf::Header *Header = &HeaderOr.get();
   Cur += Header->size();
 
   Cur = readSummary((IndexedInstrProf::ProfVersion)Header->formatVersion(), Cur,
                     /* UseCS */ false);
   if (Header->formatVersion() & VARIANT_MASK_CSIR_PROF)
-    Cur = readSummary((IndexedInstrProf::ProfVersion)Header->formatVersion(), Cur,
-                      /* UseCS */ true);
-
+    Cur =
+        readSummary((IndexedInstrProf::ProfVersion)Header->formatVersion(), Cur,
   // Read the hash type and start offset.
   IndexedInstrProf::HashT HashType = static_cast<IndexedInstrProf::HashT>(
       endian::byte_swap<uint64_t, little>(Header->HashType));
   if (HashType > IndexedInstrProf::HashT::Last)
     return error(instrprof_error::unsupported_hash_type);
 
   uint64_t HashOffset = endian::byte_swap<uint64_t, little>(Header->HashOffset);
 
   // The hash table with profile counts comes next.
   auto IndexPtr = std::make_unique<InstrProfReaderIndex<OnDiskHashTableImplV3>>(
       Start + HashOffset, Cur, Start, HashType, Header->formatVersion());
 
-  // The MemProfOffset field in the header is only valid when the format version
-  // is higher than 8 (when it was introduced).
+  // The MemProfOffset field in the header is only valid when the format
+  // version is higher than 8 (when it was introduced).
   if (GET_VERSION(Header->formatVersion()) >= 8 &&
       Header->formatVersion() & VARIANT_MASK_MEMPROF) {
     uint64_t MemProfOffset =
         endian::byte_swap<uint64_t, little>(Header->MemProfOffset);
 
     const unsigned char *Ptr = Start + MemProfOffset;
     // The value returned from RecordTableGenerator.Emit.
     const uint64_t RecordTableOffset =
         support::endian::readNext<uint64_t, little, unaligned>(Ptr);
-    // The offset in the stream right before invoking FrameTableGenerator.Emit.
+    // The offset in the stream right before invoking
+    // FrameTableGenerator.Emit.
     const uint64_t FramePayloadOffset =
         support::endian::readNext<uint64_t, little, unaligned>(Ptr);
     // The value returned from FrameTableGenerator.Emit.
     const uint64_t FrameTableOffset =
         support::endian::readNext<uint64_t, little, unaligned>(Ptr);
 
     // Read the schema.
     auto SchemaOr = memprof::readMemProfSchema(Ptr);
if (GET_VERSION(Header->formatVersion()) >= 8 &&
 
     // Initialize the frame table reader with the payload and bucket offsets.
     MemProfFrameTable.reset(MemProfFrameHashTable::Create(
         /*Buckets=*/Start + FrameTableOffset,
         /*Payload=*/Start + FramePayloadOffset,
         /*Base=*/Start, memprof::FrameLookupTrait()));
   }
 
+  // BinaryIdOffset field in the header is only valid when the format version
+  // is higher than 9 (when it was introduced).
+  if (GET_VERSION(Header->formatVersion()) >= 9) {
+    uint64_t BinaryIdOffset =
+        endian::byte_swap<uint64_t, little>(Header->BinaryIdOffset);
+    const unsigned char *Ptr = Start + BinaryIdOffset;
+    // Read binary ids size.
+    BinaryIdsSize = support::endian::readNext<uint64_t, little, unaligned>(Ptr);
+    if (BinaryIdsSize % sizeof(uint64_t))
+      return error(instrprof_error::bad_header);
+    // Set the binary ids start.
+    BinaryIdsStart = Ptr;
+    if (BinaryIdsStart > (const unsigned char *)DataBuffer->getBufferEnd())
+      return make_error<InstrProfError>(instrprof_error::malformed,
+                                        "corrupted binary ids");
+  }
+
   // Load the remapping table now if requested.
   if (RemappingBuffer) {
-    Remapper = std::make_unique<
-        InstrProfReaderItaniumRemapper<OnDiskHashTableImplV3>>(
+    Remapper =
+        std::make_unique<InstrProfReaderItaniumRemapper<OnDiskHashTableImplV3>>(
             std::move(RemappingBuffer), *IndexPtr);
     if (Error E = Remapper->populateRemappings())
       return E;
   } else {
     Remapper = std::make_unique<InstrProfReaderNullRemapper>(*IndexPtr);
   }
   Index = std::move(IndexPtr);
 
   return success();
Error IndexedInstrProfReader::readNextRecord(NamedInstrProfRecord &Record) {
   Record = Data[RecordIndex++];
   if (RecordIndex >= Data.size()) {
     Index->advanceToNextKey();
     RecordIndex = 0;
   }
   return success();
 }
 
+Error IndexedInstrProfReader::readBinaryIds(
+    std::vector<llvm::object::BuildIDRef> &BinaryIds) {
+  return readBinaryIdsInternal(*DataBuffer, BinaryIdsSize, BinaryIdsStart,
+                               BinaryIds);
+}
+
+Error IndexedInstrProfReader::printBinaryIds(raw_ostream &OS) {
+  return printBinaryIdsInternal(OS, *DataBuffer, BinaryIdsSize, BinaryIdsStart);
+}
+
 void InstrProfReader::accumulateCounts(CountSumOrPercent &Sum, bool IsCS) {
   uint64_t NumFuncs = 0;
   for (const auto &Func : *this) {
     if (isIRLevelProfile()) {
       bool FuncIsCS = NamedInstrProfRecord::hasCSFlagInHash(Func.Hash);
       if (FuncIsCS != IsCS)
         continue;
     }
     Func.accumulateCounts(Sum);
     ++NumFuncs;
   }
   Sum.NumEntries = NumFuncs;
 }

llvm/lib/ProfileData/InstrProfWriter.cpp

 public:
   ProfOStream(raw_fd_ostream &FD)
       : IsFDOStream(true), OS(FD), LE(FD, support::little) {}
   ProfOStream(raw_string_ostream &STR)
       : IsFDOStream(false), OS(STR), LE(STR, support::little) {}
 
   uint64_t tell() { return OS.tell(); }
   void write(uint64_t V) { LE.write<uint64_t>(V); }
+  void writeByte(uint8_t V) { LE.write<uint8_t>(V); }
 
   // \c patch can only be called when all data is written and flushed.
   // For raw_string_ostream, the patch is done on the target string
   // directly and it won't be reflected in the stream's internal buffer.
   void patch(PatchItem *P, int NItems) {
     using namespace support;
 
     if (IsFDOStream) {
bool InstrProfWriter::addMemProfFrame(const memprof::FrameId Id,
   if (!Result.second && Result.first->second != Frame) {
     Warn(make_error<InstrProfError>(instrprof_error::malformed,
                                     "frame to id mapping mismatch"));
     return false;
   }
   return true;
 }
 
+void InstrProfWriter::addBinaryIds(std::vector<llvm::object::BuildIDRef> BIs) {
+  for (auto BI : BIs) {
+    BinaryIds.push_back(BI);
+  }
+}
+
 void InstrProfWriter::mergeRecordsFromWriter(InstrProfWriter &&IPW,
                                              function_ref<void(Error)> Warn) {
   for (auto &I : IPW.FunctionData)
     for (auto &Func : I.getValue())
       addRecord(I.getKey(), Func.first, std::move(Func.second), 1, Warn);
 
   MemProfFrameData.reserve(IPW.MemProfFrameData.size());
   for (auto &I : IPW.MemProfFrameData) {
     // If we weren't able to add the frame mappings then it doesn't make sense
     // to try to merge the records from this profile.
     if (!addMemProfFrame(I.first, I.second, Warn))
       return;
   }
 
   MemProfRecordData.reserve(IPW.MemProfRecordData.size());
   for (auto &I : IPW.MemProfRecordData) {
     addMemProfRecord(I.first, I.second);
   }
 }
 
 bool InstrProfWriter::shouldEncodeData(const ProfilingData &PD) {
   if (!Sparse)
     return true;
   for (const auto &Func : PD) {
     const InstrProfRecord &IPR = Func.second;
     if (llvm::any_of(IPR.Counts, [](uint64_t Count) { return Count > 0; }))
       return true;
   }
static void setSummary(IndexedInstrProf::Summary *TheSummary,
   TheSummary->set(Summary::TotalNumBlocks, PS.getNumCounts());
   TheSummary->set(Summary::TotalNumFunctions, PS.getNumFunctions());
   for (unsigned I = 0; I < Res.size(); I++)
     TheSummary->setEntry(I, Res[I]);
 }
 
 Error InstrProfWriter::writeImpl(ProfOStream &OS) {
   using namespace IndexedInstrProf;
+  using namespace support;
 
   OnDiskChainedHashTableGenerator<InstrProfRecordWriterTrait> Generator;
 
   InstrProfSummaryBuilder ISB(ProfileSummaryBuilder::DefaultCutoffs);
   InfoObj->SummaryBuilder = &ISB;
   InstrProfSummaryBuilder CSISB(ProfileSummaryBuilder::DefaultCutoffs);
   InfoObj->CSSummaryBuilder = &CSISB;
 
if (static_cast<bool>(ProfileKind & InstrProfKind::FunctionEntryOnly))
     Header.Version |= VARIANT_MASK_FUNCTION_ENTRY_ONLY;
   if (static_cast<bool>(ProfileKind & InstrProfKind::MemProf))
     Header.Version |= VARIANT_MASK_MEMPROF;
 
   Header.Unused = 0;
   Header.HashType = static_cast<uint64_t>(IndexedInstrProf::HashType);
   Header.HashOffset = 0;
   Header.MemProfOffset = 0;
+  Header.BinaryIdOffset = 0;
   int N = sizeof(IndexedInstrProf::Header) / sizeof(uint64_t);
 
-  // Only write out all the fields except 'HashOffset' and 'MemProfOffset'. We
-  // need to remember the offset of these fields to allow back patching later.
-  for (int I = 0; I < N - 2; I++)
+  // Only write out all the fields except 'HashOffset', 'MemProfOffset' and
+  // 'BinaryIdOffset'. We need to remember the offset of these fields to allow
+  // back patching later.
+  for (int I = 0; I < N - 3; I++)
     OS.write(reinterpret_cast<uint64_t *>(&Header)[I]);
 
   // Save the location of Header.HashOffset field in \c OS.
   uint64_t HashTableStartFieldOffset = OS.tell();
   // Reserve the space for HashOffset field.
   OS.write(0);
 
   // Save the location of MemProf profile data. This is stored in two parts as
   // the schema and as a separate on-disk chained hashtable.
   uint64_t MemProfSectionOffset = OS.tell();
   // Reserve space for the MemProf table field to be patched later if this
   // profile contains memory profile information.
   OS.write(0);
 
+  // Save the location of binary ids section.
+  uint64_t BinaryIdSectionOffset = OS.tell();
+  // Reserve space for the BinaryIdOffset field to be patched later if this
+  // profile contains binary ids.
+  OS.write(0);
+
   // Reserve space to write profile summary data.
   uint32_t NumEntries = ProfileSummaryBuilder::DefaultCutoffs.size();
   uint32_t SummarySize = Summary::getSize(Summary::NumKinds, NumEntries);
   // Remember the summary offset.
   uint64_t SummaryOffset = OS.tell();
   for (unsigned I = 0; I < SummarySize / sizeof(uint64_t); I++)
     OS.write(0);
   uint64_t CSSummaryOffset = 0;
if (static_cast<bool>(ProfileKind & InstrProfKind::MemProf)) {
     PatchItem PatchItems[] = {
         {MemProfSectionStart, &RecordTableOffset, 1},
         {MemProfSectionStart + sizeof(uint64_t), &FramePayloadOffset, 1},
         {MemProfSectionStart + 2 * sizeof(uint64_t), &FrameTableOffset, 1},
     };
     OS.patch(PatchItems, 3);
   }
 
+  // BinaryIdSection has two parts:
+  // 1. uint64_t BinaryIdsSectionSize
+  // 2. list of binary ids that consist of:
+  //    a. uint64_t BinaryIdLength
+  //    b. uint8_t  BinaryIdData
+  //    c. uint8_t  Padding (if necessary)
+  uint64_t BinaryIdSectionStart = 0;
+  BinaryIdSectionStart = OS.tell();
+  // Calculate size of binary section.
+  uint64_t BinaryIdsSectionSize = 0;
+  for (auto BI : BinaryIds) {
+    // Increment by binary id length data type size.
+    BinaryIdsSectionSize += sizeof(uint64_t);
+    // Increment by binary id data length, aligned to 8 bytes.
+    BinaryIdsSectionSize += alignToPowerOf2(BI.size(), sizeof(uint64_t));
+  }
+  // Write binary ids section size.
+  OS.write(BinaryIdsSectionSize);
+
+  for (auto BI : BinaryIds) {
+    uint64_t BILen = BI.size();
+    // Write binary id length.
+    OS.write(BILen);
+    // Write binary id data.
+    for (unsigned K = 0; K < BILen; K++)
+      OS.writeByte(BI[K]);
+    // Write padding if necessary.
+    uint64_t PaddingSize = alignToPowerOf2(BILen, sizeof(uint64_t)) - BILen;
+    for (unsigned K = 0; K < PaddingSize; K++)
+      OS.writeByte(0);
+  }
+
   // Allocate space for data to be serialized out.
   std::unique_ptr<IndexedInstrProf::Summary> TheSummary =
       IndexedInstrProf::allocSummary(SummarySize);
   // Compute the Summary and copy the data to the data
   // structure to be serialized out (to disk or buffer).
   std::unique_ptr<ProfileSummary> PS = ISB.getSummary();
   setSummary(TheSummary.get(), *PS);
   InfoObj->SummaryBuilder = nullptr;
 
   // For Context Sensitive summary.
   std::unique_ptr<IndexedInstrProf::Summary> TheCSSummary = nullptr;
   if (static_cast<bool>(ProfileKind & InstrProfKind::ContextSensitive)) {
     TheCSSummary = IndexedInstrProf::allocSummary(SummarySize);
     std::unique_ptr<ProfileSummary> CSPS = CSISB.getSummary();
     setSummary(TheCSSummary.get(), *CSPS);
   }
   InfoObj->CSSummaryBuilder = nullptr;
 
   // Now do the final patch:
   PatchItem PatchItems[] = {
       // Patch the Header.HashOffset field.
       {HashTableStartFieldOffset, &HashTableStart, 1},
-      // Patch the Header.MemProfOffset (=0 for profiles without MemProf data).
+      // Patch the Header.MemProfOffset (=0 for profiles without MemProf
+      // data).
       {MemProfSectionOffset, &MemProfSectionStart, 1},
+      // Patch the Header.BinaryIdSectionOffset.
+      {BinaryIdSectionOffset, &BinaryIdSectionStart, 1},
       // Patch the summary data.
       {SummaryOffset, reinterpret_cast<uint64_t *>(TheSummary.get()),
        (int)(SummarySize / sizeof(uint64_t))},
       {CSSummaryOffset, reinterpret_cast<uint64_t *>(TheCSSummary.get()),
        (int)CSSummarySize}};
 
   OS.patch(PatchItems, sizeof(PatchItems) / sizeof(*PatchItems));
 

llvm/tools/llvm-profdata/llvm-profdata.cpp

WC->Writer.addRecord(std::move(I), Input.Weight, [&](Error E) {
       // Only show hint the first time an error occurs.
       instrprof_error IPE = InstrProfError::take(std::move(E));
       std::unique_lock<std::mutex> ErrGuard{WC->ErrLock};
       bool firstTime = WC->WriterErrorCodes.insert(IPE).second;
       handleMergeWriterError(make_error<InstrProfError>(IPE), Input.Filename,
                              FuncName, firstTime);
     });
   }
-  if (Reader->hasError())
+
+  if (Reader->hasError()) {
     if (Error E = Reader->getError())
       WC->Errors.emplace_back(std::move(E), Filename);
   }
 
+  std::vector<llvm::object::BuildIDRef> BinaryIds;
+  if (Error E = Reader->readBinaryIds(BinaryIds))
+    WC->Errors.emplace_back(std::move(E), Filename);
+  WC->Writer.addBinaryIds(BinaryIds);
+}
+
 /// Merge the \p Src writer context into \p Dst.
 static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
   for (auto &ErrorPair : Src->Errors)
     Dst->Errors.push_back(std::move(ErrorPair));
   Src->Errors.clear();
 
   Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) {
     instrprof_error IPE = InstrProfError::take(std::move(E));