On Fri, 22 May 2015, Jan Hubicka wrote:
> Hi,
> this patch fixes few cases where we compute alias type and don't need to
> that are found by adding type_with_alias_set_p check to alias.c (I will send
> this patch separately as there is still one ICE caught by it I believe
> originating from ipa-icf-gimple, I have more involved fix for that)
>
> The point of all this is to check that we don't make bogus querries to alias
> oracle and don't try to use them somehow - we did in ipa-icf-gimple. This is
> a
> bug as the alias oracle gives stupid answers to stupid questions and sometimes
> think the types conflicts sometimes thinks they doesn't. The other three
> cases
> are sort of just pointless computations.
>
> Next part I would like to break out is the path computing equivalences
> considering
> incomplete types (i.e. where all strucutres are equivalent and so all unions
> or
> all functions of a given return value)
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
> * tree-streamer-out.c (pack_ts_type_common_value_fields): Only consider
> alias set for types that have them.
> * lto-streamer-out.c (DFS::DFS_write_tree_body): Likewise.
> * emit-rtl.c (set_mem_attributes_minus_bitpos): Likewise.
> * ipa-icf-gimple.c (func_checker::compatible_types_p): Likewise.
> 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);
> }
>
> Index: lto-streamer-out.c
> ===================================================================
> --- lto-streamer-out.c (revision 223508)
> +++ lto-streamer-out.c (working copy)
> @@ -1146,6 +1146,7 @@ hash_tree (struct streamer_tree_cache_d
> hstate.add_int (TYPE_ALIGN (t));
> hstate.add_int ((TYPE_ALIAS_SET (t) == 0
> || (!in_lto_p
> + && type_with_alias_set_p (t)
> && get_alias_set (t) == 0))
> ? 0 : -1);
> }
> 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?
> + 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).
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)
