Support ofsetted parameters in local modref

Richard Biener rguenther@suse.de
Tue Oct 13 10:18:14 GMT 2020


On Tue, 13 Oct 2020, Jan Hubicka wrote:

> Hi,
> this patch makes local modref to track parameters that are passed through with
> and adjustment (via pointer_plus or addr_expr of mem_ref).  I intended to re-use
> logic in ipa-prop, but it turns out that it is weird:
>   if (TREE_CODE (op1) != ADDR_EXPR)
>     return;
>   op1 = TREE_OPERAND (op1, 0);
>   if (TREE_CODE (TREE_TYPE (op1)) != RECORD_TYPE)
>     return;
>   base = get_ref_base_and_extent_hwi (op1, &offset, &size, &reverse);
>   offset_int mem_offset;
>   if (!base
>       || TREE_CODE (base) != MEM_REF
>       || !mem_ref_offset (base).is_constant (&mem_offset))
>     return;
>   offset += mem_offset.to_short_addr () * BITS_PER_UNIT;
>   ssa = TREE_OPERAND (base, 0);
>   if (TREE_CODE (ssa) != SSA_NAME
>       || !SSA_NAME_IS_DEFAULT_DEF (ssa)
>       || offset < 0)
>     return;
> 
>   /* Dynamic types are changed in constructors and destructors.  */
>   index = ipa_get_param_decl_index (info, SSA_NAME_VAR (ssa));
>   if (index >= 0 && param_type && POINTER_TYPE_P (param_type))
>     ipa_set_ancestor_jf (jfunc, offset,  index,
> 			 parm_ref_data_pass_through_p (fbi, index, call, ssa));
> 
> First it does not handle POINTER_PLUS that is quite common i.e. for
> 
> test (int *a)
> {
>   test2 (a+1);
> }
> 
> and also it restrict to access paths that starts by RECORD_TYPE.  This seems
> not to make sense since all ADDR_EXPRs of refs are just pointer adjustements.
> I think it is leftover from time when ancestor was meant to track C++ type
> hiearchy.
> 
> Next it also does refuse negative offset for not very apparent reason (this
> happens in multiple inheritance in C++).
> 
> On tramp3d it is also common that the parameter ADDR_EXPR happens in
> separate statement rather than call itself (i.e. no forward propagation
> is done)
> 
> Finally it works on bits witout overflow check and uses HOST_WIDE_INT
> instead of poly_int64 that I think would be right datatype to accumulate
> offsets these days.
> 
> So I implemented my own pattern matching and I think we should switch ipa-prop
> on it incrementally. It is based on similar logic in
> ao_ref_init_from_ptr_and_size.

So instead of re-inventing the wheel what about splitting out a
common helper instead?

> I support chained statements since they happen
> in tramp3d after early opts.
> 
> Bootstrapped/regtested x86_64-linux.  Also ltobootstrapped and I am waiting
> for cc1plus stats to finish.  OK?
> 
> gcc/ChangeLog:
> 
> 2020-10-13  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* ipa-modref.c (merge_call_side_effects): Use
> 	unadjusted_ptr_and_unit_offset.
> 	* ipa-prop.c (unadjusted_ptr_and_unit_offset): Declare.
> 	* ipa-prop.h (unadjusted_ptr_and_unit_offset): New function.
> 
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 4f86b9ccea1..92119eb6fe3 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -531,6 +532,10 @@ merge_call_side_effects (modref_summary *cur_summary,
>    for (unsigned i = 0; i < gimple_call_num_args (stmt); i++)
>      {
>        tree op = gimple_call_arg (stmt, i);
> +      bool offset_known;
> +      poly_int64 offset;
> +
> +      offset_known = unadjusted_ptr_and_unit_offset (op, &op, &offset);
>        if (TREE_CODE (op) == SSA_NAME
>  	  && SSA_NAME_IS_DEFAULT_DEF (op)
>  	  && TREE_CODE (SSA_NAME_VAR (op)) == PARM_DECL)
> @@ -547,15 +552,23 @@ merge_call_side_effects (modref_summary *cur_summary,
>  	      index++;
>  	    }
>  	  parm_map[i].parm_index = index;
> -	  parm_map[i].parm_offset_known = true;
> -	  parm_map[i].parm_offset = 0;
> +	  parm_map[i].parm_offset_known = offset_known;
> +	  parm_map[i].parm_offset = offset;
>  	}
>        else if (points_to_local_or_readonly_memory_p (op))
>  	parm_map[i].parm_index = -2;
>        else
>  	parm_map[i].parm_index = -1;
>        if (dump_file)
> -	fprintf (dump_file, " %i", parm_map[i].parm_index);
> +	{
> +	  fprintf (dump_file, " %i", parm_map[i].parm_index);
> +	  if (parm_map[i].parm_offset_known)
> +	    {
> +	      fprintf (dump_file, " offset:");
> +	      print_dec ((poly_int64_pod)parm_map[i].parm_offset,
> +			 dump_file, SIGNED);
> +	    }
> +	}
>      }
>    if (dump_file)
>      fprintf (dump_file, "\n");
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 2d09d913051..fe317f56e02 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -1222,6 +1222,72 @@ load_from_unmodified_param_or_agg (struct ipa_func_body_info *fbi,
>    return index;
>  }
>  
> +/* Walk pointer adjustemnts from OP (such as POINTER_PLUS and ADDR_EXPR)
> +   to find original pointer.  Initialize RET to the pointer which results from
> +   the walk.
> +   If offset is known return true and initialize OFFSET_RET.  */
> +
> +bool
> +unadjusted_ptr_and_unit_offset (tree op, tree *ret, poly_int64 *offset_ret)
> +{
> +  poly_int64 offset = 0;
> +  bool offset_known = true;
> +
> +  while (true)
> +    {
> +      if (TREE_CODE (op) == ADDR_EXPR)
> +	{
> +	  poly_int64 extra_offset = 0;
> +	  tree base = get_addr_base_and_unit_offset (TREE_OPERAND (op, 0),
> +						     &offset);
> +	  if (!base)
> +	    {
> +	      base = get_base_address (TREE_OPERAND (op, 0));
> +	      if (TREE_CODE (base) != MEM_REF)
> +		break;
> +	      offset_known = false;
> +	    }
> +	  else
> +	    {
> +	      if (TREE_CODE (base) != MEM_REF)
> +		break;
> +	      offset += extra_offset;
> +	    }
> +	  op = TREE_OPERAND (base, 0);
> +	  if (mem_ref_offset (base).to_shwi (&extra_offset))
> +	    offset += extra_offset;
> +	  else
> +	    offset_known = false;
> +	}
> +      else if (TREE_CODE (op) == SSA_NAME
> +	       && !SSA_NAME_IS_DEFAULT_DEF (op))
> +	{
> +	  gimple *pstmt = SSA_NAME_DEF_STMT (op);
> +
> +	  if (gimple_assign_single_p (pstmt))
> +	    op = gimple_assign_rhs1 (pstmt);
> +	  else if (is_gimple_assign (pstmt)
> +		   && gimple_assign_rhs_code (pstmt) == POINTER_PLUS_EXPR)
> +	    {
> +	      poly_int64 extra_offset = 0;
> +	      if (ptrdiff_tree_p (gimple_assign_rhs2 (pstmt),
> +		  &extra_offset))
> +		offset += extra_offset;
> +	      else
> +		offset_known = false;
> +	      op = gimple_assign_rhs1 (pstmt);
> +	    }
> +	  else
> +	    break;
> +	}
> +      else
> +	break;
> +    }
> +  *ret = op;
> +  *offset_ret = offset;
> +  return offset_known;
> +}
> +
>  /* Given that an actual argument is an SSA_NAME (given in NAME) and is a result
>     of an assignment statement STMT, try to determine whether we are actually
>     handling any of the following cases and construct an appropriate jump
> diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
> index 8b2edf6300c..0bbbbf9bd9f 100644
> --- a/gcc/ipa-prop.h
> +++ b/gcc/ipa-prop.h
> @@ -1144,6 +1144,8 @@ void ipa_dump_param (FILE *, class ipa_node_params *info, int i);
>  void ipa_release_body_info (struct ipa_func_body_info *);
>  tree ipa_get_callee_param_type (struct cgraph_edge *e, int i);
>  bool ipcp_get_parm_bits (tree, tree *, widest_int *);
> +bool unadjusted_ptr_and_unit_offset (tree op, tree *ret,
> +				     poly_int64 *offset_ret);
>  
>  /* From tree-sra.c:  */
>  tree build_ref_for_offset (location_t, tree, poly_int64, bool, tree,
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend


More information about the Gcc-patches mailing list