> > +  if (RECORD_OR_UNION_TYPE_P (t)
> > +      && odr_type_p (t) && !odr_type_violation_reported_p (t))
> > +    {
> > +      /* Here we rely on fact that all non-ODR types was inserted into
> > +    canonical type hash and thus we can safely detect conflicts between
> > +    ODR types and interoperable non-ODR types.  */
> > +      gcc_checking_assert (type_streaming_finished
> > +                      && TYPE_MAIN_VARIANT (t) == t);
> > +      slot = htab_find_slot_with_hash (gimple_canonical_types, t, hash,
> > +                                  NO_INSERT);
> > +      if (slot && !TYPE_CXX_ODR_P (*(tree *)slot))
> > +   {
> > +     tree nonodr = *(tree *)slot;
> > +     if (symtab->dump_file)
> > +       {
> > +         char *name = cplus_demangle_v3
> > +                            (IDENTIFIER_POINTER
> > +                              (DECL_ASSEMBLER_NAME (TYPE_NAME (t))), 0);
> 
> Ugh - do we really want to demangle here?  I think not.  Surely
> not without -details or with -slim.  Readers can c++filt this
> easily.

OK, i can dump mangled name. 
Just print_generic_expr prints all instantiations of the tempalte same
way which makes it look odd that there are so many types of same name.
> > +     /* Set canonical for T and all other ODR equivalent duplicates
> > +        including incomplete structures.  */
> > +     set_type_canonical_for_odr_type (t, prevail);
> > +     enable_odr_based_tbaa (t);
> 
> I suppose there never will be a set of ODR types with the same
> prevailing type but some of them having a conflict with a nonodr
> type and some not?

No, we check them by structural equality while verifying ODR violations
that is more strict than gimple_canonical_types_compatible.
> 
> > +     if (!type_in_anonymous_namespace_p (t))
> > +       hash = htab_hash_string (IDENTIFIER_POINTER
> > +                                      (DECL_ASSEMBLER_NAME
> > +                                              (TYPE_NAME (prevail))));
> > +     else
> > +       hash = TYPE_UID (TYPE_MAIN_VARIANT (t));
> > +     num_canonical_type_hash_entries++;
> > +     bool existed_p = canonical_type_hash_cache->put (prevail, hash);
> 
> but those hash differently?  I think you wanted to put t, not prevail
> here.  And you want to use the TYPE_UID of prevail as well?

Actually this was intended to be prevail in both cases, but it is
harmless.  The difference here is that anonymous types have
DECL_ASSEMBLED_NAME <anon>, so if we inserted them to the hash
table based on names they will all conflict.

Anonymous namespace types never have duplicated main variants, so
t==prevail here, but I will update the patch to use prevail since it is
much more obvious (I am also not sure what I was thinking of when
I typed TYPE_MAIN_VARIANT)
> 
> Otherwise looks good.
> 
> You can commit the C++ FE change with the adjustment in case it
> fixes the reported verification ICEs.

It only fixes ICE WRT the sanity checking that all non-ODR types
are inserted first.  W/o that change the as-base type will get inserted
during streaming in and recursively we will insert ODR types early.

I am waiting for testcase WRT the other ODR ICE reported today. I think
tree.c when creating type variants wants to copy the flag: the wrong
type is va_list which is likely created by a backend bypassing the C++
hook.

I will re-test with these changes.

Honza

Reply via email to