https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/228158
>From ef79cbf330b9a7bcd1a320e401045401dd17a87a Mon Sep 17 00:00:00 2001 From: Vitaly Buka <[email protected]> Date: Thu, 1 Oct 2026 09:55:10 -0700 Subject: [PATCH] [NFCI][llvm-profdata] Propagate Error in loadInput and mergeWriterContexts Propagate Error from loadInput and mergeWriterContexts in mergeInstrProfile, supplementInstrProfile, and overlapInstrProfile. In mergeInstrProfile's ThreadPool, catch errors from worker threads, stop scheduling new jobs, and return the first encountered fatal error. Ensure ~WriterContext() consumes any pending unhandled errors in WriterContext::Errors upon destruction. Not NFC as destructors are run on the stack and ThreadPool workers exit earlier on error. Assisted-by: Gemini --- llvm/tools/llvm-profdata/llvm-profdata.cpp | 90 +++++++++++++++------- 1 file changed, 64 insertions(+), 26 deletions(-) diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp index cb09fdd3c104f..4ff3d6667ac64 100644 --- a/llvm/tools/llvm-profdata/llvm-profdata.cpp +++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp @@ -753,6 +753,11 @@ struct WriterContext { MemProfVersionRequested, MemProfFullSchema, MemprofGenerateRandomHotness, RandomSeed), ErrLock(ErrLock), WriterErrorCodes(WriterErrorCodes) {} + + ~WriterContext() { + for (auto &ErrorPair : Errors) + consumeError(std::move(ErrorPair.first)); + } }; /// Computer the overlap b/w profile BaseFilename and TestFileName, @@ -784,7 +789,7 @@ static void overlapInput(const std::string &BaseFilename, } /// Load an input into a writer context. -static void +static Error loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, const InstrProfCorrelator *Correlator, const StringRef ProfiledBinary, WriterContext *WC, const object::BuildIDFetcher *BIDFetcher = nullptr, @@ -799,9 +804,8 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, using ::llvm::memprof::RawMemProfReader; if (RawMemProfReader::hasFormat(Input.Filename)) { auto ReaderOrErr = RawMemProfReader::create(Input.Filename, ProfiledBinary); - if (!ReaderOrErr) { - exitWithError(ReaderOrErr.takeError(), Input.Filename); - } + if (!ReaderOrErr) + return makeError(ReaderOrErr.takeError(), Input.Filename); std::unique_ptr<RawMemProfReader> Reader = std::move(ReaderOrErr.get()); // Check if the profile types can be merged, e.g. clang frontend profiles // should not be merged with memprof profiles. @@ -812,7 +816,7 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, "Cannot merge MemProf profile with Clang generated profile.", std::error_code()), Filename); - return; + return Error::success(); } auto MemProfError = [&](Error E) { @@ -822,14 +826,14 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, }; WC->Writer.addMemProfData(Reader->takeMemProfData(), MemProfError); - return; + return Error::success(); } using ::llvm::memprof::YAMLMemProfReader; if (YAMLMemProfReader::hasFormat(Input.Filename)) { auto ReaderOrErr = YAMLMemProfReader::create(Input.Filename); if (!ReaderOrErr) - exitWithError(ReaderOrErr.takeError(), Input.Filename); + return makeError(ReaderOrErr.takeError(), Input.Filename); std::unique_ptr<YAMLMemProfReader> Reader = std::move(ReaderOrErr.get()); // Check if the profile types can be merged, e.g. clang frontend profiles // should not be merged with memprof profiles. @@ -840,7 +844,7 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, "Cannot merge MemProf profile with incompatible profile.", std::error_code()), Filename); - return; + return Error::success(); } auto MemProfError = [&](Error E) { @@ -863,7 +867,7 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, WC->Writer.addMemProfData(std::move(MemProfData), MemProfError); WC->Writer.addDataAccessProfData(std::move(DataAccessProfData)); - return; + return Error::success(); } auto FS = vfs::getRealFileSystem(); @@ -899,13 +903,13 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, if (ErrCode != instrprof_error::empty_raw_profile) WC->Errors.emplace_back(make_error<InstrProfError>(ErrCode, Msg), Filename); - return; + return Error::success(); } auto Reader = std::move(ReaderOrErr.get()); if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) { WC->Errors.emplace_back(std::move(E), Filename); - return; + return Error::success(); } for (auto &I : *Reader) { @@ -946,14 +950,14 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, if (Reader->hasError()) { if (Error E = Reader->getError()) { WC->Errors.emplace_back(std::move(E), Filename); - return; + return Error::success(); } } std::vector<llvm::object::BuildID> BinaryIds; if (Error E = Reader->readBinaryIds(BinaryIds)) { WC->Errors.emplace_back(std::move(E), Filename); - return; + return Error::success(); } WC->Writer.addBinaryIds(BinaryIds); @@ -961,16 +965,17 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper, WC->Errors.emplace_back(std::move(ReaderWarning->first), ReaderWarning->second); } + return Error::success(); } /// Merge the \p Src writer context into \p Dst. -static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) { +static Error mergeWriterContexts(WriterContext *Dst, WriterContext *Src) { for (auto &ErrorPair : Src->Errors) Dst->Errors.push_back(std::move(ErrorPair)); Src->Errors.clear(); if (Error E = Dst->Writer.mergeProfileKind(Src->Writer.getProfileKind())) - exitWithError(std::move(E)); + return makeError(std::move(E)); Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) { auto [ErrorCode, Msg] = InstrProfError::take(std::move(E)); @@ -979,6 +984,7 @@ static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) { if (firstTime) warn(toString(make_error<InstrProfError>(ErrorCode, Msg))); }); + return Error::success(); } static StringRef @@ -1137,20 +1143,46 @@ static Error mergeInstrProfile(const WeightedFileVector &Inputs, if (NumThreads == 1) { for (const auto &Input : Inputs) - loadInput(Input, Remapper, Correlator.get(), ProfiledBinary, - Contexts[0].get(), BIDFetcher.get(), &BIDFetcherCorrelateKind); + if (Error E = loadInput(Input, Remapper, Correlator.get(), ProfiledBinary, + Contexts[0].get(), BIDFetcher.get(), + &BIDFetcherCorrelateKind)) + return E; } else { + Error FatalError = Error::success(); + auto hasFatalError = [&] { + std::unique_lock<std::mutex> ErrGuard{ErrorLock}; + return static_cast<bool>(FatalError); + }; + DefaultThreadPool Pool(hardware_concurrency(NumThreads)); + auto Async = [&](auto F, auto &&...Args) { + Pool.async( + [&, F](auto &&...InnerArgs) { + if (hasFatalError()) + return; + if (Error E = F(std::forward<decltype(InnerArgs)>(InnerArgs)...)) { + std::unique_lock<std::mutex> ErrGuard{ErrorLock}; + if (FatalError) + consumeError(std::move(E)); + else + FatalError = std::move(E); + } + }, + std::forward<decltype(Args)>(Args)...); + }; // Load the inputs in parallel (N/NumThreads serial steps). unsigned Ctx = 0; for (const auto &Input : Inputs) { - Pool.async(loadInput, Input, Remapper, Correlator.get(), ProfiledBinary, - Contexts[Ctx].get(), BIDFetcher.get(), - &BIDFetcherCorrelateKind); + if (hasFatalError()) + break; + Async(loadInput, Input, Remapper, Correlator.get(), ProfiledBinary, + Contexts[Ctx].get(), BIDFetcher.get(), &BIDFetcherCorrelateKind); Ctx = (Ctx + 1) % NumThreads; } Pool.wait(); + if (FatalError) + return FatalError; // Merge the writer contexts together (~ lg(NumThreads) serial steps). unsigned Mid = Contexts.size() / 2; @@ -1158,13 +1190,15 @@ static Error mergeInstrProfile(const WeightedFileVector &Inputs, assert(Mid > 0 && "Expected more than one context"); do { for (unsigned I = 0; I < Mid; ++I) - Pool.async(mergeWriterContexts, Contexts[I].get(), - Contexts[I + Mid].get()); + Async(mergeWriterContexts, Contexts[I].get(), Contexts[I + Mid].get()); Pool.wait(); + if (FatalError) + return FatalError; if (End & 1) { - Pool.async(mergeWriterContexts, Contexts[0].get(), - Contexts[End - 1].get()); + Async(mergeWriterContexts, Contexts[0].get(), Contexts[End - 1].get()); Pool.wait(); + if (FatalError) + return FatalError; } End = Mid; Mid /= 2; @@ -1570,7 +1604,9 @@ static Error supplementInstrProfile(const WeightedFileVector &Inputs, SmallSet<instrprof_error, 4> WriterErrorCodes; auto WC = std::make_unique<WriterContext>(OutputSparse, ErrorLock, WriterErrorCodes); - loadInput(Inputs[0], nullptr, nullptr, /*ProfiledBinary=*/"", WC.get()); + if (Error E = loadInput(Inputs[0], nullptr, nullptr, /*ProfiledBinary=*/"", + WC.get())) + return E; if (!WC->Errors.empty()) return makeError(std::move(WC->Errors[0].first), InstrFilename); @@ -1981,7 +2017,9 @@ static Error overlapInstrProfile(const std::string &BaseFilename, OS << "Sum of edge counts for profile " << TestFilename << " is 0.\n"; return Error::success(); } - loadInput(WeightedInput, nullptr, nullptr, /*ProfiledBinary=*/"", &Context); + if (Error E = loadInput(WeightedInput, nullptr, nullptr, + /*ProfiledBinary=*/"", &Context)) + return E; overlapInput(BaseFilename, TestFilename, &Context, Overlap, FuncFilter, OS, IsCS); Overlap.dump(OS); _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
