Skip to content

Commit 769c7ad

Browse files
committed
[InstrProf] Fix bug when merging empty profile with multiple threads
When merging profiles with multiple threads, the `mergeWriterContexts()` function is used to merge profile data between writers. This must be in sync with `loadInput()` which merges profiles to a single writer. This diff merges the profile kind correctly in `mergeWriterContexts()` to fix a subtle bug. Reviewed By: phosek Differential Revision: https://reviews.llvm.org/D139755
1 parent ce93255 commit 769c7ad

File tree

3 files changed

+12
-7
lines changed

3 files changed

+12
-7
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,7 @@
11
RUN: not llvm-profdata merge %p/Inputs/fe-basic.proftext %p/Inputs/ir-basic.proftext -o /dev/null 2>&1 | FileCheck %s
22
CHECK: ir-basic.proftext: Merge IR generated profile with Clang generated profile.
3+
4+
// We get a slightly different error message when using multiple threads because
5+
// we do not know which files have incompatible kinds.
6+
RUN: not llvm-profdata merge %p/Inputs/fe-basic.proftext %p/Inputs/ir-basic.proftext --num-threads=2 -o /dev/null 2>&1 | FileCheck %s --check-prefix=THREADS
7+
THREADS: unsupported instrumentation profile format version

llvm/test/tools/llvm-profdata/merge_empty_profile.test

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
# Tests for merge of empty profile files.
22

33
RUN: touch %t_empty.proftext
4-
RUN: llvm-profdata merge -text -o %t_clang.proftext %t_empty.proftext %p/Inputs/clang_profile.proftext
5-
RUN: FileCheck --input-file=%t_clang.proftext %s -check-prefix=CLANG_PROF_TEXT
4+
RUN: llvm-profdata merge -text -o - %t_empty.proftext %p/Inputs/clang_profile.proftext | FileCheck %s -check-prefix=CLANG_PROF_TEXT
5+
RUN: llvm-profdata merge -text -o - %t_empty.proftext %p/Inputs/clang_profile.proftext --num-threads=2 | FileCheck %s -check-prefix=CLANG_PROF_TEXT
66
CLANG_PROF_TEXT: main
77
CLANG_PROF_TEXT: 0
88
CLANG_PROF_TEXT: 1
99
CLANG_PROF_TEXT: 1
1010

11-
RUN: llvm-profdata merge -text -o %t_ir.proftext %t_empty.proftext %p/Inputs/IR_profile.proftext
12-
RUN: FileCheck --input-file=%t_ir.proftext %s -check-prefix=IR_PROF_TEXT
11+
RUN: llvm-profdata merge -text -o - %t_empty.proftext %p/Inputs/IR_profile.proftext | FileCheck %s -check-prefix=IR_PROF_TEXT
12+
RUN: llvm-profdata merge -text -o - %t_empty.proftext %p/Inputs/IR_profile.proftext --num-threads=2 | FileCheck %s -check-prefix=IR_PROF_TEXT
1313
IR_PROF_TEXT: :ir
1414
IR_PROF_TEXT: main
1515
IR_PROF_TEXT: 0

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,9 @@ static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
349349
Dst->Errors.push_back(std::move(ErrorPair));
350350
Src->Errors.clear();
351351

352+
if (Error E = Dst->Writer.mergeProfileKind(Src->Writer.getProfileKind()))
353+
exitWithError(std::move(E));
354+
352355
Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) {
353356
instrprof_error IPE = InstrProfError::take(std::move(E));
354357
std::unique_lock<std::mutex> ErrGuard{Dst->ErrLock};
@@ -406,9 +409,6 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs,
406409
if (NumThreads == 0)
407410
NumThreads = std::min(hardware_concurrency().compute_thread_count(),
408411
unsigned((Inputs.size() + 1) / 2));
409-
// FIXME: There's a bug here, where setting NumThreads = Inputs.size() fails
410-
// the merge_empty_profile.test because the InstrProfWriter.ProfileKind isn't
411-
// merged, thus the emitted file ends up with a PF_Unknown kind.
412412

413413
// Initialize the writer contexts.
414414
SmallVector<std::unique_ptr<WriterContext>, 4> Contexts;

0 commit comments

Comments
 (0)