LLD: TargetInfo constness

[Shankar Easwaran]

Hi Rui,

Yes, I am fine with it. The derived classes of TargetInfo should also 
match their names.

ELFTargetInfo has multiple TargetInfo's that derive from it 
(PPCTargetInfo, X86_64TargetInfo, HexagonTargetInfo).

Thanks

Shankar Easwaran

On 7/23/2013 12:57 PM, Rui Ueyama wrote:
> 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?
>>>
>>
>>
>
>
> _______________________________________________
> 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/20130723/886c9420/attachment.html>