On Thu, 11 Sep 2014, Jan Hubicka wrote:

> > On Thu, 11 Sep 2014, Jan Hubicka wrote:
> > 
> > > Hi,
> > > this patch adds computation and streaming of mangled type names.  As 
> > > suggested by Jason,
> > > it simple calls DECL_ASSEMBLER_NAME on all names types and lets C++ 
> > > supply them.
> > > This makes it possible to stablish precise ODR type equivalency at LTO 
> > > (till now we can
> > > do that only for complete class types with virtual methods attached to 
> > > them).
> > > Lto type merging is then updated to register all types into the ODR type 
> > > hash.  This
> > > makes warnings to be output for ODR violations. Here are ones output for 
> > > Firefox:
> > > http://kam.mff.cuni.cz/~hubicka/odr-warnings-firefox.txt
> > > 
> > > As discussed earlier, in addition to ODR warnings that seems useful, I 
> > > would
> > > like to use it for TBAA analysis for ODR types that are not structurally
> > > equivalent to non-ODR types, so C++ programs will get better alias 
> > > analysis and
> > > for other tricks, such as more agresively merging ODR types.
> > > 
> > > I believe this makes sense (is orthogonal) with early debug info (for 
> > > warnings, TBAA
> > > and devirtualization).  It can be also used to more agresively merge 
> > > debug information
> > > as done by LLVM.
> > > 
> > > The change increase LTO object fules by about 2% (uncompressed by 6%) and 
> > > also
> > > increase WPA memory use and streaming times by about same percentage.  It 
> > > is
> > > not small and thus I made it optional (enabled by default for now).  We 
> > > could see
> > > how benefits relate to this cost once the other three parts are 
> > > implemented.
> > > 
> > > Bootstrapped/regtested x86_64-linux, seems sane?
> > 
> > It looks sane, but when early debug is completed we likely will drop
> > all the elaborated types from decls.  Thus to keep the ODR type you'd
> > have to keep (and compute early as well) their DECL_ASSEMBLER_NAME?
> 
> I currently compute it in free_lang_data.  Obviously we can compute earlier
> (in the frontend) as fit.
> > 
> > Can't we just store a hash of the assembler name?  From alias analysis
> > perspective false aliasing due to a hash collision is harmless, no?
> > Maybe not for ODR warnings though.  At least a hash would be way
> > cheaper than those usually very large strings....
> 
> Hmm, interesting idea.  False positives are harmless for alias analysis, they
> do matter for type inheritance graph construction but if we decide we will 
> ever
> care only about polymorphic types, we can always use the virtual table name to
> resolve conflicts.
> 
> We will get false ODR violation warnings, but the chances would be very low.
> > 
> > You probably want to restrict ODR types to aggregates?
> 
> For ODR warnings and TBAA I think i want other types, too.  But yep, we need 
> to handle
> gracefuly component types that does not have names and we could drop names of 
> types
> and handle them as component types as it seems fit.
> 
> OK, so if you agree, I will go ahead with this patch and we can resolve these 
> details
> incrementally.

Yes, but please disable !record type handing for now.

Richard.

Reply via email to