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


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

-- 
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]