This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Proper fix for PR58477


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 <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]