There are two points.<br><br>1. The core linker already knows about ELF or other platform specific features. A few examples are a tentative symbol, a section group, or a weak external symbol. It would be very difficult to do to implement them outside of the core. <div><br></div><div>Also, what the core linker implements is the Unix linker's semantics, which is different from, for example, Windows. It's not platform neutral as a whole.<div><div><br></div><div>(We can assume a linker for an embedded system or something, whose feature is pretty limited. Say, it doesn't support an archive file and a shared library file. Now, all of sudden, archive files and shared library files become "platform dependent" feature. Should we move the code handling them out of the core? No, we don't want to do that. Why do we think the set of features that we currently have in the core is "platform neutral"? There's actually no obvious reason.)</div><div><br></div><div>And having platform dependent feature in the core linker is not immediately a bad thing, as I noted as (3) in the previous mail. What makes code more maintainable for all of us is the right thing to do, whether that's a patch to the core or a platform specific tree.</div><div><div><br></div><div>2. It's totally OK to get nothing from rewriting the InputGraph. It even shouldn't change the functionality. It's refactoring to improve code quality.</div><div><br><div class="gmail_quote">On Wed Oct 22 2014 at 6:16:29 PM Shankar Easwaran <<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">
<div>On 10/22/2014 8:01 PM, Rui Ueyama
wrote:<br>
</div>
<blockquote type="cite">
<pre>On Tue Oct 21 2014 at 9:38:57 PM Nick Kledzik <a href="mailto:kledzik@apple.com" target="_blank"><kledzik@apple.com></a> wrote:
</pre>
<blockquote type="cite">
<pre>On Oct 21, 2014, at 6:31 PM, Rui Ueyama <a href="mailto:ruiu@google.com" target="_blank"><ruiu@google.com></a> 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.
</pre>
</blockquote>
<pre>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.
</pre>
</blockquote></div><div bgcolor="#FFFFFF" text="#000000">
I dont think the Core resolver should not understand anything about
ELF. <br>
<br>
Even changing this to a std::vector<FileOrGroup> doesnot
change what can be achieved with the new functionality compared to
what we have now.<br>
<br>
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.<br>
<blockquote type="cite"></blockquote></div><div bgcolor="#FFFFFF" text="#000000"><blockquote type="cite">
<pre></pre>
<blockquote type="cite">
<pre>On Tue, Oct 21, 2014 at 5:33 PM, Shankar Easwaram <a href="mailto:shankarke@gmail.com" target="_blank"><shankarke@gmail.com></a>
wrote:
</pre>
<blockquote type="cite">
<pre>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 <a href="mailto:ruiu@google.com" target="_blank"><ruiu@google.com></a> wrote:
On Tue, Oct 21, 2014 at 3:41 PM, Michael Spencer <a href="mailto:bigcheesegs@gmail.com" target="_blank"><bigcheesegs@gmail.com></a>
wrote:
</pre>
<blockquote type="cite">
<pre>On Mon, Oct 20, 2014 at 9:21 PM, Rui Ueyama <a href="mailto:ruiu@google.com" target="_blank"><ruiu@google.com></a> wrote:
</pre>
<blockquote type="cite">
<pre>I'm not still sure whether or not "InputGraph" thing was after all the
</pre>
</blockquote>
<pre>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.
</pre>
<blockquote type="cite">
<pre>One thing we probably should consider is, in my opinion, we are
</pre>
</blockquote>
<pre>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.
</pre>
</blockquote>
<pre>To deal with the issue of the InputGraph I have a few ugly hacks in
PECOFF/<u></u>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.
</pre>
</blockquote>
<pre></pre>
</blockquote>
<pre></pre>
<br>
<fieldset></fieldset>
<br>
</blockquote></div><div bgcolor="#FFFFFF" text="#000000"><blockquote type="cite"><pre>______________________________<u></u>_________________
llvm-commits mailing list
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a>
</pre>
</blockquote></div><div bgcolor="#FFFFFF" text="#000000">
<br>
<br>
<pre cols="72">--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation</pre>
</div></blockquote></div></div></div></div></div>