> On 08/19/2013 10:01 AM, Jan Hubicka wrote: > >+ /* All equivalent types, if more than one. */ > >+ vec<tree, va_gc> *types; > >+ /* Set of all equivalent types, if NON-NULL. */ > >+ pointer_set_t * GTY((skip)) types_set; > > Why do you need both a vector and a pointer set? Can't you drop the > vector and use pointer_set_traverse for operating on each of the > elements?
I was mostly worried about ordering issues. pointer_set is not stable across memory layout changes and we may end up outputting warnings/merging binfos in random orders. This seemed uncool. Maybe the final form is quite safe, since we do everything with the designated leader, but I would rather keep this here rather than later go into nasty issues of divergences in between compilation on different hosts. It is quite standard way to get list with unique elements for LTO - see implementation of the encoders/cgraph sets/varpool sets. Perhaps this asks for suitable C++ template with more sane representation. Note that for Firefox I get about 717 types have duplicates, 387 have two duplicates (that I can explain by external/internal vtables) 260 have 3 duplicates and the most duplicated type is duplicated 171 times. So memory usage of the code above is not critical in any way. Still we have a lot more duplicates than I would like. I plan to dig into this later, or perhaps convince Richard. > > >+ "type %qD violate one definition rule ", > > "violates" > > >+ "the type of same name with different memory layout " > >+ "defined in other compilation unit"); > > Is there any way to identify which TUs the definitions come from? Not that I know of. We can walk to vtable associated by the type that can give me object file (often temporary name for .a archives) but that is after declaration merging, so it will not point to the original file anymore. Actually thinking of this deeper, perhaps we can walk out into tarnslation unit decl. We explicitely allow merging cross translation unit mergin for file scope types, so this probably has chance to work only if type in question is file scope or its parent is duplicated too. We can also maybe walk state infos for first appeareance of the type (it is the unit it came from) but that is expensive and we will need to ensure early construction of the graph before global states are thrown away. I will leave this for incremental update and discuss it with Richard. I basically added the comment about other unit to make the message less confusing: when you manage to get difference because some earlier type changed, you get error pointing out the precisely same location twice. > BTW, if anything asks for the DECL_ASSEMBLER_NAME of the TYPE_NAME, > it will get an ODR-unique mangled name for the type. But we don't > currently set that during normal compilation because it isn't used > in mangling of actual symbols. I see, I wondered if I can get mangling of type names. So, once we will need uniqueness of ODR types for C++ (probably for alias info), we can basically convince C++ FE to produce the mangling and then discover and match ODR types based on presence of the assembler names? Can't this be used for merging of dwarf debug info on types? Thank you! Honza > > Jason
