> 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
