[PATCH] [lld][inputGraph] Implement flavor specific resolver

Shankar Kalpathi Easwaran shankarke at gmail.com
Fri Oct 4 19:13:00 PDT 2013


  A sub-class of ELFFileNode parse can detect that a new file needs to be added, and parse an additional file as part of parsing it. This is because the command line specified only this file.


================
Comment at: include/lld/Core/Error.h:76
@@ +75,3 @@
+
+struct inputGraph_error {
+  enum _ {
----------------
Michael Spencer wrote:
> input_graph_error to match existing. We should probably switch them all over to LLVM style later.
Will change.

================
Comment at: include/lld/Core/LinkingContext.h:314
@@ +313,3 @@
+  /// atoms.
+  virtual void setResolverState(int32_t resolverState) const;
+
----------------
Michael Spencer wrote:
> Shankar Kalpathi Easwaran wrote:
> > kledzik at apple.com wrote:
> > > You can eliminate the ResolveState enum and all the bit values by just making this method be:
> > >   void setResolverState(bool addedDefines, bool addedUndefs, bool addedShared, bool addedAbsolute);
> > > though I am not sure why anything needs to now what kind of atoms was added.
> > > 
> > I want to keep the method as is, as its convenient to add new types of atoms that are found.
> > 
> > The reason I thought it might be useful, was 
> > 
> > a) Depending on if weak atoms were found in the process, and if the linker flag dictated to resolve weak atoms within the group.
> > b) To derive stats of how many atoms were added/resolved from the inputelement (the weight function)
> This seems really complicated. Isn't the main purpose of this to know if the resolver added anything? Why not just an argument to nextFile?
The original intention is to know if the resolver added anything. This is only a secondary intention. This was discussed on the llvm dev list and we thought it would be more flexible to add a function to set the resolver state.

http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-September/065774.html

================
Comment at: include/lld/Core/LinkingContext.h:363-364
@@ -357,3 +362,4 @@
   std::unique_ptr<InputGraph> _inputGraph;
-  llvm::BumpPtrAllocator _allocator;
+  mutable llvm::BumpPtrAllocator _allocator;
+  mutable InputElement *_currentInputElement;
 
----------------
Michael Spencer wrote:
> Should we go ahead and make LinkingContext non-const first?
I will make this non const in the next patch.

================
Comment at: include/lld/Core/LinkingContext.h:360
@@ -355,2 +359,3 @@
   std::unique_ptr<Reader> _yamlReader;
+  std::unique_ptr<Reader> _lldNativeReader;
   StringRefVector _initialUndefinedSymbols;
----------------
Michael Spencer wrote:
> _nativeReader?
Will change.

================
Comment at: include/lld/Core/Resolver.h:31-37
@@ -31,5 +30,9 @@
 public:
-  Resolver(const LinkingContext &context, const InputFiles &inputs)
-      : _context(context), _inputFiles(inputs), _symbolTable(context),
-        _result(context), _haveLLVMObjs(false), _addToFinalSection(false),
-        _completedInitialObjectFiles(false) {}
+  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
+  };
+
----------------
Michael Spencer wrote:
> Not llvm style.
Will change.

================
Comment at: include/lld/Driver/InputGraph.h:247
@@ -212,1 +246,3 @@
+/// directly.
+///
 class FileNode : public InputElement {
----------------
Michael Spencer wrote:
> Remove extra empty line.
Ok.

================
Comment at: lib/Driver/InputGraph.cpp:63-64
@@ +62,4 @@
+    pos = _inputArgs.size();
+  _inputArgs.resize(_inputArgs.size() +
+                    std::distance(inputElements.begin(), inputElements.end()));
+  _inputArgs.insert(_inputArgs.begin() + pos,
----------------
Michael Spencer wrote:
> This is unneeded. insert will do the reserve.
ok.

================
Comment at: test/elf/X86_64/dynlib-search.test:2
@@ -1,3 +1,3 @@
 # This tests the functionality for finding the shared library libfn.so for ELF
-RUN: lld -flavor gnu -target x86_64 %p/Inputs/main.o -L%p/Inputs/ -lfn -o %t \
+RUN: lld -flavor gnu -target x86_64 -L%p/Inputs/ -lfn -o %t \
 RUN: --noinhibit-exec -t > %t1
----------------
Michael Spencer wrote:
> Why this change?
There is a bug with llvm raw_ostream with buffered outputs. I will re-add the change (If the current buffer is not freed and the function is called before its written, it will assert).

================
Comment at: test/Driver/undef-basic.objtxt:22
@@ -21,2 +21,1 @@
 # CHECK:   - name:            undefinedsymbol
-# CHECK:     can-be-null:     at-buildtime
----------------
Michael Spencer wrote:
> Why was this removed?
Ah an accidental removed change. Thanks for catching this



http://llvm-reviews.chandlerc.com/D1830



More information about the llvm-commits mailing list