[llvm] 769c7ad - [InstrProf] Fix bug when merging empty profile with multiple threads

[Ellis Hoag via llvm-commits]

Author: Ellis Hoag
Date: 2022-12-12T09:23:26-08:00
New Revision: 769c7ad2b1b9bca7e13cf6211e9d61439f6df8d6

URL: https://github.com/llvm/llvm-project/commit/769c7ad2b1b9bca7e13cf6211e9d61439f6df8d6
DIFF: https://github.com/llvm/llvm-project/commit/769c7ad2b1b9bca7e13cf6211e9d61439f6df8d6.diff

LOG: [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

Added: 
    

Modified: 
    llvm/test/tools/llvm-profdata/merge-incompatible.test
    llvm/test/tools/llvm-profdata/merge_empty_profile.test
    llvm/tools/llvm-profdata/llvm-profdata.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-profdata/merge-incompatible.test b/llvm/test/tools/llvm-profdata/merge-incompatible.test
index dad1c0ad5dbbe..db77af9a8a54a 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 
diff erent 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 7f9d31bd8f57b..013a4dcda6554 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 a1f68e1b41095..76b745b13448e 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<std::mutex> 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<std::unique_ptr<WriterContext>, 4> Contexts;