On Fri, 22 May 2015, Jan Hubicka wrote:
> > > Index: emit-rtl.c
> > > ===================================================================
> > > --- emit-rtl.c (revision 223508)
> > > +++ emit-rtl.c (working copy)
> > > @@ -1787,8 +1787,15 @@ set_mem_attributes_minus_bitpos (rtx ref
> > > memset (&attrs, 0, sizeof (attrs));
> > >
> > > /* Get the alias set from the expression or type (perhaps using a
> > > - front-end routine) and use it. */
> > > - attrs.alias = get_alias_set (t);
> > > + front-end routine) and use it.
> > > +
> > > + We may be called to produce MEM RTX for variable of incomplete type.
> > > + This MEM RTX will only be used to produce address of a vairable, so
> > > + we do not need to compute alias set. */
> >
> > But why do we set mem_attributes for it then?
>
> Because we are stupid. We want to produce &var, where var is of incomplete
> type
> and to do it, we need to compute DECL_RTL of var and that is MEM RTX and we
> assign
> attributes to every DECL_RTL MEM we produce. I do not see how to cut this
> down
> except for having something like DECL_RTL_ADDR and have way to build it
> without
> actually making MEM expr... (which would save some memory for the MEM RTX and
> for
> attributes but it would make it difficult to hook that value somewhere)
Ok, I see...
> >
> > > + if (!DECL_P (t) || type_with_alias_set_p (TYPE_MAIN_VARIANT (TREE_TYPE
> > > (t))))
> > > + attrs.alias = get_alias_set (t);
> > > + else
> > > + gcc_assert (DECL_P (t) && TREE_ADDRESSABLE (t));
> >
> > This assert also looks fishy to me... (also just use 'type' instead
> > of TREE_TYPE (t), not sure why you need to pun to the main variant
> > here either - get_alias_set will do that for you and so should
> > type_with_alias_set_p if that is necessary).
>
> I am not sure if TYPE_MAIN_VARIANT is really needed here. What I know is that
> complete types may have incomplete variants.
How can that be? TYPE_FIELDS is shared across variants and all variants
should be layed out.
> I was bit worried that I can
> disable calculation of alias set of something that do matter by not checking
> main variant in case we somehow manage to have a variable of incomplete
> vairant
> type but still access it because we know its complete main variant (that would
> be somewhat broken).
>
> That is also why I added the check - the idea is that at least I can check
> that this
> particular var is indeed having address taken. Other option would be to
> define
> an invalid alias set and assign MEM RTX this invalid alias set and make
> alias.c
> bomb when it trips across it.
>
> I actually noticed it triggers during producing some libjava glue where we
> forgot to set TREE_ADDRESSABLE for artificial variable that takes address.
> I will look into fix later today or tomorrow.
Ok, thanks.
Richard.
> Honza
> >
> > The rest of the patch looks ok.
> >
> > Thanks,
> > Richard.
> >
> > > MEM_VOLATILE_P (ref) |= TYPE_VOLATILE (type);
> > > MEM_POINTER (ref) = POINTER_TYPE_P (type);
> > > Index: ipa-icf-gimple.c
> > > ===================================================================
> > > --- ipa-icf-gimple.c (revision 223508)
> > > +++ ipa-icf-gimple.c (working copy)
> > > @@ -274,6 +274,12 @@ func_checker::compatible_types_p (tree t
> > > if (!types_compatible_p (t1, t2))
> > > return return_false_with_msg ("types are not compatible");
> > >
> > > + /* FIXME: type compatiblity checks for types that are never stored
> > > + to memory is not very useful. We should update the code to avoid
> > > + calling compatible_types_p on types that will never be used. */
> > > + if (!type_with_alias_set_p (t1) || !type_with_alias_set_p (t2))
> > > + return true;
> > > +
> > > if (get_alias_set (t1) != get_alias_set (t2))
> > > return return_false_with_msg ("alias sets are different");
> > >
> > >
> > >
> >
> > --
> > Richard Biener <[email protected]>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu,
> > Graham Norton, HRB 21284 (AG Nuernberg)
>
>
--
Richard Biener <[email protected]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham
Norton, HRB 21284 (AG Nuernberg)
