[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#661140, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28978#661111, @pcc wrote:
>
> > In https://reviews.llvm.org/D28978#661105, @mehdi_amini wrote:
> >
> > > In https://reviews.llvm.org/D28978#661102, @pcc wrote:
> > >
> > > > > 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.
> > >
> > >
> > > You're right, we can't do any optimization or any better codegen based on this information.
> > >
> > > I still think we should refine the partitioning detection, since only the first two cases would be merged.
> > >
> > >   External 
> > >         | > Outside of LTO
> > >         | > PartitionLTO
> > >               | > RegularLTO
> > >               | > PartitionThinLTO
> > >                     | > Individual ThinLTO Module
> > >
> > >
> > > What is introduced that we don't have today is a `PartitionLTO` and a `PartitionThinLTO`. This avoids going up to External immediately when a symbol is used in multiples individual ThinLTO modules, or when it is used across Regular and ThinLTO.
> > >
> > > Right new we have to reconstruct this information in ThinLTO because we're losing it when constructing the partitioning.
> >
> >
> > I'm not sure I understand how `PartitionLTO` would be different from `!IsVisibleToRegularObj`, or `PartitionThinLTO` from `Partition == External && !VisibleOutsideThinLTO`.
>
>
> It is not different, that's the point: VisibleOutsideThinLTO was added after the fact because the partition were losing the information. I think it was not a good addition and fixing the partitions instead was the way to go,  `Partition == External && !VisibleOutsideThinLTO` is not something I'd like to read anywhere.


Agreed that we should rethink partitions, it would be nice to have a better way to write that query.


https://reviews.llvm.org/D28978