[PATCH] D28978: [ThinLTO] Add an auto-hide feature

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 30 15:46:07 PST 2017

pcc added a comment.

In https://reviews.llvm.org/D28978#661021, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28978#660963, @pcc wrote:
> > Does this need to be in the summary?
> The part that is  in the summary is not set in the compiler phase, but after the think-link, in order to communicate the *result* of the auto-hide analysis to the parallel backends.

Oh I see. So you're communicating `VisibleToRegularObj` and information about the partitioning to the backends. Seems reasonable to me.

>>   I would have thought that the linker can use `canBeOmittedFromSymbolTable()` to implement its own auto-hiding logic. That is what ELF lld currently does.
> That's kind of what this patch is doing :)
> Especially we're using `Res.VisibleToRegularObj` as an input information, and this is set in gold for example there:
>     R.Prevailing = true;
>     if (!Res.CanOmitFromDynSym)
>       R.VisibleToRegularObj = true;
>     break;
> And the `Res.CanOmitFromDynSym` is set this way: `Res.CanOmitFromDynSym &= Sym.canBeOmittedFromSymbolTable();`.
> I haven't checked Gold, but I think you get the point: this is only using the symbol resolution API and no "side-information" from the summary.
> That said, I'm not totally happy by the resolution API right now, and especially how we derived the partition. We don't have all the information we want I believe.
>  I discussed it this morning with Teresa and what I think my current frustration is with the lack of precise enough lattice in the partitions resolution. I think we should be able to get this:
>   External 
>         | > Regular Object
>         | > Outside of the DSO
>         | > PartitionLTO
>               | > RegularLTO
>               | > PartitionThinLTO
>                     | > Individual ThinLTO Module
> To get this, I believe right now we need one mode field in the symbol resolution API (example: `unsigned VisibleOutsideDSO : 1`) to distinguish  between the two first cases,

As we discussed on IRC, I don't think that's necessary. The linker can resolve this and decide on its own whether to add the symbol to the dynamic symbol table.


More information about the llvm-commits mailing list