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

[Peter Collingbourne via Phabricator via llvm-commits]

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:
> 
>   case LDPR_PREVAILING_DEF_IRONLY_EXP:
>     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.


https://reviews.llvm.org/D28978