Simplify InputGraph and other enhancements ...

[Shankar Easwaran]

Hi,

At the LLVM developers conference, We (Myself/Bigcheese) were discussing 
one of the ways to cleanup the InputGraph and the different kinds of 
filenodes that lld has internally.

lld::FileLocator
-----------------
Motivation: In the current design, the resolver deals with files that 
are already in parsed state, but there is a need that several files may 
need to be read after a native object file is read(COFF/ELF on PS4).

lld would have a FileLocator with the below interface :-

class FileLocator {

enum Kind {
     InputPath,
     LibraryPath // The user has specified the path using -l switches.
     Memory // File is in memory
}

// Kind = MemoryBuffer
explicit FileLocator(std::unique_ptr<MemoryBuffer>);

// Kind = InputPath
explicit FileLocator(StringRef path);


private:
     std::unique_ptr<MemoryBuffer> _buffer;
     StringRef _path;
};

Simplify InputGraph
--------------------
Motivation: Discussion on InputGraph in the lld mailing list, that the 
current InputGraph interface is complicated.

Tags
^^^^^
The InputGraph can only be simplified when the Resolver would be able to 
handle flavor specific tags. All the positional parameters the driver 
would handle would have a tag associated with the FileLocator. Examples 
of tags are which affects the Resolver (or) which affects the way an 
input file is handled.

Examples of tags for the Gnu Flavor are below :-

enum class Tag {
     StartGroup,
     EndGroup,
     PreferSharedLibrary,
     PrefererStaticLibrary
};

InputElement
^^^^^^^^^^^^^^
The InputElement class will be simplified which would contain a tag and 
the FileLocator.

struct InputElement {
     Tag _tag;
     FileLocator _locator;
};

LinkingContext
^^^^^^^^^^^^^^^^
LinkingContext would have a vector of InputElements, which the driver 
would process and parse.

The Driver would create InputElements and add to the vector of 
InputElements in the LinkingContext.

FileManager
-------------
All files would be accessed using a FileManager interface that is an 
interface with the FileLocator and handles conversion of a Input file to 
a MemoryBuffer.

A new class that interfaces with the Driver will need to be created for 
searching input files depending on the user search path provided to lld.

class DriverInterface {
     std::string getAbsolutePath(FileLocator &loc) const = 0;
};

class FileManager {
     explicit FileManager(DriverInterface &d);
     MemoryBuffer &getBuffer(FileLocator &loc);
     const DriverInterface& _interface;
};

Resolver
---------
The Resolver processes InputElements and the tags relevant for the resolver.

Currently start/end group tags are the only ones I think are needed to 
be supported by the resolver.

Since the resolver processes InputElements which may not be parsed, the 
Resolver would access the registry to convert the InputElement to a 
parsed File representation.

lld::File
----------
class File {
     range<InputElement> getInputs();
}

Every lld::File will return a range of InputElements that may need to be 
additionally parsed when a file is read.

Please raise your opinions and thoughts on the design.

Shankar Easwaran



On 10/22/2014 8:16 PM, Shankar Easwaran wrote:
> On 10/22/2014 8:01 PM, Rui Ueyama wrote:
>> On Tue Oct 21 2014 at 9:38:57 PM Nick Kledzik <kledzik at apple.com> wrote:
>>
>>> On Oct 21, 2014, at 6:31 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>
>>> Let's not add a back reference to the input element. It will make the
>>> complicated data structure even worse and would raise new issues (for
>>> example, I could imagine that it's not clear whether the back reference
>>> should point to a parsed file in an archive file or should point to the
>>> entire archive file.)
>>>
>>> Before discussing possible alternatives, it's worth discussing the 
>>> issues
>>> that the current InputGraph has. Here's my take.
>>>
>>> 1. We have too many classes to represent the concept of "file".
>>>
>>> FileNode in the InputGraph is different from the File class that
>>> represents parsed file. And the InputGraph has various kinds of 
>>> FileNodes
>>> -- FileNode, SimpleFileNode, <flavor>FileNode, etc. That needless
>>> distinction makes hard to handle files.
>>>
>>> FileNode needs to be merged with the File class. Or if we are going to
>>> replace the InputGraph with a simpler data structure, it should be 
>>> removed.
>>>
>>> 2. We have too many (other) classes.
>>>
>>> We have too many classes for the InputGraph that are simply 
>>> overdesigned.
>>> Command line argument is not an easy stuff, but it shouldn't need that
>>> amount of code and number of classes. We have created too deep class
>>> hierarchy there.
>>>
>>> 3. Making the core linker platform neutral is not an ultimate goal, but
>>> the InputGraph is designed for that.
>>>
>>> The InputGraph made made the core linker more or less platform neutral.
>>> That's basically a good thing. We'd like to keep the core part as 
>>> platform
>>> neutral as possible because it would help us maintain the code. 
>>> However,
>>> neutrality is not a goal that we need to achieve at any cost. If 
>>> separating
>>> platform neutral code from dependent code will mess up the entire code
>>> base, we simply shouldn't do that. (It's worth noting that the 
>>> InputGraph
>>> failed to hide everything already, one example is the ELF section 
>>> group.)
>>>
>>> We should teach the core linker a little bit more about platform 
>>> specific
>>> features and removes complexity from the InputGraph.
>>>
>>> I agree on all the above.
>>>
>>> Looking at how core linking uses InputGraph, it is always through the
>>> LinkingContext and:
>>>    Resolver.cpp needs an iterator (getNextFile()) and a way to 
>>> notify when
>>> it used a file (notifyProgress())
>>>    Driver.cpp needs a way to iterate files (inputElements) to parse 
>>> them in
>>> parallel
>>>
>>> So this could be simplified to hide/implement the “graph” in the
>>> LinkingContext.  That is, the LinkingContext subclasses each use the 
>>> data
>>> structures natural for their needs.  The drivers just call addFile() 
>>> on the
>>> LinkingContext, and the gnu driver calls startGroup()/endGroup() on the
>>> LinkingContext.  The Resolver could call a forEachFile() method on
>>> LinkingContext which takes a lambda on what to do with each file.  The
>>> lambda can return if the Resolver used the file (to track whether the
>>> current group should be scanned again). The parallel parsing 
>>> currently done
>>> in the Driver could be a utility method in the base LinkingContext 
>>> class
>>> that each concrete LinkingContext uses.
>>>
>> Why does the GNU ld driver have to ask the LinkingContext to construct a
>> startGroup/endGroup? The GNU driver should be capable of 
>> understanding all
>> the command line options for GNU ld, so it can construct a data 
>> structure
>> directly.
>>
>> forEachFile() doesn't seem different from what we have now. It's a
>> getNextFile(). I don't think it will improve the situation.
>>
>> It feels to me that we don't need a sophisticated technique here. I'd
>> suggest making the Resolver to handle ELF groups to eliminate
>> notifyProgress. I'd even remove InputGraph entirely and replace it 
>> with a
>> std::vector<FileOrGroup> or something like that where FileOrGroup is a
>> class representing a file or a ELF group.
> I dont think the Core resolver should not understand anything about ELF.
>
> Even changing this to a std::vector<FileOrGroup> doesnot change what 
> can be achieved with the new functionality compared to what we have now.
>
> As this is one of the major things that we need to handle, I think We 
> could move this discusssion to a hallway discussion during the LLVM 
> developers meeting, if you are all ok.
>>
>>>
>>> On Tue, Oct 21, 2014 at 5:33 PM, Shankar Easwaram <shankarke at gmail.com>
>>> wrote:
>>>
>>>> If we could have lld::file have a back reference to the input element
>>>> (or) use the file ordinal to access the input element in the input 
>>>> graph
>>>> you could just add inputs on a need basis ?
>>>>
>>>> Let's discuss alternative proposals as part of this thread ?
>>>>
>>>> I am also interested in moving the input file which could be a linker
>>>> script from the driver to the registry too in addition.
>>>>
>>>> We still need to have the getNextFile as it made the resolver more 
>>>> flavor
>>>> neutral.
>>>>
>>>> Shankar easwaran
>>>>
>>>>
>>>>
>>>> On Oct 21, 2014, at 17:58, Rui Ueyama <ruiu at google.com> wrote:
>>>>
>>>> On Tue, Oct 21, 2014 at 3:41 PM, Michael Spencer 
>>>> <bigcheesegs at gmail.com>
>>>> wrote:
>>>>
>>>>> On Mon, Oct 20, 2014 at 9:21 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>>>> I'm not still sure whether or not "InputGraph" thing was after 
>>>>>> all the
>>>>> right way to abstract the input file list. That often makes easy 
>>>>> things
>>>>> hard. In this case, sorting a list of input files became that 
>>>>> hard. When I
>>>>> proposed the idea I was thinking that would make this kind of 
>>>>> things easier
>>>>> to handle. I don't think the current shape of the API is not 
>>>>> desirable at
>>>>> least. Or the fundamental idea was not very good.
>>>>>> One thing we probably should consider is, in my opinion, we are
>>>>> pushing too hard to separate all ports. We are trying to write any
>>>>> architecture-dependent code into architecture-specific file. But the
>>>>> natural border of API doesn't always fit to the architecture
>>>>> dependent/independent border. We should probably relax that 
>>>>> constraint a
>>>>> bit where that makes sense and write code that's short and easier to
>>>>> understand.
>>>>>
>>>>> I agree. The current InputGraph code makes modifying the input very
>>>>> difficult to do. Part of the reason for the model in the first place
>>>>> was to make this easy. I'm currently working on adding #pragma lib
>>>>> like support for ELF (PS4 specific stuff) and have found it basically
>>>>> impossible to add a new input file in the correct position in the
>>>>> graph with the current model.
>>>>>
>>>> To deal with the issue of the InputGraph I have a few ugly hacks in
>>>> PECOFF/LinkerGeneratedSymbolFile.h. There are a few virtual .a 
>>>> files there
>>>> that generate magical symbols on the fly to control the core linker 
>>>> in an
>>>> obscure way. These hacks need to be removed, but because the 
>>>> InputFile API
>>>> exists in between, it's impossible to do.
>>>>
>>>> The InputGraph API design needs to be revisited and probably be 
>>>> rewritten.
>>>>
>>>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141030/0f783d9c/attachment.html>