LLD: TargetInfo constness

Fri Jul 26 10:17:20 PDT 2013

OK I will proceed. I will probably start with using const_cast at locations
where I need to update TargetInfo, to see how (simulated) mutable
TargetInfo works, then remove "const" from the core linker. Renaming the
class would be the step after that.

On Fri, Jul 26, 2013 at 9:59 AM, Chandler Carruth <chandlerc at google.com>wrote:

> On Fri, Jul 26, 2013 at 9:53 AM, Rui Ueyama <ruiu at google.com> wrote:
>
>> Any comments? Is it OK to proceed?
>>
>
> If you, Nick, and Shankar are all OK with it, I would proceed. We have
> version control and so we can always fix things if Michael shows up and has
> some concerns.
>
>
>>
>>
>> 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
>>>>
>>>>
>>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130726/2024a744/attachment.html>