[Profile] Allow online merging with debug info correlation.

[zequanwu]

Summary

When using debug info correlation, value profiling needs to be switched off.
So, we are only merging counter sections. In that case the existance of data
section is just used to provide an extra check in case of corrupted profile.

This patch performs counter merging by iterating the counter section by counter
size and add them together.

[zequanwu]

Remove debug includes.

[ellis]

Thanks for working on this!

compiler-rt/lib/profile/InstrProfilingMerge.c

105–107

I just realized that I need to throw an error here for temporal profiles since online merging won't work for that case.

131

Since we don't have the data section, we need to be confident that existing profile comes from the same binary (so that the counter section is identical). Can we add some extra checks here? I'm thinking we can verify that some fields in the headers match and that the variant flags are identical.

136–138

Can you add a check to make sure src and dst have the same number of counters (SrcCountersEnd - SrcCountersStart)?

compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c

26

We need to run this line twice to correctly test __llvm_profile_merge_from_buffer()

compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c

35

I know this is outside the scope of this patch, but I don't think simply llvm-profdata show is sufficient to compare profiles. I'll try to fix this.

[ellis]

I've just published https://reviews.llvm.org/D157664, so you'll want to rebase ontop of it if it lands soon. I would also like to see some more tests added to instrprof-merge-error.c to make sure two different binaries can't merge profiles together with --debug-info-correlate. I was thinking the test would be something like this.

// RUN: %clang_pgogen -o %t/a -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %t/main.c
// RUN: %clang_pgogen -o %t/b -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %t/main.c %t/foo.c
// RUN: env LLVM_PROFILE_FILE=%t/default_%m.profdata %run %t/a
// This next line should fail to merge because the counter sections have different sizes
// RUN: env LLVM_PROFILE_FILE=%t/default_%m.profdata %run %t/b

//--- main.c
int main() { return 0; }
//--- foo.c
int foo() { return 4; }

[zequanwu]

Address comments.

compiler-rt/lib/profile/InstrProfilingMerge.c

131

The comment says the caller of this function is responsible for the check. And we verifies that fields in both headers math before merging at doProfileMerging.

136–138

Added the check at __llvm_profile_check_compatibility.

I use in-memory __llvm_profile_counter_entry_size() to calculate in-file SrcCountersEnd, because the header only tells number of counters.

compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c

26

Added.

[zequanwu]

In D157632#4578582, @ellis wrote:

I've just published https://reviews.llvm.org/D157664, so you'll want to rebase ontop of it if it lands soon. I would also like to see some more tests added to instrprof-merge-error.c to make sure two different binaries can't merge profiles together with --debug-info-correlate. I was thinking the test would be something like this.

// RUN: %clang_pgogen -o %t/a -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %t/main.c
// RUN: %clang_pgogen -o %t/b -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %t/main.c %t/foo.c
// RUN: env LLVM_PROFILE_FILE=%t/default_%m.profdata %run %t/a
// This next line should fail to merge because the counter sections have different sizes
// RUN: env LLVM_PROFILE_FILE=%t/default_%m.profdata %run %t/b

//--- main.c
int main() { return 0; }
//--- foo.c
int foo() { return 4; }

In this example, %m will translated to two different values so they won't be merged. As I mentioned in the inline comment, __llvm_profile_check_compatibility will verify that for us.

[zequanwu]

BTW, I noticed something strange with -pgo-function-entry-coverage when merging via llvm-profdata.
In this file compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c, I ran the following:

// RUN: %clang_pgogen -o %t.cov.normal -mllvm --pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
// RUN: rm -rf %t.dir && mkdir %t.dir
// RUN: env LLVM_PROFILE_FILE=%t.dir/%m.profraw %run %t.cov.normal
// RUN: env LLVM_PROFILE_FILE=%t.dir/%m.profraw %run %t.cov.normal
// RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.dir/
// RUN: llvm-profdata show --all-functions --counts %t.cov.normal.profdata
// It shows block counts 1.
Counters:
  main:
    Hash: 0x06d15c67b2c35b9c
    Counters: 1
    Block counts: [1]
  _Z3fooi:
    Hash: 0x0209aa3e3852da94
    Counters: 1
    Block counts: [1]
  _Z3bari:
    Hash: 0x0209aa3e1d398548
    Counters: 1
    Block counts: [1]
  _Z6unusedi:
    Hash: 0x0a4d0ad3efffffff
    Counters: 1
    Block counts: [0]
Instrumentation level: IR  entry_first = 0
Functions shown: 4
Total functions: 4
Maximum function count: 1
Maximum internal block count: 0

// RUN: rm -rf %t.dir && mkdir %t.dir
// RUN: env LLVM_PROFILE_FILE=%t.dir/%t.cov-1.profraw %run %t.cov.normal
// RUN: env LLVM_PROFILE_FILE=%t.dir/%t.cov-2.profraw %run %t.cov.normal
// RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.dir/
// RUN: llvm-profdata show --all-functions --counts %t.cov.normal.profdata
// It shows block counts 2, as opposed to 1.
Counters:
  main:
    Hash: 0x06d15c67b2c35b9c
    Counters: 1
    Block counts: [2]
  _Z3fooi:
    Hash: 0x0209aa3e3852da94
    Counters: 1
    Block counts: [2]
  _Z3bari:
    Hash: 0x0209aa3e1d398548
    Counters: 1
    Block counts: [2]
  _Z6unusedi:
    Hash: 0x0a4d0ad3efffffff
    Counters: 1
    Block counts: [0]
Instrumentation level: IR  entry_first = 0
Functions shown: 4
Total functions: 4
Maximum function count: 2
Maximum internal block count: 0

[ellis]

In D157632#4580576, @zequanwu wrote:

BTW, I noticed something strange with -pgo-function-entry-coverage when merging via llvm-profdata.

This is intentional. The two raw profiles individually store blocks as either covered or not, but when we merge them they get converted to counters and accumulated.
https://github.com/llvm/llvm-project/blob/6a0feb1503e21432e63d93b44357bad43f8026d1/llvm/lib/ProfileData/InstrProf.cpp#L726
Then in PGOUseFunc::populateCoverage() we annotate blocks as alive if their counter value is non-zero.
https://github.com/llvm/llvm-project/blob/1aee2434ce4c9b5785cbc8f72cbbbd64f9e85297/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1399
My logic was that the indexed profile represents these counters as ints, so we might as well accumulate them. Also, this makes the implementation simpler.

[zequanwu]

In D157632#4581219, @ellis wrote:

In D157632#4580576, @zequanwu wrote:

BTW, I noticed something strange with -pgo-function-entry-coverage when merging via llvm-profdata.

This is intentional. The two raw profiles individually store blocks as either covered or not, but when we merge them they get converted to counters and accumulated.
https://github.com/llvm/llvm-project/blob/6a0feb1503e21432e63d93b44357bad43f8026d1/llvm/lib/ProfileData/InstrProf.cpp#L726
Then in PGOUseFunc::populateCoverage() we annotate blocks as alive if their counter value is non-zero.
https://github.com/llvm/llvm-project/blob/1aee2434ce4c9b5785cbc8f72cbbbd64f9e85297/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1399
My logic was that the indexed profile represents these counters as ints, so we might as well accumulate them. Also, this makes the implementation simpler.

Thanks for explanation. That makes sense.

[ellis]

compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c

24

Let's move this line down since we don't use it until line 31.

30

This looks to be the same as line 2. Lets just reuse that executable.

39

Same here. This is the same as line 12. (And I believe these comments also apply to the Linux test)

[zequanwu]

Rebase and address comments.

[aeubanks]

compiler-rt/lib/profile/InstrProfilingMerge.c

133

we're running into this error message even though we don't have debug info correlation turned on, is that expected in some configurations? I'm still trying to understand how we could end up in this code path without debug info correlation

https://crbug.com/1473935

[aeubanks]

compiler-rt/lib/profile/InstrProfilingMerge.c

133

also it seems like this code path isn't tested?

[ellis]

compiler-rt/lib/profile/InstrProfilingMerge.c

133

It's very strange that you are hitting this code path because I didn't think it was possible. Were you ever able to reproduce locally?

[aeubanks]

compiler-rt/lib/profile/InstrProfilingMerge.c

133

no, it's only showed up on some bots (both mac and linux), I'm still trying to understand what's going on

[aeubanks]

compiler-rt/lib/profile/InstrProfilingMerge.c

133

managed to repro having no DataSize (NumData now).

$ cat /tmp/a.cc
int main() {}
$ cat /tmp/funlist
[clang]
default:skip
$ clang++ -O1 -fprofile-instr-generate -fcoverage-mapping -o /tmp/a -fuse-ld=lld /tmp/z.cc -fprofile-list=/tmp/funlist
$ export LLVM_PROFILE_FILE='a%m.profraw'
$ /tmp/a
$ /tmp/a
LLVM Profile Error: Missing profile data section.
LLVM Profile Error: Invalid profile data to merge
LLVM Profile Error: Profile Merging of file a18405209413954990729_0.profraw failed: Success
LLVM Profile Error: Failed to write file "a18405209413954990729_0.profraw": Success

basically if nothing is instrumented we'll hit this

will revert

[aeubanks]

reverted in 9b6b6bbc9127d604881cdc9dcea0e7c74ef821fd

[vitalybuka]

Reverted cd591e02d4563d4b90792ed5f441e1cf5b30443e

clang/lib/CodeGen/BackendUtil.cpp

static cl::opt<bool> ClSanitizeOnOptimizerEarlyEP(
     "sanitizer-early-opt-ep", cl::Optional,
     cl::desc("Insert sanitizers on OptimizerEarlyEP."), cl::init(false));
 }
 
 namespace {
 
 // Default filename used for profile generation.
 std::string getDefaultProfileGenName() {
-  return DebugInfoCorrelate ? "default_%p.proflite" : "default_%m.profraw";
+  return DebugInfoCorrelate ? "default_%m.proflite" : "default_%m.profraw";
 }
 
 class EmitAssemblyHelper {
   DiagnosticsEngine &Diags;
   const HeaderSearchOptions &HSOpts;
   const CodeGenOptions &CodeGenOpts;
   const clang::TargetOptions &TargetOpts;
   const LangOptions &LangOpts;

compiler-rt/lib/profile/InstrProfilingMerge.c

+#include "/tmp/debug.h"
 /*===- InstrProfilingMerge.c - Profile in-process Merging  ---------------===*\
 |*
 |* Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 |* See https://llvm.org/LICENSE.txt for license information.
 |* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 |*
 |*===----------------------------------------------------------------------===*
 |* This file defines the API needed for in-process merging of profile data
 |* stored in memory buffer.
 \*===---------------------------------------------------------------------===*/
 
 #include "InstrProfiling.h"
 #include "InstrProfilingInternal.h"
+#include "InstrProfilingPort.h"
 #include "InstrProfilingUtil.h"
 
 #define INSTR_PROF_VALUE_PROF_DATA
 #include "profile/InstrProfData.inc"
 
 COMPILER_RT_VISIBILITY
 void (*VPMergeHook)(ValueProfData *, __llvm_profile_data *);
 

 #ifdef _WIN64
   return (uintptr_t)(int32_t)(uintptr_t)V;
 #else
   return (uintptr_t)V;
 #endif
 }
 
 COMPILER_RT_VISIBILITY
 int __llvm_profile_merge_from_buffer(const char *ProfileData,
                                      uint64_t ProfileSize) {
-  if (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) {
-    PROF_ERR(
-        "%s\n",
-        "Debug info correlation does not support profile merging at runtime. "
-        "Instead, merge raw profiles using the llvm-profdata tool.");
-    return 1;
-  }
-
   __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
   __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
-  char *SrcCountersStart;
+  char *SrcCountersStart, *DstCounter;
+  const char *SrcCountersEnd, *SrcCounter;
   const char *SrcNameStart;
   const char *SrcValueProfDataStart, *SrcValueProfData;
   uintptr_t CountersDelta = Header->CountersDelta;
 
   SrcDataStart =
       (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header) +
                               Header->BinaryIdsSize);
   SrcDataEnd = SrcDataStart + Header->DataSize;
   SrcCountersStart = (char *)SrcDataEnd;
-  SrcNameStart = SrcCountersStart +
-                 Header->CountersSize * __llvm_profile_counter_entry_size();
+  SrcCountersEnd = SrcCountersStart +
+                 Header->CountersSize * __llvm_profile_counter_entry_size(); 
+  SrcNameStart = SrcCountersEnd;
   SrcValueProfDataStart =
       SrcNameStart + Header->NamesSize +
       __llvm_profile_get_num_padding_bytes(Header->NamesSize);
   if (SrcNameStart < SrcCountersStart)
     return 1;
 
+  // Merge counters when there is no data section and debug info correlation is
+  // enabled.
+  if (Header->DataSize == 0) {
+    if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) {
+      PROF_ERR("%s\n", "Missing profile data section.");
+      return 1;
+    }
+    for (SrcCounter = SrcCountersStart,
+        DstCounter = __llvm_profile_begin_counters();
+         SrcCounter < SrcCountersEnd;) {
+      if (__llvm_profile_get_version() & VARIANT_MASK_BYTE_COVERAGE) {
+        *DstCounter &= *SrcCounter;
+      } else {
+        *(uint64_t *)DstCounter += *(uint64_t *)SrcCounter;
+      }
+      SrcCounter += __llvm_profile_counter_entry_size();
+      DstCounter += __llvm_profile_counter_entry_size();
+    }
+    return 0;
+  }
+
   for (SrcData = SrcDataStart,
       DstData = (__llvm_profile_data *)__llvm_profile_begin_data(),
       SrcValueProfData = SrcValueProfDataStart;
        SrcData < SrcDataEnd; ++SrcData, ++DstData) {
     // For the in-memory destination, CounterPtr is the distance from the start
     // address of the data to the start address of the counter. On WIN64,
     // CounterPtr is a truncated 32-bit value due to COFF limitation. Sign
     // extend CounterPtr to get the original value.

compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c

 // RUN: env LLVM_PROFILE_FILE=%t.cov.proflite %run %t.cov
 // RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov.dSYM %t.cov.proflite
 
 // RUN: %clang_pgogen -o %t.cov.normal -mllvm --pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
 // RUN: env LLVM_PROFILE_FILE=%t.cov.profraw %run %t.cov.normal
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff %t.cov.normal.profdata %t.cov.profdata
+
+// Test debug info correlate with online merging.
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profdir/
+
+// RUN: diff %t.normal.profdata %t.profdata
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: %clang_pgogen -o %t.cov -g -mllvm --debug-info-correlate -mllvm -pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov.dSYM %t.profdir/
+
+// RUN: diff %t.cov.normal.profdata %t.cov.profdata

compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c

 // RUN: env LLVM_PROFILE_FILE=%t.cov.proflite %run %t.cov
 // RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov %t.cov.proflite
 
 // RUN: %clang_pgogen -o %t.cov.normal -mllvm --pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
 // RUN: env LLVM_PROFILE_FILE=%t.cov.profraw %run %t.cov.normal
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show %t.cov.normal.profdata) <(llvm-profdata show %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.profdir/
+
+// RUN: diff <(llvm-profdata show %t.normal.profdata) <(llvm-profdata show %t.profdata)
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: %clang_pgogen -o %t.cov -g -mllvm --debug-info-correlate -mllvm -pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov %t.profdir/
+
+// RUN: diff <(llvm-profdata show %t.cov.normal.profdata) <(llvm-profdata show %t.cov.profdata)