Re: PATCH: new linker option for shared library COMDAT optimization

[Sandra Loosemore <sandra at codesourcery dot com>]

On 02/29/2012 04:28 PM, Alan Modra wrote:
> On Wed, Feb 29, 2012 at 11:23:14AM -0700, Sandra Loosemore wrote:
> > Would it suffice for the purposes of the current patch to add a bit
> > to the documentation for --shared-comdat explaining that it requires
> > that the comdat symbols have default (exported) visibility in the
> > shared library, and that this may require changes to how the library
> > is built?
>
> [snip]
>
> > Perhaps as a
> > less-strict version of what I have now, I could require that the
> > shared library provide definitions for only the comdat symbols that
> > are referenced, instead of all of them?
>
> Yes, I think we could go ahead with these two changes.

Last night I was looking at implementing the changes for relaxing the 
requirement to match referenced symbols only, and I started thinking 
that this isn't a good idea after all, for a whole bunch of reasons.

* The ref_regular and ref_dynamic bits on the symbol don't distinguish 
between references within the group (or within another group that is a 
candidate for removal), and "real" references.  So, without some more 
sophisticated form of symbol GC, this isn't likely to produce results 
that are transparent either to users or library implementors.

* Consider the degenerate case when none of the symbols defined in a 
group are referenced.  Throwing the whole group away seems like the 
wrong thing -- e.g., consider that you're building a shared library that 
specifically wants to provide and export some template instantiations to 
its consumers, and is itself built with --shared-comdat.

* In the original version of the patch I posted previously, I'd left 
some debugging hooks in place.  One of them printed a message if it 
found a partial match -- some of the symbols defined in a comdat group 
were provided by a shared library, but not all of them.  In practice, 
when I had the debugging enabled I never once saw that condition 
triggered in all the testing I did, so I suspect this is really a corner 
case and not critical functionality.

The more I think about it, allowing for a library to provide only a 
subset of symbols in a group just seems like a Bad Idea, and in 
violation of the spirit that groups are supposed to be manipulated as a 
unit.  So I'd like to drop this suggestion, and just go with the 
documentation change to my original patch instead.  OK?

-Sandra