[PATCH] D139755: [InstrProf] Fix bug when merging empty profile with multiple threads

[Ellis Hoag via Phabricator via llvm-commits]

ellis created this revision.
Herald added a project: All.
ellis edited the summary of this revision.
ellis added reviewers: xur, aganea, phosek.
ellis updated this revision to Diff 481799.
ellis added a comment.
ellis published this revision for review.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Expand comment.


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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139755

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


Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===================================================================
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -349,6 +349,9 @@
     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 @@
   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;
Index: llvm/test/tools/llvm-profdata/merge_empty_profile.test
===================================================================
--- llvm/test/tools/llvm-profdata/merge_empty_profile.test
+++ 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
Index: llvm/test/tools/llvm-profdata/merge-incompatible.test
===================================================================
--- llvm/test/tools/llvm-profdata/merge-incompatible.test
+++ 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139755.481799.patch
Type: text/x-patch
Size: 3342 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221210/87c48a4b/attachment.bin>