From d4730ea555f1ac7a84a12199a2e9627dc9e4e31c Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Fri, 16 Jan 2015 15:54:13 +0000 Subject: [PATCH] Run the resolver in parallel with the reader. This patch makes File::parse() multi-thread safe. If one thread is running File::parse(), other threads will block if they try to call the same method. File::parse() is idempotent, so you can safely call multiple times. With this change, we don't have to wait for all worker threads to finish in Driver::link(). Previously, Driver::link() calls TaskGroup::sync() to wait for all threads running File::parse(). This was not ideal because we couldn't start the resolver until we parse all files. This patch increase parallelism by making Driver::link() to not wait for worker threads. The resolver calls parse() to make sure that the file being read has been parsed, and then uses the file. In this approach, the resolver can run with the parser threads in parallel. http://reviews.llvm.org/D6994 llvm-svn: 226281 --- lld/include/lld/Core/File.h | 10 ++--- lld/include/lld/Core/LinkingContext.h | 4 ++ lld/include/lld/Core/Resolver.h | 2 +- lld/lib/Core/File.cpp | 8 ++++ lld/lib/Core/Resolver.cpp | 14 +++++-- lld/lib/Driver/Driver.cpp | 39 ++----------------- lld/lib/ReaderWriter/ELF/ELFFile.h | 4 +- .../PECOFF/PECOFFLinkingContext.cpp | 11 +++++- 8 files changed, 43 insertions(+), 49 deletions(-) diff --git a/lld/include/lld/Core/File.h b/lld/include/lld/Core/File.h index 6708592e7c67..a60985068ef1 100644 --- a/lld/include/lld/Core/File.h +++ b/lld/include/lld/Core/File.h @@ -20,6 +20,7 @@ #include "llvm/Support/ErrorHandling.h" #include #include +#include #include namespace lld { @@ -161,11 +162,7 @@ public: /// (because YAML reader does not read blobs but structured data). void setLastError(std::error_code err) { _lastError = err; } - std::error_code parse() { - if (!_lastError.hasValue()) - _lastError = doParse(); - return _lastError.getValue(); - } + std::error_code parse(); // Usually each file owns a std::unique_ptr. // However, there's one special case. If a file is an archive file, @@ -239,7 +236,6 @@ protected: static atom_collection_empty _noUndefinedAtoms; static atom_collection_empty _noSharedLibraryAtoms; static atom_collection_empty _noAbsoluteAtoms; - llvm::Optional _lastError; mutable llvm::BumpPtrAllocator _allocator; private: @@ -247,6 +243,8 @@ private: Kind _kind; mutable uint64_t _ordinal; std::shared_ptr _sharedMemoryBuffer; + llvm::Optional _lastError; + std::mutex _parseMutex; }; /// \brief A mutable File. diff --git a/lld/include/lld/Core/LinkingContext.h b/lld/include/lld/Core/LinkingContext.h index 24108fbb9e5e..7b5e64612acd 100644 --- a/lld/include/lld/Core/LinkingContext.h +++ b/lld/include/lld/Core/LinkingContext.h @@ -13,6 +13,7 @@ #include "lld/Core/Error.h" #include "lld/Core/LLVM.h" #include "lld/Core/Node.h" +#include "lld/Core/Parallel.h" #include "lld/Core/Reference.h" #include "lld/Core/range.h" #include "lld/ReaderWriter/Reader.h" @@ -323,6 +324,8 @@ public: // Derived classes may use that chance to rearrange the input files. virtual void maybeSortInputFiles() {} + TaskGroup &getTaskGroup() { return _taskGroup; } + /// @} protected: LinkingContext(); // Must be subclassed @@ -370,6 +373,7 @@ protected: private: /// Validate the subclass bits. Only called by validate. virtual bool validateImpl(raw_ostream &diagnostics) = 0; + TaskGroup _taskGroup; }; } // end namespace lld diff --git a/lld/include/lld/Core/Resolver.h b/lld/include/lld/Core/Resolver.h index db578fb5b4b9..9d0a1da4c9a6 100644 --- a/lld/include/lld/Core/Resolver.h +++ b/lld/include/lld/Core/Resolver.h @@ -62,7 +62,7 @@ private: void maybeAddSectionGroupOrGnuLinkOnce(const DefinedAtom &atom); /// \brief The main function that iterates over the files to resolve - void resolveUndefines(); + bool resolveUndefines(); void updateReferences(); void deadStripOptimize(); bool checkUndefines(); diff --git a/lld/lib/Core/File.cpp b/lld/lib/Core/File.cpp index 624fca9386da..dbac86b368aa 100644 --- a/lld/lib/Core/File.cpp +++ b/lld/lib/Core/File.cpp @@ -9,6 +9,7 @@ #include "lld/Core/File.h" #include "lld/Core/LLVM.h" +#include namespace lld { @@ -19,4 +20,11 @@ File::atom_collection_empty File::_noUndefinedAtoms; File::atom_collection_empty File::_noSharedLibraryAtoms; File::atom_collection_empty File::_noAbsoluteAtoms; +std::error_code File::parse() { + std::lock_guard lock(_parseMutex); + if (!_lastError.hasValue()) + _lastError = doParse(); + return _lastError.getValue(); +} + } // namespace lld diff --git a/lld/lib/Core/Resolver.cpp b/lld/lib/Core/Resolver.cpp index ae2c71f06ad0..da7fd53c1203 100644 --- a/lld/lib/Core/Resolver.cpp +++ b/lld/lib/Core/Resolver.cpp @@ -236,7 +236,7 @@ bool Resolver::undefinesAdded(int begin, int end) { for (int i = begin; i < end; ++i) if (FileNode *node = dyn_cast(inputs[i].get())) if (_newUndefinesAdded[node->getFile()]) - return true; + return true; return false; } @@ -263,7 +263,7 @@ File *Resolver::getFile(int &index, int &groupLevel) { // Keep adding atoms until _context.getNextFile() returns an error. This // function is where undefined atoms are resolved. -void Resolver::resolveUndefines() { +bool Resolver::resolveUndefines() { ScopedTask task(getDefaultDomain(), "resolveUndefines"); int index = 0; int groupLevel = 0; @@ -271,7 +271,12 @@ void Resolver::resolveUndefines() { bool undefAdded = false; File *file = getFile(index, groupLevel); if (!file) - return; + return true; + if (std::error_code ec = file->parse()) { + llvm::errs() << "Cannot open " + file->path() + << ": " << ec.message() << "\n"; + return false; + } switch (file->kind()) { case File::kindObject: if (groupLevel > 0) @@ -441,7 +446,8 @@ void Resolver::removeCoalescedAwayAtoms() { } bool Resolver::resolve() { - resolveUndefines(); + if (!resolveUndefines()) + return false; updateReferences(); deadStripOptimize(); if (checkUndefines()) diff --git a/lld/lib/Driver/Driver.cpp b/lld/lib/Driver/Driver.cpp index 85c2bd9d4029..dc4b49132d09 100644 --- a/lld/lib/Driver/Driver.cpp +++ b/lld/lib/Driver/Driver.cpp @@ -77,42 +77,9 @@ bool Driver::link(LinkingContext &context, raw_ostream &diagnostics) { if (context.getNodes().empty()) return false; - bool fail = false; - - // Read inputs - ScopedTask readTask(getDefaultDomain(), "Read Args"); - TaskGroup tg; - std::mutex diagnosticsMutex; - for (std::unique_ptr &ie : context.getNodes()) { - tg.spawn([&] { - // Writes to the same output stream is not guaranteed to be thread-safe. - // We buffer the diagnostics output to a separate string-backed output - // stream, acquire the lock, and then print it out. - std::string buf; - llvm::raw_string_ostream stream(buf); - - if (FileNode *node = dyn_cast(ie.get())) { - if (File *file = node->getFile()) { - if (std::error_code ec = file->parse()) { - stream << "Cannot open " + file->path() - << ": " << ec.message() << "\n"; - fail = true; - } - } - } - - stream.flush(); - if (!buf.empty()) { - std::lock_guard lock(diagnosticsMutex); - diagnostics << buf; - } - }); - } - tg.sync(); - readTask.end(); - - if (fail) - return false; + for (std::unique_ptr &ie : context.getNodes()) + if (FileNode *node = dyn_cast(ie.get())) + context.getTaskGroup().spawn([node] { node->getFile()->parse(); }); std::vector> internalFiles; context.createInternalFiles(internalFiles); diff --git a/lld/lib/ReaderWriter/ELF/ELFFile.h b/lld/lib/ReaderWriter/ELF/ELFFile.h index baab0d3a5bad..388e5b89089f 100644 --- a/lld/lib/ReaderWriter/ELF/ELFFile.h +++ b/lld/lib/ReaderWriter/ELF/ELFFile.h @@ -116,7 +116,9 @@ template class ELFFile : public File { public: ELFFile(StringRef name) - : File(name, kindObject), _ordinal(0), _doStringsMerge(false) {} + : File(name, kindObject), _ordinal(0), _doStringsMerge(false) { + setLastError(std::error_code()); + } ELFFile(std::unique_ptr mb, bool atomizeStrings = false) : File(mb->getBufferIdentifier(), kindObject), _mb(std::move(mb)), diff --git a/lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp b/lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp index 97ef52a5eaf1..e7e07f1a51d6 100644 --- a/lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp +++ b/lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp @@ -87,6 +87,13 @@ std::unique_ptr PECOFFLinkingContext::createUndefinedSymbolFile() const { ""); } +static int getGroupStartPos(std::vector> &nodes) { + for (int i = 0, e = nodes.size(); i < e; ++i) + if (GroupEnd *group = dyn_cast(nodes[i].get())) + return i - group->getSize(); + llvm::report_fatal_error("internal error"); +} + void PECOFFLinkingContext::addLibraryFile(std::unique_ptr file) { GroupEnd *currentGroupEnd; int pos = -1; @@ -111,7 +118,7 @@ bool PECOFFLinkingContext::createImplicitFiles( // Create a file for the entry point function. std::unique_ptr entry(new FileNode( llvm::make_unique(*this, syms))); - members.insert(members.begin(), std::move(entry)); + members.insert(members.begin() + getGroupStartPos(members), std::move(entry)); // Create a file for __ImageBase. std::unique_ptr fileNode(new FileNode( @@ -339,6 +346,8 @@ void pecoff::ResolvableSymbols::add(File *file) { void pecoff::ResolvableSymbols::readAllSymbols() { std::lock_guard lock(_mutex); for (File *file : _queue) { + if (file->parse()) + return; if (auto *archive = dyn_cast(file)) { for (const std::string &sym : archive->getDefinedSymbols()) _defined.insert(sym); -- GitLab