Do not compute alias sets for types that don't need them

Jan Hubicka hubicka@ucw.cz
Tue May 26 19:00:00 GMT 2015


> Hi,
> 
> On Fri, 22 May 2015, Jan Hubicka wrote:
> 
> > Index: tree-streamer-out.c
> > ===================================================================
> > --- tree-streamer-out.c	(revision 223508)
> > +++ tree-streamer-out.c	(working copy)
> > @@ -346,6 +346,7 @@ pack_ts_type_common_value_fields (struct
> >       alias-set zero to this type.  */
> >    bp_pack_var_len_int (bp, (TYPE_ALIAS_SET (expr) == 0
> >  			    || (!in_lto_p
> > +				&& type_with_alias_set_p (expr)
> >  				&& get_alias_set (expr) == 0)) ? 0 : -1);
> 
> I find such interfaces very ugly.  IOW, when it's always (or often) 
> necessary to call check_foo_p() before foo() can be called then the 
> checking should be part of foo() (and it should then return a conservative 
> value, i.e. alias set 0), and that requirement not be imposed on the 
> callers of foo().  I.e. why can't whatever checks you do in 
> type_with_alias_set_p be included in get_alias_set?

Because of sanity checking: I want to make alias sets of those types undefined
rather than having random values.  The point is that using the alias set in
alias oracle querry is wrong.

Now I run into the case that we do produce MEM exprs for incomplete variants
just to take their address so I was thinking the other day about defining an
invalid alias set -2, making get_alias_set to return it and ICE later when query
is actually made?

We do have wrong query problems at least in ipa-icf, so I think it is worthwhile
sanity check.
> 
> > +     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.  */
> > +  if (!DECL_P (t) || type_with_alias_set_p (TYPE_MAIN_VARIANT (TREE_TYPE (t))))
> > +    attrs.alias = get_alias_set (t);
> 
> And if the checking needs to go down the main-variant chain then this 
> should be done inside type_with_alias_set_p(), not in the caller, 
> otherwise even the symmetry between arguments of type_with_alias_set_p(xy) 
> and get_alias_set(xy) is destroyed (but see above for why I think 
> type_with_alias_set_p shouldn't even exist).

Yep, good point - I will cleanup this.

Honza



More information about the Gcc-patches mailing list