This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Stream ODR types


> 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.

Honza
> 
> Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]