From 769c7ad2b1b9bca7e13cf6211e9d61439f6df8d6 Mon Sep 17 00:00:00 2001 From: Ellis Hoag Date: Fri, 9 Dec 2022 17:12:30 -0800 Subject: [PATCH] [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 --- llvm/test/tools/llvm-profdata/merge-incompatible.test | 5 +++++ llvm/test/tools/llvm-profdata/merge_empty_profile.test | 8 ++++---- llvm/tools/llvm-profdata/llvm-profdata.cpp | 6 +++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/llvm/test/tools/llvm-profdata/merge-incompatible.test b/llvm/test/tools/llvm-profdata/merge-incompatible.test index dad1c0a..db77af9 100644 --- a/llvm/test/tools/llvm-profdata/merge-incompatible.test +++ b/llvm/test/tools/llvm-profdata/merge-incompatible.test @@ -1,2 +1,7 @@ RUN: not llvm-profdata merge %p/Inputs/fe-basic.proftext %p/Inputs/ir-basic.proftext -o /dev/null 2>&1 | FileCheck %s CHECK: ir-basic.proftext: Merge IR generated profile with Clang generated profile. + +// We get a slightly different error message when using multiple threads because +// we do not know which files have incompatible kinds. +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 +THREADS: unsupported instrumentation profile format version diff --git a/llvm/test/tools/llvm-profdata/merge_empty_profile.test b/llvm/test/tools/llvm-profdata/merge_empty_profile.test index 7f9d31b..013a4dc 100644 --- a/llvm/test/tools/llvm-profdata/merge_empty_profile.test +++ b/llvm/test/tools/llvm-profdata/merge_empty_profile.test @@ -1,15 +1,15 @@ # Tests for merge of empty profile files. RUN: touch %t_empty.proftext -RUN: llvm-profdata merge -text -o %t_clang.proftext %t_empty.proftext %p/Inputs/clang_profile.proftext -RUN: FileCheck --input-file=%t_clang.proftext %s -check-prefix=CLANG_PROF_TEXT +RUN: llvm-profdata merge -text -o - %t_empty.proftext %p/Inputs/clang_profile.proftext | FileCheck %s -check-prefix=CLANG_PROF_TEXT +RUN: llvm-profdata merge -text -o - %t_empty.proftext %p/Inputs/clang_profile.proftext --num-threads=2 | FileCheck %s -check-prefix=CLANG_PROF_TEXT CLANG_PROF_TEXT: main CLANG_PROF_TEXT: 0 CLANG_PROF_TEXT: 1 CLANG_PROF_TEXT: 1 -RUN: llvm-profdata merge -text -o %t_ir.proftext %t_empty.proftext %p/Inputs/IR_profile.proftext -RUN: FileCheck --input-file=%t_ir.proftext %s -check-prefix=IR_PROF_TEXT +RUN: llvm-profdata merge -text -o - %t_empty.proftext %p/Inputs/IR_profile.proftext | FileCheck %s -check-prefix=IR_PROF_TEXT +RUN: llvm-profdata merge -text -o - %t_empty.proftext %p/Inputs/IR_profile.proftext --num-threads=2 | FileCheck %s -check-prefix=IR_PROF_TEXT IR_PROF_TEXT: :ir IR_PROF_TEXT: main IR_PROF_TEXT: 0 diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp index a1f68e1..76b745b 100644 --- a/llvm/tools/llvm-profdata/llvm-profdata.cpp +++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp @@ -349,6 +349,9 @@ static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) { Dst->Errors.push_back(std::move(ErrorPair)); Src->Errors.clear(); + if (Error E = Dst->Writer.mergeProfileKind(Src->Writer.getProfileKind())) + exitWithError(std::move(E)); + Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) { instrprof_error IPE = InstrProfError::take(std::move(E)); std::unique_lock ErrGuard{Dst->ErrLock}; @@ -406,9 +409,6 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs, if (NumThreads == 0) NumThreads = std::min(hardware_concurrency().compute_thread_count(), unsigned((Inputs.size() + 1) / 2)); - // FIXME: There's a bug here, where setting NumThreads = Inputs.size() fails - // the merge_empty_profile.test because the InstrProfWriter.ProfileKind isn't - // merged, thus the emitted file ends up with a PF_Unknown kind. // Initialize the writer contexts. SmallVector, 4> Contexts; -- 2.7.4