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: Do not compute alias sets for types that don't need them


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

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 <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)


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