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