[profile] Fix profile merging with binary IDs
authorPetr Hosek <phosek@google.com>
Fri, 30 Jul 2021 09:40:27 +0000 (02:40 -0700)
committerPetr Hosek <phosek@google.com>
Sat, 31 Jul 2021 00:38:53 +0000 (17:38 -0700)
This fixes support for merging profiles which broke as a consequence
of e50a38840dc3db5813f74b1cd2e10e6d984d0e67. The issue was missing
adjustment in merge logic to account for the binary IDs which are
now included in the raw profile just after header.

In addition, this change also:
* Includes the version in module signature that's used for merging
to avoid accidental attempts to merge incompatible profiles.
* Moves the binary IDs size field after version field in the header
as was suggested in the review.

Differential Revision: https://reviews.llvm.org/D107143

14 files changed:
compiler-rt/include/profile/InstrProfData.inc
compiler-rt/lib/profile/InstrProfilingBuffer.c
compiler-rt/lib/profile/InstrProfilingMerge.c
compiler-rt/test/profile/Linux/binary-id.c
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/InstrProfData.inc
llvm/lib/ProfileData/InstrProfReader.cpp
llvm/test/tools/llvm-profdata/Inputs/c-general.profraw
llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
llvm/test/tools/llvm-profdata/raw-32-bits-be.test
llvm/test/tools/llvm-profdata/raw-32-bits-le.test
llvm/test/tools/llvm-profdata/raw-64-bits-be.test
llvm/test/tools/llvm-profdata/raw-64-bits-le.test
llvm/test/tools/llvm-profdata/raw-two-profiles.test

index 2870c9f..cb2d6a6 100644 (file)
@@ -127,6 +127,7 @@ INSTR_PROF_VALUE_NODE(PtrToNodeT, llvm::Type::getInt8PtrTy(Ctx), Next, \
 #endif
 INSTR_PROF_RAW_HEADER(uint64_t, Magic, __llvm_profile_get_magic())
 INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version())
+INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
 INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize)
 INSTR_PROF_RAW_HEADER(uint64_t, PaddingBytesBeforeCounters, PaddingBytesBeforeCounters)
 INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize)
@@ -136,7 +137,6 @@ INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta,
                       (uintptr_t)CountersBegin - (uintptr_t)DataBegin)
 INSTR_PROF_RAW_HEADER(uint64_t, NamesDelta, (uintptr_t)NamesBegin)
 INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last)
-INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
 #undef INSTR_PROF_RAW_HEADER
 /* INSTR_PROF_RAW_HEADER  end */
 
@@ -645,7 +645,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
         (uint64_t)'f' << 16 | (uint64_t)'R' << 8 | (uint64_t)129
 
 /* Raw profile format version (start from 1). */
-#define INSTR_PROF_RAW_VERSION 6
+#define INSTR_PROF_RAW_VERSION 7
 /* Indexed profile format version (start from 1). */
 #define INSTR_PROF_INDEX_VERSION 7
 /* Coverage mapping format version (start from 0). */
index 21fa7ba..68b4f5c 100644 (file)
@@ -116,7 +116,7 @@ uint64_t __llvm_profile_get_size_for_buffer_internal(
       DataSize, CountersSize, NamesSize, &PaddingBytesBeforeCounters,
       &PaddingBytesAfterCounters, &PaddingBytesAfterNames);
 
-  return sizeof(__llvm_profile_header) +
+  return sizeof(__llvm_profile_header) + __llvm_write_binary_ids(NULL) +
          (DataSize * sizeof(__llvm_profile_data)) + PaddingBytesBeforeCounters +
          (CountersSize * sizeof(uint64_t)) + PaddingBytesAfterCounters +
          NamesSize + PaddingBytesAfterNames;
index d54689e..674b189 100644 (file)
@@ -22,6 +22,7 @@ void (*VPMergeHook)(ValueProfData *, __llvm_profile_data *);
 COMPILER_RT_VISIBILITY
 uint64_t lprofGetLoadModuleSignature() {
   /* A very fast way to compute a module signature.  */
+  uint64_t Version = __llvm_profile_get_version();
   uint64_t CounterSize = (uint64_t)(__llvm_profile_end_counters() -
                                     __llvm_profile_begin_counters());
   uint64_t DataSize = __llvm_profile_get_data_size(__llvm_profile_begin_data(),
@@ -33,7 +34,7 @@ uint64_t lprofGetLoadModuleSignature() {
   const __llvm_profile_data *FirstD = __llvm_profile_begin_data();
 
   return (NamesSize << 40) + (CounterSize << 30) + (DataSize << 20) +
-         (NumVnodes << 10) + (DataSize > 0 ? FirstD->NameRef : 0);
+         (NumVnodes << 10) + (DataSize > 0 ? FirstD->NameRef : 0) + Version;
 }
 
 /* Returns 1 if profile is not structurally compatible.  */
@@ -44,7 +45,8 @@ int __llvm_profile_check_compatibility(const char *ProfileData,
   __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
   __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
   SrcDataStart =
-      (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header));
+      (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header) +
+                              Header->BinaryIdsSize);
   SrcDataEnd = SrcDataStart + Header->DataSize;
 
   if (ProfileSize < sizeof(__llvm_profile_header))
@@ -63,7 +65,7 @@ int __llvm_profile_check_compatibility(const char *ProfileData,
       Header->ValueKindLast != IPVK_Last)
     return 1;
 
-  if (ProfileSize < sizeof(__llvm_profile_header) +
+  if (ProfileSize < sizeof(__llvm_profile_header) + Header->BinaryIdsSize +
                         Header->DataSize * sizeof(__llvm_profile_data) +
                         Header->NamesSize + Header->CountersSize)
     return 1;
@@ -100,7 +102,8 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
   uintptr_t CountersDelta = Header->CountersDelta;
 
   SrcDataStart =
-      (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header));
+      (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header) +
+                              Header->BinaryIdsSize);
   SrcDataEnd = SrcDataStart + Header->DataSize;
   SrcCountersStart = (uint64_t *)SrcDataEnd;
   SrcNameStart = (const char *)(SrcCountersStart + Header->CountersSize);
index e165283..04f54b5 100644 (file)
 // 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
+
 void foo() {
 }
 
@@ -34,3 +41,10 @@ int main() {
 // BINARY-ID-RAW-PROF-NEXT: Maximum internal block count: 0
 // BINARY-ID-RAW-PROF-NEXT: Binary IDs:
 // 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]+}}
index 08a934e..c0cedb2 100644 (file)
@@ -1104,6 +1104,7 @@ namespace RawInstrProf {
 // Version 5: Bit 60 of FuncHash is reserved for the flag for the context
 // sensitive records.
 // Version 6: Added binary id.
+// Version 7: Reorder binary id and include version in signature.
 const uint64_t Version = INSTR_PROF_RAW_VERSION;
 
 template <class IntPtrT> inline uint64_t getMagic();
index 2870c9f..cb2d6a6 100644 (file)
@@ -127,6 +127,7 @@ INSTR_PROF_VALUE_NODE(PtrToNodeT, llvm::Type::getInt8PtrTy(Ctx), Next, \
 #endif
 INSTR_PROF_RAW_HEADER(uint64_t, Magic, __llvm_profile_get_magic())
 INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version())
+INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
 INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize)
 INSTR_PROF_RAW_HEADER(uint64_t, PaddingBytesBeforeCounters, PaddingBytesBeforeCounters)
 INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize)
@@ -136,7 +137,6 @@ INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta,
                       (uintptr_t)CountersBegin - (uintptr_t)DataBegin)
 INSTR_PROF_RAW_HEADER(uint64_t, NamesDelta, (uintptr_t)NamesBegin)
 INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last)
-INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
 #undef INSTR_PROF_RAW_HEADER
 /* INSTR_PROF_RAW_HEADER  end */
 
@@ -645,7 +645,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
         (uint64_t)'f' << 16 | (uint64_t)'R' << 8 | (uint64_t)129
 
 /* Raw profile format version (start from 1). */
-#define INSTR_PROF_RAW_VERSION 6
+#define INSTR_PROF_RAW_VERSION 7
 /* Indexed profile format version (start from 1). */
 #define INSTR_PROF_INDEX_VERSION 7
 /* Coverage mapping format version (start from 0). */
index f619ad0..d7b8844 100644 (file)
@@ -366,6 +366,7 @@ Error RawInstrProfReader<IntPtrT>::readHeader(
   if (GET_VERSION(Version) != RawInstrProf::Version)
     return error(instrprof_error::unsupported_version);
 
+  BinaryIdsSize = swap(Header.BinaryIdsSize);
   CountersDelta = swap(Header.CountersDelta);
   NamesDelta = swap(Header.NamesDelta);
   auto DataSize = swap(Header.DataSize);
@@ -374,7 +375,6 @@ Error RawInstrProfReader<IntPtrT>::readHeader(
   auto PaddingBytesAfterCounters = swap(Header.PaddingBytesAfterCounters);
   NamesSize = swap(Header.NamesSize);
   ValueKindLast = swap(Header.ValueKindLast);
-  BinaryIdsSize = swap(Header.BinaryIdsSize);
 
   auto DataSizeInBytes = DataSize * sizeof(RawInstrProf::ProfileData<IntPtrT>);
   auto PaddingSize = getNumPaddingBytes(NamesSize);
index 63b00c9..b5a4799 100644 (file)
Binary files a/llvm/test/tools/llvm-profdata/Inputs/c-general.profraw and b/llvm/test/tools/llvm-profdata/Inputs/c-general.profraw differ
index c601882..b734544 100644 (file)
@@ -2,16 +2,17 @@
 //
 // INSTR_PROF_RAW_HEADER(uint64_t, Magic, __llvm_profile_get_magic())
 // INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version())
+// INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
 // INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize)
 // INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize)
 // INSTR_PROF_RAW_HEADER(uint64_t, NamesSize,  NamesSize)
 // INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta, (uintptr_t)CountersBegin)
 // INSTR_PROF_RAW_HEADER(uint64_t, NamesDelta, (uintptr_t)NamesBegin)
 // INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last)
-// INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
 
 RUN: printf '\201rforpl\377' > %t.profraw
-RUN: printf '\6\0\0\0\0\0\0\0' >> %t.profraw
+RUN: printf '\7\0\0\0\0\0\0\0' >> %t.profraw
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t.profraw
@@ -20,7 +21,6 @@ RUN: printf '\10\0\0\0\0\0\0\0' >> %t.profraw
 RUN: printf '\0\0\6\0\1\0\0\0' >> %t.profraw
 RUN: printf '\0\0\6\0\2\0\0\0' >> %t.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
-RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
 
 // Data Section
 //
index 4afb6b4..308873a 100644 (file)
@@ -1,5 +1,6 @@
 RUN: printf '\377lprofR\201' > %t
-RUN: printf '\0\0\0\0\0\0\0\6' >> %t
+RUN: printf '\0\0\0\0\0\0\0\7' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\3' >> %t
@@ -8,7 +9,6 @@ RUN: printf '\0\0\0\0\0\0\0\20' >> %t
 RUN: printf '\0\0\0\0\1\0\0\0' >> %t
 RUN: printf '\0\0\0\0\2\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
-RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 
 RUN: printf '\134\370\302\114\333\030\275\254' >> %t
 RUN: printf '\0\0\0\0\0\0\0\1' >> %t
index f82287a..cae7282 100644 (file)
@@ -1,5 +1,6 @@
 RUN: printf '\201Rforpl\377' > %t
-RUN: printf '\6\0\0\0\0\0\0\0' >> %t
+RUN: printf '\7\0\0\0\0\0\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
@@ -8,7 +9,6 @@ RUN: printf '\20\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\1\0\0\0\0' >> %t
 RUN: printf '\0\0\0\2\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
-RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 
 RUN: printf '\254\275\030\333\114\302\370\134' >> %t
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t
index aed2678..a39e9f4 100644 (file)
@@ -1,5 +1,6 @@
 RUN: printf '\377lprofr\201' > %t
-RUN: printf '\0\0\0\0\0\0\0\6' >> %t
+RUN: printf '\0\0\0\0\0\0\0\7' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\3' >> %t
@@ -8,7 +9,6 @@ RUN: printf '\0\0\0\0\0\0\0\20' >> %t
 RUN: printf '\0\0\0\1\0\4\0\0' >> %t
 RUN: printf '\0\0\0\2\0\4\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
-RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 
 RUN: printf '\134\370\302\114\333\030\275\254' >> %t
 RUN: printf '\0\0\0\0\0\0\0\1' >> %t
index d5b6e8f..e0bdf22 100644 (file)
@@ -1,5 +1,6 @@
 RUN: printf '\201rforpl\377' > %t
-RUN: printf '\6\0\0\0\0\0\0\0' >> %t
+RUN: printf '\7\0\0\0\0\0\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
@@ -8,7 +9,6 @@ RUN: printf '\20\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\4\0\1\0\0\0' >> %t
 RUN: printf '\0\0\4\0\2\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
-RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 
 RUN: printf '\254\275\030\333\114\302\370\134' >> %t
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t
index a4f97a5..669c468 100644 (file)
@@ -1,5 +1,6 @@
 RUN: printf '\201rforpl\377' > %t-foo.profraw
-RUN: printf '\6\0\0\0\0\0\0\0' >> %t-foo.profraw
+RUN: printf '\7\0\0\0\0\0\0\0' >> %t-foo.profraw
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
@@ -8,7 +9,6 @@ RUN: printf '\10\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\4\0\1\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\4\0\2\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
-RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
 
 RUN: printf '\254\275\030\333\114\302\370\134' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
@@ -21,7 +21,8 @@ RUN: printf '\023\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\3\0foo\0\0\0' >> %t-foo.profraw
 
 RUN: printf '\201rforpl\377' > %t-bar.profraw
-RUN: printf '\6\0\0\0\0\0\0\0' >> %t-bar.profraw
+RUN: printf '\7\0\0\0\0\0\0\0' >> %t-bar.profraw
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t-bar.profraw
@@ -30,7 +31,6 @@ RUN: printf '\10\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\6\0\1\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\6\0\2\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
-RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
 
 RUN: printf '\067\265\035\031\112\165\023\344' >> %t-bar.profraw
 RUN: printf '\02\0\0\0\0\0\0\0' >> %t-bar.profraw