[lld] r205463 - Simplify communication between Resolver and Input Graph.
Rui Ueyama
ruiu at google.com
Wed Apr 2 14:02:44 PDT 2014
Author: ruiu
Date: Wed Apr 2 16:02:44 2014
New Revision: 205463
URL: http://llvm.org/viewvc/llvm-project?rev=205463&view=rev
Log:
Simplify communication between Resolver and Input Graph.
Resolver is sending too much information to Input Graph than Input
Graph actually needs. In order to collect the detailed information,
which wouldn't be consumed by anyone, we have a good amount of code
in Resolver, Input Graph and Input Elements. This patch is to
simplify it. No functionality change.
Specifically, this patch replaces ResolverState enum with a boolean
value. The enum defines many bits to notify the progress about
linking to Input Graph using bit masks, however, what Input Graph
actually does is to compare a given value with 0. The details of
the bit mask is simply being ignored, so the efforts to collect
such data is wasted.
This patch also changes the name of the notification interface from
setResolverState to notifyProgress, to make it sounds more like
message passing style. It's not a setter but something to notify of
an update, so the new name should be more appropriate than before.
Differential Revision: http://llvm-reviews.chandlerc.com/D3267
Modified:
lld/trunk/include/lld/Core/InputGraph.h
lld/trunk/include/lld/Core/Resolver.h
lld/trunk/include/lld/Driver/GnuLdInputGraph.h
lld/trunk/lib/Core/InputGraph.cpp
lld/trunk/lib/Core/Resolver.cpp
lld/trunk/unittests/DriverTests/InputGraphTest.cpp
Modified: lld/trunk/include/lld/Core/InputGraph.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Core/InputGraph.h?rev=205463&r1=205462&r2=205463&view=diff
==============================================================================
--- lld/trunk/include/lld/Core/InputGraph.h (original)
+++ lld/trunk/include/lld/Core/InputGraph.h Wed Apr 2 16:02:44 2014
@@ -62,12 +62,11 @@ public:
/// resolved.
ErrorOr<File &> nextFile();
- /// Set the resolver state for the current Input element. This is used by the
- /// InputGraph to decide the next file that needs to be processed for various
- /// types of nodes in the InputGraph. The resolver state is nothing but a
- /// bitmask of various types of states that the resolver handles when adding
- /// atoms.
- void setResolverState(uint32_t resolverState);
+ /// Notifies the current input element of Resolver made some progress on
+ /// resolving undefined symbols using the current file. Group (representing
+ /// --start-group and --end-group) uses that notification to make a decision
+ /// whether it should iterate over again or terminate or not.
+ void notifyProgress();
/// \brief Adds a node into the InputGraph
bool addInputElement(std::unique_ptr<InputElement>);
@@ -153,11 +152,9 @@ public:
/// Get the next file to be processed by the resolver
virtual ErrorOr<File &> getNextFile() = 0;
- /// \brief Set the resolve state for the element
- virtual void setResolveState(uint32_t state) = 0;
-
- /// \brief Get the resolve state for the element
- virtual uint32_t getResolveState() const = 0;
+ /// Refer InputGraph::notifyProgress(). By default, it does nothing. Only
+ /// Group is interested in this message.
+ virtual void notifyProgress() {};
/// \brief Reset the next index
virtual void resetNextIndex() = 0;
@@ -183,7 +180,7 @@ class Group : public InputElement {
public:
Group(int64_t ordinal)
: InputElement(InputElement::Kind::Group, ordinal),
- _currentElementIndex(0), _nextElementIndex(0) {}
+ _currentElementIndex(0), _nextElementIndex(0), _madeProgress(false) {}
static inline bool classof(const InputElement *a) {
return a->kind() == InputElement::Kind::Group;
@@ -200,7 +197,9 @@ public:
}
void resetNextIndex() override {
- _currentElementIndex = _nextElementIndex = 0;
+ _madeProgress = false;
+ _currentElementIndex = 0;
+ _nextElementIndex = 0;
for (auto &elem : _elements)
elem->resetNextIndex();
}
@@ -213,14 +212,21 @@ public:
return error_code::success();
}
- uint32_t getResolveState() const override;
- void setResolveState(uint32_t) override;
+ /// If Resolver made a progress using the current file, it's ok to revisit
+ /// files in this group in future.
+ void notifyProgress() override {
+ for (auto &elem : _elements)
+ elem->notifyProgress();
+ _madeProgress = true;
+ }
+
ErrorOr<File &> getNextFile() override;
protected:
InputGraph::InputElementVectorT _elements;
uint32_t _currentElementIndex;
uint32_t _nextElementIndex;
+ bool _madeProgress;
};
/// \brief Represents an Input file in the graph
@@ -267,15 +273,7 @@ public:
/// \brief Reset the file index if the resolver needs to process
/// the node again.
- void resetNextIndex() override;
-
- /// \brief Set the resolve state for the FileNode.
- void setResolveState(uint32_t resolveState) override {
- _resolveState = resolveState;
- }
-
- /// \brief Retrieve the resolve state of the FileNode.
- uint32_t getResolveState() const override { return _resolveState; }
+ void resetNextIndex() override { _nextFileIndex = 0; }
protected:
/// \brief Read the file into _buffer.
Modified: lld/trunk/include/lld/Core/Resolver.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Core/Resolver.h?rev=205463&r1=205462&r2=205463&view=diff
==============================================================================
--- lld/trunk/include/lld/Core/Resolver.h (original)
+++ lld/trunk/include/lld/Core/Resolver.h Wed Apr 2 16:02:44 2014
@@ -28,14 +28,6 @@ class LinkingContext;
/// and producing a merged graph.
class Resolver {
public:
- enum ResolverState {
- StateNoChange = 0, // The default resolver state
- StateNewDefinedAtoms = 1, // New defined atoms were added
- StateNewUndefinedAtoms = 2, // New undefined atoms were added
- StateNewSharedLibraryAtoms = 4, // New shared library atoms were added
- StateNewAbsoluteAtoms = 8 // New absolute atoms were added
- };
-
Resolver(LinkingContext &context)
: _context(context), _symbolTable(context), _result(new MergedFile()) {}
Modified: lld/trunk/include/lld/Driver/GnuLdInputGraph.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Driver/GnuLdInputGraph.h?rev=205463&r1=205462&r2=205463&view=diff
==============================================================================
--- lld/trunk/include/lld/Driver/GnuLdInputGraph.h (original)
+++ lld/trunk/include/lld/Driver/GnuLdInputGraph.h Wed Apr 2 16:02:44 2014
@@ -89,7 +89,6 @@ public:
(_files[0]->kind() == File::kindSharedLibrary)) {
_nextFileIndex = 0;
}
- setResolveState(Resolver::StateNoChange);
}
/// \brief Return the file that has to be processed by the resolver
@@ -146,14 +145,6 @@ public:
return make_range(_expandElements.begin(), _expandElements.end());
}
- void setResolveState(uint32_t) override {
- llvm_unreachable("cannot use this function: setResolveState");
- }
-
- uint32_t getResolveState() const override {
- llvm_unreachable("cannot use this function: getResolveState");
- }
-
// Do nothing here.
void resetNextIndex() override {}
Modified: lld/trunk/lib/Core/InputGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/InputGraph.cpp?rev=205463&r1=205462&r2=205463&view=diff
==============================================================================
--- lld/trunk/lib/Core/InputGraph.cpp (original)
+++ lld/trunk/lib/Core/InputGraph.cpp Wed Apr 2 16:02:44 2014
@@ -47,9 +47,7 @@ ErrorOr<File &> InputGraph::nextFile() {
}
}
-void InputGraph::setResolverState(uint32_t state) {
- _currentInputElement->setResolveState(state);
-}
+void InputGraph::notifyProgress() { _currentInputElement->notifyProgress(); }
bool InputGraph::addInputElement(std::unique_ptr<InputElement> ie) {
_inputArgs.push_back(std::move(ie));
@@ -123,7 +121,7 @@ InputElement::InputElement(Kind type, in
/// FileNode
FileNode::FileNode(StringRef path, int64_t ordinal)
: InputElement(InputElement::Kind::File, ordinal), _path(path),
- _resolveState(Resolver::StateNoChange), _nextFileIndex(0) {}
+ _nextFileIndex(0) {}
/// \brief Read the file into _buffer.
error_code FileNode::getBuffer(StringRef filePath) {
@@ -135,33 +133,6 @@ error_code FileNode::getBuffer(StringRef
return error_code::success();
}
-// Reset the next file that would be be processed by the resolver.
-// Reset the resolve state too.
-void FileNode::resetNextIndex() {
- _nextFileIndex = 0;
- setResolveState(Resolver::StateNoChange);
-}
-
-/// ControlNode
-
-/// \brief Get the resolver State. The return value of the resolve
-/// state for a control node is the or'ed value of the resolve states
-/// contained in it.
-uint32_t Group::getResolveState() const {
- uint32_t resolveState = Resolver::StateNoChange;
- for (auto &elem : _elements)
- resolveState |= elem->getResolveState();
- return resolveState;
-}
-
-/// \brief Set the resolve state for the current element
-/// thats processed by the resolver.
-void Group::setResolveState(uint32_t resolveState) {
- if (_elements.empty())
- return;
- _elements[_currentElementIndex]->setResolveState(resolveState);
-}
-
/// Group
/// \brief Return the next file that need to be processed by the resolver.
@@ -173,14 +144,15 @@ ErrorOr<File &> Group::getNextFile() {
return make_error_code(InputGraphError::no_more_files);
for (;;) {
- // If we have processed all the elements as part of this node
- // check the resolver status for each input element and if the status
- // has not changed, move onto the next file.
+ // If we have processed all the elements, and have made no progress on
+ // linking, we cannot resolve any symbol from this group. Continue to the
+ // next one by returning no_more_files.
if (_nextElementIndex == _elements.size()) {
- if (getResolveState() == Resolver::StateNoChange)
+ if (!_madeProgress)
return make_error_code(InputGraphError::no_more_files);
resetNextIndex();
}
+
_currentElementIndex = _nextElementIndex;
auto file = _elements[_nextElementIndex]->getNextFile();
// Move on to the next element if we have finished processing all
Modified: lld/trunk/lib/Core/Resolver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/Resolver.cpp?rev=205463&r1=205462&r2=205463&view=diff
==============================================================================
--- lld/trunk/lib/Core/Resolver.cpp (original)
+++ lld/trunk/lib/Core/Resolver.cpp Wed Apr 2 16:02:44 2014
@@ -69,19 +69,19 @@ private:
} // namespace
void Resolver::handleFile(const File &file) {
- uint32_t resolverState = Resolver::StateNoChange;
const SharedLibraryFile *sharedLibraryFile =
dyn_cast<SharedLibraryFile>(&file);
- for (const DefinedAtom *atom : file.defined()) {
+ for (const DefinedAtom *atom : file.defined())
doDefinedAtom(*atom);
- resolverState |= StateNewDefinedAtoms;
- }
+ bool progress = false;
+
if (!sharedLibraryFile ||
_context.addUndefinedAtomsFromSharedLibrary(sharedLibraryFile)) {
+ progress = (file.undefined().size() > 0);
+
for (const UndefinedAtom *undefAtom : file.undefined()) {
doUndefinedAtom(*undefAtom);
- resolverState |= StateNewUndefinedAtoms;
// If the undefined symbol has an alternative name, try to resolve the
// symbol with the name to give it a second chance. This feature is used
// for COFF "weak external" symbol.
@@ -93,15 +93,19 @@ void Resolver::handleFile(const File &fi
}
}
}
- for (const SharedLibraryAtom *shlibAtom : file.sharedLibrary()) {
+ for (const SharedLibraryAtom *shlibAtom : file.sharedLibrary())
doSharedLibraryAtom(*shlibAtom);
- resolverState |= StateNewSharedLibraryAtoms;
- }
- for (const AbsoluteAtom *absAtom : file.absolute()) {
+
+ for (const AbsoluteAtom *absAtom : file.absolute())
doAbsoluteAtom(*absAtom);
- resolverState |= StateNewAbsoluteAtoms;
+
+ // If we make some progress on linking, notify that fact to the input file
+ // manager, because it may want to know that for --start-group/end-group.
+ progress = progress || file.sharedLibrary().size() ||
+ file.absolute().size() || file.defined().size();
+ if (progress) {
+ _context.inputGraph().notifyProgress();
}
- _context.inputGraph().setResolverState(resolverState);
}
void Resolver::forEachUndefines(UndefCallback callback,
@@ -302,7 +306,6 @@ bool Resolver::resolveUndefines() {
for (;;) {
ErrorOr<File &> file = _context.inputGraph().nextFile();
- _context.inputGraph().setResolverState(Resolver::StateNoChange);
error_code ec = file.getError();
if (ec == InputGraphError::no_more_files)
return true;
Modified: lld/trunk/unittests/DriverTests/InputGraphTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/unittests/DriverTests/InputGraphTest.cpp?rev=205463&r1=205462&r2=205463&view=diff
==============================================================================
--- lld/trunk/unittests/DriverTests/InputGraphTest.cpp (original)
+++ lld/trunk/unittests/DriverTests/InputGraphTest.cpp Wed Apr 2 16:02:44 2014
@@ -315,7 +315,7 @@ TEST_F(InputGraphTest, AddNodeWithGroupI
EXPECT_NE(InputGraphError::no_more_files, objfile.getError());
EXPECT_EQ("group_objfile2", (*objfile).path());
- group->setResolveState(Resolver::StateNewDefinedAtoms);
+ group->notifyProgress();
objfile = group->getNextFile();
EXPECT_NE(InputGraphError::no_more_files, objfile.getError());
More information about the llvm-commits
mailing list