On Tue, 8 Jul 2014, Jan Hubicka wrote:
> Hi,
> I had to revert change to ipa-prop.c that made stmt_may_be_vtbl_ptr_store
> to know that clobbers can not be such stores.
>
> This triggered byg in detect_type_change_from_memory_writes. This function
> does
> two things. First is that it tries to prove that vtbl pointer was not modified
> in the function body. Second it tries to prove that the modification is a
> known
> type. For this it needs to find proper vtbl store along every path to the
> call.
> This is not what happens because it only checks that all vtbl stores it found
> are of the same type. The object may come pre-initialized to the function.
>
> The bug on stopping of clobbers sort of fixed the issue, because we consider
> only
> object in declarations and those are constructed in a dominating block, but it
> works by accident.
>
> This is patch we discussed back then adding interface to walk_aliased_vdefs
> declaring whether entry of function was reached and avoids
> stmt_may_be_vtbl_ptr_store
> to jump into conclussion about the dynamic type in that case.
>
> Bootstrapped/regtested x86_64-linux, OK?
Ok.
Thanks,
Richard.
> Honza
>
> * tree-ssa-alias.c (walk_aliased_vdefs_1): Add FUNCTION_ENTRY_REACHED
> parameter.
> (walk_aliased_vdefs): Likewise.
> * tree-ssa-alias.h (walk_aliased_vdefs): Likewise.
> * ipa-prop.c (stmt_may_be_vtbl_ptr_store): Skip clobbers
> (detect_type_change_from_memory_writes): Check if entry was reached.
> Index: tree-ssa-alias.c
> ===================================================================
> --- tree-ssa-alias.c (revision 212339)
> +++ tree-ssa-alias.c (working copy)
> @@ -2643,6 +2643,9 @@ walk_non_aliased_vuses (ao_ref *ref, tre
> WALKER is called with REF, the current vdef and DATA. If WALKER
> returns true the walk is stopped, otherwise it continues.
>
> + If function entry is reached, FUNCTION_ENTRY_REACHED is set to true.
> + The pointer may be NULL and then we do not track this information.
> +
> At PHI nodes walk_aliased_vdefs forks into one walk for reach
> PHI argument (but only one walk continues on merge points), the
> return value is true if any of the walks was successful.
> @@ -2652,8 +2655,11 @@ walk_non_aliased_vuses (ao_ref *ref, tre
> static unsigned int
> walk_aliased_vdefs_1 (ao_ref *ref, tree vdef,
> bool (*walker)(ao_ref *, tree, void *), void *data,
> - bitmap *visited, unsigned int cnt)
> + bitmap *visited, unsigned int cnt,
> + bool *function_entry_reached)
> {
> + if (function_entry_reached)
> + *function_entry_reached = false;
> do
> {
> gimple def_stmt = SSA_NAME_DEF_STMT (vdef);
> @@ -2663,7 +2669,11 @@ walk_aliased_vdefs_1 (ao_ref *ref, tree
> return cnt;
>
> if (gimple_nop_p (def_stmt))
> - return cnt;
> + {
> + if (function_entry_reached)
> + *function_entry_reached = true;
> + return cnt;
> + }
> else if (gimple_code (def_stmt) == GIMPLE_PHI)
> {
> unsigned i;
> @@ -2671,7 +2681,8 @@ walk_aliased_vdefs_1 (ao_ref *ref, tree
> *visited = BITMAP_ALLOC (NULL);
> for (i = 0; i < gimple_phi_num_args (def_stmt); ++i)
> cnt += walk_aliased_vdefs_1 (ref, gimple_phi_arg_def (def_stmt, i),
> - walker, data, visited, 0);
> + walker, data, visited, 0,
> + function_entry_reached);
> return cnt;
> }
>
> @@ -2690,7 +2701,8 @@ walk_aliased_vdefs_1 (ao_ref *ref, tree
> unsigned int
> walk_aliased_vdefs (ao_ref *ref, tree vdef,
> bool (*walker)(ao_ref *, tree, void *), void *data,
> - bitmap *visited)
> + bitmap *visited,
> + bool *function_entry_reached)
> {
> bitmap local_visited = NULL;
> unsigned int ret;
> @@ -2698,7 +2710,8 @@ walk_aliased_vdefs (ao_ref *ref, tree vd
> timevar_push (TV_ALIAS_STMT_WALK);
>
> ret = walk_aliased_vdefs_1 (ref, vdef, walker, data,
> - visited ? visited : &local_visited, 0);
> + visited ? visited : &local_visited, 0,
> + function_entry_reached);
> if (local_visited)
> BITMAP_FREE (local_visited);
>
> Index: tree-ssa-alias.h
> ===================================================================
> --- tree-ssa-alias.h (revision 212339)
> +++ tree-ssa-alias.h (working copy)
> @@ -123,7 +123,8 @@ extern void *walk_non_aliased_vuses (ao_
> void *);
> extern unsigned int walk_aliased_vdefs (ao_ref *, tree,
> bool (*)(ao_ref *, tree, void *),
> - void *, bitmap *);
> + void *, bitmap *,
> + bool *function_entry_reached = NULL);
> extern void dump_alias_info (FILE *);
> extern void debug_alias_info (void);
> extern void dump_points_to_solution (FILE *, struct pt_solution *);
> Index: ipa-prop.c
> ===================================================================
> --- ipa-prop.c (revision 212339)
> +++ ipa-prop.c (working copy)
> @@ -638,7 +638,8 @@ stmt_may_be_vtbl_ptr_store (gimple stmt)
> {
> if (is_gimple_call (stmt))
> return false;
> - /* TODO: Skip clobbers, doing so triggers problem in PR60306. */
> + if (gimple_clobber_p (stmt))
> + return false;
> else if (is_gimple_assign (stmt))
> {
> tree lhs = gimple_assign_lhs (stmt);
> @@ -817,6 +818,7 @@ detect_type_change_from_memory_writes (t
> {
> struct type_change_info tci;
> ao_ref ao;
> + bool entry_reached = false;
>
> gcc_checking_assert (DECL_P (arg)
> || TREE_CODE (arg) == MEM_REF
> @@ -847,13 +849,16 @@ detect_type_change_from_memory_writes (t
> tci.multiple_types_encountered = false;
>
> walk_aliased_vdefs (&ao, gimple_vuse (call), check_stmt_for_type_change,
> - &tci, NULL);
> + &tci, NULL, &entry_reached);
> if (!tci.type_maybe_changed)
> return false;
>
> if (!tci.known_current_type
> || tci.multiple_types_encountered
> - || offset != 0)
> + || offset != 0
> + /* When the walk reached function entry, it means that type
> + is set along some paths but not along others. */
> + || entry_reached)
> jfunc->type = IPA_JF_UNKNOWN;
> else
> ipa_set_jf_known_type (jfunc, 0, tci.known_current_type, comp_type);
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
