[PATCH] Add DarwinInputGraph

[Shankar Easwaram]

Nick,

Your idea sounds good. I have only one issue that when a file is parsed it needs an access to the input element. Am I missing something this detail in this change in design ?

Shankar Easwaran 



> On Oct 21, 2014, at 23:38, 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.
> 
> 
> -Nick
> 
> 
>> 
>>> 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.
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141022/feabcf0b/attachment.html>