[PATCH] D19351: ELF: Add initial ThinLTO support.

[Teresa Johnson via llvm-commits]

On Thu, May 12, 2016 at 6:29 PM, Peter Collingbourne <peter at pcc.me.uk>
wrote:

> Thanks Rafael.
>
> On Thu, May 12, 2016 at 4:32 PM, Rafael Espíndola <
> rafael.espindola at gmail.com> wrote:
>
>> On 12 May 2016 at 16:13, Peter Collingbourne <peter at pcc.me.uk> wrote:
>> > pcc added a comment.
>> >
>> > I have updated the patch. I just want to make sure that people are
>> happy with the situation around the ThinLTO API. I've been working on an
>> LTO/ThinLTO API that could be used by both the gold plugin and lld which
>> should address Mehdi's concerns, but it will take some time until that's
>> ready.
>> >
>>
>> Sorry I dropped out of this thread.
>>
>> I don't known a lot about ThinLTO, so most comments will be generic about
>> LTO.
>>
>> The way the existing lib/LTO is implemented is fundamentally different
>> from what we do in the gold plugin or lld. We want to make sure we
>> don't even read and copy symbols if the linker decided to not keep it.
>> The lib/LTO interface is designed to talk about symbols in the merged
>> module.
>>
>
> Right, I certainly don't want to use any of the existing lib/LTO anywhere
> we're not using it now. The new API I'm working on deals with symbols in
> individual modules.
>
> Having said that, I dislike duplicated code as much as anyone else.
>> That is why I really don't want lld calling into Linker::linkModules:
>> it duplicates symbol resolution. The same goes for the call to
>> addPassesToEmitFile. We really should figure out a single place to
>> populate the pipeline.
>>
>> If it is possible for lld/gold/LTO to share more code, excellent but:
>>
>> * I would probably not put that in lib/LTO. Lets keep that just for
>> the existing api.
>>
>
> Putting the new API in some other lib makes sense to me.
>
>
>> * It still seems a bit early for this. We still having more basic
>> things to finish in lld and the thinLTO api is still moving.
>>
>
> Regarding the new API, I think there’s a secondary reason why we might
> want to introduce it sooner rather than later. That reason is that we plan
> to store symbol resolutions in a combined module index, which basically
> describes which module contains the prevailing definition of each function.
> Getting this right is basically a critical correctness issue, as we could
> otherwise for example import the definition of a weak symbol instead of the
> prevailing strong symbol. I also understand that Teresa plans to use this
> information in distributed backends.
>
> One way we could implement this could be to expose gold's symbol
> resolutions to the ThinLTO importer in some form. The problem with this is
> that it adds more complexity in a gold-specific layer, when we know that we
> will eventually need to deal with that in every linker. Or we could
> implement symbol resolution ourselves in the importer via the linkage info
> in the summaries, but as I’m sure you are aware, this would be duplicating
> logic from the linker.
>

Note that this is the current approach taken by libLTO for ODR resolution
in ThinLTO.

This intersects with what I have been working on today towards the goal of
function importing via the gold-plugin (and lld) using the IRMover directly
instead of the ModuleLinker, which it currently relies on for lazy
importing of linkonce. That requires comdat summary info (specifically for
supporting COFF size based comdat selection types). I was then going to
refactor and extend the linkonce -> weak resolution being done in
ThinLTOCodeGenerator into the function importer.

If we will get the symbol resolution from the linker (via the index for
distributed backends) then we presumably don't need the comdat summary
info, unless I am missing something (the linker should take care of marking
all comdat members as prevailing I think). If this is the direction then I
should put that on hold. The linkonce->weak resolution still needs to be
refactored out of ThinLTOCodeGenerator but it would be based on the
resolution info rather than the summary linkage types.

That’s why I feel it’s best to have the importer deal with linker-agnostic
> symbol resolution info.
>
> Getting this right in the gold plugin is basically orthogonal to whether
> we implement anything in lld. So, the course of action I was considering was
> 1) Get the new interface in place, and port the gold plugin to it;
> 2) (in parallel with 1) Polish regular LTO in lld as necessary;
> 3) Port lld to the new interface.
> 4) (low priority for me, but maybe Mehdi can help) Port libLTO to the new
> interface.
>
> Thanks,
> --
> --
> Peter
>



-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160512/62b3afdb/attachment.html>