[PATCH] Add DarwinInputGraph

[Shankar Easwaran]

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


-- 
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/20141022/c4b8ea48/attachment.html>