LLD: TargetInfo constness

[Rui Ueyama]

Any comments? Is it OK to proceed?


On Wed, Jul 24, 2013 at 2:11 PM, Rui Ueyama <ruiu at google.com> wrote:

> Bigcheese?
>
>
> On Tue, Jul 23, 2013 at 12:26 PM, Shankar Easwaran <
> shankare at codeaurora.org> wrote:
>
>>  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> <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> <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 forhttp://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 listllvm-commits at cs.uiuc.eduhttp://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/20130726/53b41bd6/attachment.html>