[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:
> case LDPR_PREVAILING_DEF_IRONLY_EXP:
> R.Prevailing = true;
> if (!Res.CanOmitFromDynSym)
> R.VisibleToRegularObj = true;
> 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:
> | > 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