> > Index: tree.c
> > ===================================================================
> > --- tree.c (revision 229968)
> > +++ tree.c (working copy)
> > @@ -13198,6 +13198,7 @@ gimple_canonical_types_compatible_p (con
> > /* If the types have been previously registered and found equal
> > they still are. */
> > if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t2)
> > + && !POINTER_TYPE_P (t1) && !POINTER_TYPE_P (t2)
>
> But TYPE_CANONICAL (t1) should be NULL_TREE for POINTER_TYPE_P?
The reason is that TYPE_CANONICAL is initialized in get_alias_set that may be
called before we finish all merging and then it is more fine grained than what
we need here (i.e. TYPE_CANONICAL of pointers to two differnt types will be
different, but here we want them to be equal so we can match:
struct aa { void *ptr;};
struct bb { int * ptr;};
Which is actually required for Fortran interoperability.
Removing this hunk triggers false type incompatibility warning in one of the
interoperability testcases I added.
Even if I drop the code bellow setting TYPE_CANOINCAL, I think I need to keep
this conditional: the types may be built in and those get TYPE_CANONICAL set as
they are constructed by build_pointer_type. I can gcc_checking_assert for this
scenario and see. Perhaps we never build LTO type from builtin type and this
won't happen. If we did, we would probably have a trouble with false negatives
in return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2); on non-pointers anyway.
>
> > && trust_type_canonical)
> > return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
> >
> > Index: alias.c
> > ===================================================================
> > --- alias.c (revision 229968)
> > +++ alias.c (working copy)
> > @@ -869,13 +874,19 @@ get_alias_set (tree t)
> > set = lang_hooks.get_alias_set (t);
> > if (set != -1)
> > return set;
> > - return 0;
> > + /* LTO frontend does not assign canonical types to pointers (which we
> > + ignore anyway) and we compute them. The following path may be
> > + probably enabled for non-LTO, too, and it may improve TBAA for
> > + pointers to types with structural equality. */
> > + if (!in_lto_p || !POINTER_TYPE_P (t))
> > + return 0;
>
> No new LTO paths please, do the suggested change immediately.
OK, I originally tested the patch without if and there was no problems.
Just chickened out before preparing final version of the patch.
> > + p = TYPE_MAIN_VARIANT (p);
> > + /* Normally all pointer types are built by
> > + build_pointer_type_for_mode which ensures they have canonical
> > + type unless they point to type with structural equality.
> > + LTO frontend produce pointer types without TYPE_CANONICAL
> > + that are then added to TYPE_POINTER_TO lists and
> > + build_pointer_type_for_mode will end up picking one for us.
> > + Declare it the canonical one. This is the same as
> > + build_pointer_type_for_mode would do. */
> > + if (!TYPE_CANONICAL (p))
> > + {
> > + TYPE_CANONICAL (p) = p;
> > + gcc_checking_assert (in_lto_p);
> > + }
> > + else
> > + gcc_checking_assert (p == TYPE_CANONICAL (p));
>
> The assert can trigger as
> build_pointer_type_for_mode builds SET_TYPE_STRUCTURAL_EQUALITY pointer
> types for SET_TYPE_STRUCTURAL_EQUALITY pointed-to types. Ah,
> looking up more context reveals
>
> if (TREE_CODE (p) == VOID_TYPE || TYPE_STRUCTURAL_EQUALITY_P (p))
> set = get_alias_set (ptr_type_node);
Yep, we don't get here.
>
> Not sure why you adjust TYPE_CANONICAL here at all either.
You are right, I may probably just drop all the code and just do:
gcc_checking_assert (!TYPE_CANONICAL || p == TYPE_CANONICAL (p));
I will test this and re-think the build_pointer_type code to be sure that we
won't get into a problem there.
As I recall, the original code
p = TYPE_CANONICAL (p);
was there to permit frontends to glob two pointers by setting same canonical
type to them. My original plan was to use this for LTO frotnend and make
gimple_compare_canonical_types to do the right thing for pointers and this would
follow gimple_compare_canonical_types globbing then.
This idea was wrong: since pointer rules are not transitive (i.e. void
* alias them all), we can't model that by an equivalence produced by
gimple_compare_canonical_types.
Since the assert does not trigger, seems no frontend is doing that and moreover
I do not see how that would be useful (well, perhaps for some kind of internal
bookeeping when build TYPE_CANONICAL of more complex types from pointer types,
like arrays, but for those we ignore TYPE_CANONICAL anyway). Grepping over
TYPE_CANONICAL sets in frotneds, I see no code that I would suspect from doing
something like this.
Thank you!
Honza
>
> Otherwise looks ok.
>
> RIchard.
>
>
> > }
> > - gcc_checking_assert (TYPE_CANONICAL (p) == p);
> >
> > /* Assign the alias set to both p and t.
> > We can not call get_alias_set (p) here as that would trigger
> > @@ -1015,11 +1043,6 @@ get_alias_set (tree t)
> > }
> > }
> > }
> > - /* In LTO the rules above needs to be part of canonical type machinery.
> > - For now just punt. */
> > - else if (POINTER_TYPE_P (t)
> > - && t != TYPE_CANONICAL (ptr_type_node) && in_lto_p)
> > - set = get_alias_set (TYPE_CANONICAL (ptr_type_node));
> >
> > /* Otherwise make a new alias set for this type. */
> > else
> >
> >
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
> 21284 (AG Nuernberg)
