llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-bolt Author: Amir Ayupov (aaupov) <details> <summary>Changes</summary> To avoid mixed up error messages in multi-perf case, provide diagnostics buffer and stream for each aggregator job. Test Plan: updated pre-aggregated-perf.test --- Full diff: https://github.com/llvm/llvm-project/pull/203464.diff 4 Files Affected: - (modified) bolt/include/bolt/Profile/DataAggregator.h (+11-3) - (modified) bolt/include/bolt/Profile/DataReader.h (+4-2) - (modified) bolt/lib/Profile/DataAggregator.cpp (+53-29) - (modified) bolt/test/X86/pre-aggregated-perf.test (+2-1) ``````````diff diff --git a/bolt/include/bolt/Profile/DataAggregator.h b/bolt/include/bolt/Profile/DataAggregator.h index 6f472f4084d50..fde7187973edd 100644 --- a/bolt/include/bolt/Profile/DataAggregator.h +++ b/bolt/include/bolt/Profile/DataAggregator.h @@ -51,7 +51,12 @@ class BoltAddressTranslation; /// specified by the user. class DataAggregator : public DataReader { public: - explicit DataAggregator(StringRef Filename) : DataReader(Filename) {} + explicit DataAggregator(StringRef Filename) : DataReader(Filename) { + InputFilenames.emplace_back(Filename); + } + + DataAggregator(StringRef Filename, raw_ostream &Diag) + : DataReader(Filename, Diag) {} ~DataAggregator(); @@ -156,8 +161,8 @@ class DataAggregator : public DataReader { std::vector<PerfMemSample> MemSamples; /// Perf.data or pre-aggregated inputs to aggregate and merge into this - /// reader. - std::vector<std::string> InputFilenames; + /// reader. The first entry is the primary profile input. + SmallVector<std::string, 1> InputFilenames; bool Parsed{false}; @@ -415,6 +420,9 @@ class DataAggregator : public DataReader { /// Parse this aggregator's input file. Error parseInput(); + /// Parse all input files and merge successfully parsed profiles. + Error parseAllInputs(BinaryContext &BC); + /// Merge parsed profile data from another aggregation job. void mergeFrom(const DataAggregator &Other); diff --git a/bolt/include/bolt/Profile/DataReader.h b/bolt/include/bolt/Profile/DataReader.h index 2661056772c0b..706de5a846df8 100644 --- a/bolt/include/bolt/Profile/DataReader.h +++ b/bolt/include/bolt/Profile/DataReader.h @@ -236,8 +236,10 @@ struct FuncBasicSampleData { /// class DataReader : public ProfileReaderBase { public: - explicit DataReader(StringRef Filename) - : ProfileReaderBase(Filename), Diag(errs()) {} + explicit DataReader(StringRef Filename) : DataReader(Filename, errs()) {} + + DataReader(StringRef Filename, raw_ostream &Diag) + : ProfileReaderBase(Filename), Diag(Diag) {} StringRef getReaderName() const override { return "branch profile reader"; } diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index dc716d15fb1e2..19bb63e88bdae 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -851,48 +851,49 @@ Error DataAggregator::parseInput() { return errorCodeToError(parsePreAggregated()); } -Error DataAggregator::preprocessProfile(BinaryContext &BC) { - // Turn on heatmap building if requested by --heatmap flag. - if (!opts::HeatmapMode && opts::HeatmapOutput.getNumOccurrences()) - opts::HeatmapMode = opts::HeatmapModeKind::HM_Optional; - - this->BC = &BC; +Error DataAggregator::parseAllInputs(BinaryContext &BC) { + if (InputFilenames.size() == 1) + return parseInput(); - if (!opts::ReadPreAggregated) - findPerfExecutable(); + // Multiple inputs - if (opts::ProfileFormat == opts::ProfileFormatKind::PF_PerfScript) { - if (Error E = start()) - return E; - if (Error E = generatePerfScriptData()) - return E; - exit(0); - } + // Struct keeping DataAggregator and its diagnostics buffer/stream. + struct AggregatorJob { + DataAggregator *DA{nullptr}; + std::string DiagBuffer{}; + raw_string_ostream DiagStream{DiagBuffer}; + }; - SmallVector<DataAggregator *, 1> Aggregators(1, this); + // Create a job for each input file. + SmallVector<AggregatorJob *, 1> Jobs; for (StringRef InputFilename : InputFilenames) { - auto *DA = Aggregators.emplace_back(new DataAggregator(InputFilename)); - DA->BC = &BC; - DA->PerfPath = PerfPath; + auto *Job = Jobs.emplace_back(new AggregatorJob()); + Job->DA = new DataAggregator(InputFilename, Job->DiagStream); + Job->DA->BAT = BAT; + Job->DA->BC = &BC; + Job->DA->PerfPath = PerfPath; } ThreadPoolStrategy SavedStrategy = parallel::strategy; parallel::strategy = hardware_concurrency(opts::PerfDataJobs); Error ParseErrors = - parallelForEachError(Aggregators, [](DataAggregator *DA) -> Error { - if (Error E = DA->parseInput()) - return createStringError(inconvertibleErrorCode(), "%s: %s", - DA->Filename.c_str(), - toString(std::move(E)).c_str()); - DA->Parsed = true; + parallelForEachError(Jobs, [](AggregatorJob *Job) -> Error { + if (Error E = Job->DA->parseInput()) { + Job->DiagStream.flush(); + return createStringError( + inconvertibleErrorCode(), "%s: %s%s", Job->DA->Filename.c_str(), + Job->DiagBuffer.c_str(), toString(std::move(E)).c_str()); + } + Job->DA->Parsed = true; return Error::success(); }); parallel::strategy = SavedStrategy; - for (DataAggregator *DA : llvm::drop_begin(Aggregators)) { - if (DA->Parsed) - mergeFrom(*DA); - delete DA; + for (AggregatorJob *Job : Jobs) { + if (Job->DA->Parsed) + mergeFrom(*Job->DA); + delete Job->DA; + delete Job; } if (!Parsed) @@ -900,6 +901,29 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { handleAllErrors(std::move(ParseErrors), [](const ErrorInfoBase &EI) { errs() << "PERF2BOLT-WARNING: " << EI.message() << "\n"; }); + return Error::success(); +} + +Error DataAggregator::preprocessProfile(BinaryContext &BC) { + // Turn on heatmap building if requested by --heatmap flag. + if (!opts::HeatmapMode && opts::HeatmapOutput.getNumOccurrences()) + opts::HeatmapMode = opts::HeatmapModeKind::HM_Optional; + + this->BC = &BC; + + if (!opts::ReadPreAggregated) + findPerfExecutable(); + + if (opts::ProfileFormat == opts::ProfileFormatKind::PF_PerfScript) { + if (Error E = start()) + return E; + if (Error E = generatePerfScriptData()) + return E; + exit(0); + } + + if (Error E = parseAllInputs(BC)) + return E; markFunctionsWithProfile(); diff --git a/bolt/test/X86/pre-aggregated-perf.test b/bolt/test/X86/pre-aggregated-perf.test index c2d5ea77f6703..06e57b475a4fd 100644 --- a/bolt/test/X86/pre-aggregated-perf.test +++ b/bolt/test/X86/pre-aggregated-perf.test @@ -37,11 +37,12 @@ RUN: llvm-bolt %t.exe --pa -perfdata %p/Inputs/pre-aggregated.txt -perfdata %p/I RUN: llvm-bolt %t.exe --pa -p %p/Inputs/pre-aggregated.txt,%p/Inputs/pre-aggregated.txt -o %t.multi-perfdata-comma.null | FileCheck %s --check-prefix=CHECK-MULTI RUN: not llvm-bolt %t.exe --pa -p %p/Inputs/pre-aggregated.txt -p %t.missing -o %t.multi-perfdata-missing.null 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING RUN: llvm-bolt %t.exe --pa -p %p/Inputs/pre-aggregated.txt -p %p/Inputs/pre-aggregated-bad-type.txt -o %t.multi-perfdata-bad-type.null 2>&1 | FileCheck %s --check-prefix=CHECK-BAD-TYPE-WARNING +RUN: llvm-bolt %t.exe --pa -p %p/Inputs/pre-aggregated-bad-type.txt -p %p/Inputs/pre-aggregated.txt -o %t.multi-perfdata-bad-type.null 2>&1 | FileCheck %s --check-prefix=CHECK-BAD-TYPE-WARNING CHECK: BOLT-INFO: 4 out of 7 functions in the binary (57.1%) have non-empty execution profile CHECK-MULTI: BOLT-INFO: 4 out of 7 functions in the binary (57.1%) have non-empty execution profile CHECK-MISSING: {{.*}}missing': No such file or directory. -CHECK-BAD-TYPE-WARNING: PERF2BOLT-WARNING: {{.*}}pre-aggregated-bad-type.txt: +CHECK-BAD-TYPE-WARNING: PERF2BOLT-WARNING: {{.*}}pre-aggregated-bad-type.txt: Error reading BOLT data input file: line 1, column 2: expected T, R, S, E, B, F, f or r RUN: FileCheck %s -check-prefix=PERF2BOLT --input-file %t RUN: FileCheck %s -check-prefix=NEWFORMAT --input-file %t.new `````````` </details> https://github.com/llvm/llvm-project/pull/203464 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
