LLD: TargetInfo constness

[Rui Ueyama]

Michael and Shankar, any thoughts?

On Sun, Jul 21, 2013 at 5:49 PM, Nick Kledzik <kledzik at apple.com> wrote:

> A couple of thoughts:
>
> 1) The name TargetInfo does not describe what this class is doing and does
> not help resolve this.  We should consider renaming it.
>
> 2) The TargetInfo object is the one "context" object passed around
> throughout the linker.   It would be a pain to pass around two objects (a
> const and non-const object).  *If* we wind up with two objects, then the
> non-const one should hold a reference to the the const one, and just have
> the non-const one passed everywhere.
>
> 3) I think the motivation for it being const is that with lld as a library
> (say for JIT'ing code) you may want to set it up once and re-use it for
> multiple links.  This presupposes that constructing and setting one up is
> expensive.
>
> So, my feeling is that we keep one object, but renaming it something like
> LinkingContext, and strive to have a way to make construction cheap.  For
> instance, have a MachOLinkingContext constructor that takes a few
> parameters and quickly initializes an object in a way useful for most
> library clients, as opposed to passing a command line list which must be
> parsed to construct the object.
>

+1. I'm in favor of renaming.


>
> -Nick
>
> On Jul 21, 2013, at 5:25 PM, Rui Ueyama wrote:
>
> Here is the background information about this topic, I'll write it down
> for convenience.
>
> *Issue*
> Some types of object files may have directives to the linker which has
> side effects. Specifically I know there are two cases:
>
>  - COFF object file
> On Windows, the object file may contain linker options in .drectve
> section, and the linker needs to handle them as if it were given via the
> command line. This feature is used mainly for propagating required library
> name from the compiler to the linker, so that you can omit /defaultlib
> option (which is mostly equivalent to Unix -l option) when linking, but
> it's a generic feature and not limited to that.
>
>  - Linker script
> Many linker script directives, such as ENTRY or SEARCH_DIR, need to update
> TargetInfo information.
>
> TargetInfo is the object having all the information needed by the core
> linker. It's constructed by the driver based on the command line (or in
> some way if used as a library), and passed to the core linker. The core
> linker links objects as instructed by a TargetInfo. The core linker treats
> a TargetInfo as a const object, so that we can reuse a TargetInfo multiple
> times.
>
> The needs to update the TargetInfo conflicts with the constness.
>
> I remember Nick suggested two pass solution. In the first pass, we read
> all the object files given via the command line only to gather linker
> directives to construct a TargetInfo. In the second pass, the TargetInfo is
> passed to the core linker and is treated as a const object. This might work
> for ELF, but wouldn't for COFF, because of the issue when linking the
> archive file. We should not parse directives of all files in a library
> file, but parse only the file needed for linking. That information is not
> available until we actually try to link objects, so we can't split it into
> two passes.
>
>
>
> On Sun, Jul 21, 2013 at 4:55 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>> I think we've now got to the point where we should revisit the topic
>> about how to handle linker directives embedded in the input file for
>> further development of the lld linker. We had a discussion in May for
>> http://llvm-reviews.chandlerc.com/D833, but at that time we didn't reach
>> a conclusion.
>>
>> It appears to me that there are two choices to handle it.
>>
>> 1. Treat TargetInfo as a mutable object
>> It's a simple solution, but we will lose reusability of TargetInfo.
>>
>> 2. Make TagetInfo clonable
>> Make a copy of a TagetInfo in the core linker and use it internally. It
>> probably needs large refactoring of TargetInfo because the class currently
>> has many non-copyable members.
>>
>> I'm inclined to choice 1, for choice 2 seems to need too much work. What
>> do you guys think?
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130723/19aab8c6/attachment.html>