diff mbox series

Support ofsetted parameters in local modref

Message ID 20201013091852.GA72371@kam.mff.cuni.cz
State New
Headers show
Series Support ofsetted parameters in local modref | expand

Commit Message

Jan Hubicka Oct. 13, 2020, 9:18 a.m. UTC
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. 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.

Comments

Jan Hubicka Oct. 13, 2020, 9:55 a.m. UTC | #1
> 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. 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?

So stats finished, for record:
Alias oracle query stats:
  refs_may_alias_p: 64156070 disambiguations, 74373175 queries
  ref_maybe_used_by_call_p: 142785 disambiguations, 65060092 queries
  call_may_clobber_ref_p: 22991 disambiguations, 28817 queries
  nonoverlapping_component_refs_p: 0 disambiguations, 37222 queries
  nonoverlapping_refs_since_match_p: 19427 disambiguations, 55940 must overlaps, 76158 queries
  aliasing_component_refs_p: 54747 disambiguations, 760197 queries
  TBAA oracle: 23613733 disambiguations 55949704 queries
               16074768 are in alias set 0
               10619115 queries asked about the same object
               125 queries asked about the same alias set
               0 access volatile
               3995285 are dependent in the DAG
               1646678 are aritificially in conflict with void *

Modref stats:
  modref use: 11758 disambiguations, 40510 queries
  modref clobber: 1505226 disambiguations, 1825004 queries
  3894419 tbaa queries (2.133924 per modref query)
  621924 base compares (0.340780 per modref query)

So about 8% increase in clobber disambiguation rate. Use disambiguation
seems unchanged.

Honza

> 
> 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 Oct. 13, 2020, 10:18 a.m. UTC | #2
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,
>
Jan Hubicka Oct. 13, 2020, 10:37 a.m. UTC | #3
> > 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?

It was my original idea. However it is not completely trivial:
ao_ref_init_from_ptr_and_size does bit something else since it is trying
to keep the original ref inside ADDR_EXPR (if one is found) to feed it
into oracle rather then stripping them all.  If refs are present it does
not need to build MEM_REF.

static void
ao_ref_init_from_ptr_and_range (ao_ref *ref, tree ptr,
				bool range_known,
				poly_int64 offset,
				poly_int64 size,
				poly_int64 max_size)
{
  poly_int64 t, extra_offset = 0;

  ref->ref = NULL_TREE;
  if (TREE_CODE (ptr) == SSA_NAME)
    {
      gimple *stmt = SSA_NAME_DEF_STMT (ptr);
      if (gimple_assign_single_p (stmt)
	  && gimple_assign_rhs_code (stmt) == ADDR_EXPR)
	ptr = gimple_assign_rhs1 (stmt);
      else if (is_gimple_assign (stmt)
	       && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
	       && ptrdiff_tree_p (gimple_assign_rhs2 (stmt), &extra_offset))
	{
	  ptr = gimple_assign_rhs1 (stmt);
	  extra_offset *= BITS_PER_UNIT;
	}
    }

  if (TREE_CODE (ptr) == ADDR_EXPR)
    {
      ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0), &t);
      if (ref->base)
	ref->offset = BITS_PER_UNIT * t;
      else
	{
	  range_known = false;
	  ref->offset = 0;
	  ref->base = get_base_address (TREE_OPERAND (ptr, 0));
	}
    }
  else
    {
      gcc_assert (POINTER_TYPE_P (TREE_TYPE (ptr)));
      ref->base = build2 (MEM_REF, char_type_node,
			  ptr, null_pointer_node);
      ref->offset = 0;
    }

I could add a flag if I am looking for ADDR_EXPR or just the base
pointer but it seemed more for incremental change.  The new helper
should be immediately useful for ipa-modref, ipa-prop and
ipa-polymorphic-call though.

Honza
Richard Biener Oct. 13, 2020, 11:24 a.m. UTC | #4
On Tue, 13 Oct 2020, Jan Hubicka wrote:

> > > 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?
> 
> It was my original idea. However it is not completely trivial:
> ao_ref_init_from_ptr_and_size does bit something else since it is trying
> to keep the original ref inside ADDR_EXPR (if one is found) to feed it
> into oracle rather then stripping them all.  If refs are present it does
> not need to build MEM_REF.

You've also coded other subtle differences - is it really better
to have p_1 with unknown offset for in p_3 = p_1 + i_2 instead of
having p_3?  I suppose the idea is to look for an offsetted
default def (aka parameter)?

Frankly I don't like the loop - unbounded walk over unoptimized ptr += 4;
ptr += 4; ... chain, possibly quadratic if it's like

void foo (void *p, int i)
{
  void *p1 = p + i;
  void *p2 = p1 + i;
  void *p3 = p2 + i;
  bar (p1, p2, p3);
}

so I wonder if the propagation engine instead wants to cache
the discovered base + offset for a (pointer?) SSA name?  Not
sure if this recursion when the offset becomes unknown is
of any help to ipa-prop - IPA prop is also interested in
other than pointer parameters I guess.

> static void
> ao_ref_init_from_ptr_and_range (ao_ref *ref, tree ptr,
> 				bool range_known,
> 				poly_int64 offset,
> 				poly_int64 size,
> 				poly_int64 max_size)
> {
>   poly_int64 t, extra_offset = 0;
> 
>   ref->ref = NULL_TREE;
>   if (TREE_CODE (ptr) == SSA_NAME)
>     {
>       gimple *stmt = SSA_NAME_DEF_STMT (ptr);
>       if (gimple_assign_single_p (stmt)
> 	  && gimple_assign_rhs_code (stmt) == ADDR_EXPR)
> 	ptr = gimple_assign_rhs1 (stmt);
>       else if (is_gimple_assign (stmt)
> 	       && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
> 	       && ptrdiff_tree_p (gimple_assign_rhs2 (stmt), &extra_offset))
> 	{
> 	  ptr = gimple_assign_rhs1 (stmt);
> 	  extra_offset *= BITS_PER_UNIT;
> 	}
>     }
> 
>   if (TREE_CODE (ptr) == ADDR_EXPR)
>     {
>       ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0), &t);
>       if (ref->base)
> 	ref->offset = BITS_PER_UNIT * t;
>       else
> 	{
> 	  range_known = false;
> 	  ref->offset = 0;
> 	  ref->base = get_base_address (TREE_OPERAND (ptr, 0));
> 	}
>     }
>   else
>     {
>       gcc_assert (POINTER_TYPE_P (TREE_TYPE (ptr)));
>       ref->base = build2 (MEM_REF, char_type_node,
> 			  ptr, null_pointer_node);
>       ref->offset = 0;
>     }
> 
> I could add a flag if I am looking for ADDR_EXPR or just the base
> pointer but it seemed more for incremental change.  The new helper
> should be immediately useful for ipa-modref, ipa-prop and
> ipa-polymorphic-call though.
> 
> Honza
>
Jan Hubicka Oct. 13, 2020, 12:27 p.m. UTC | #5
> On Tue, 13 Oct 2020, Jan Hubicka wrote:
> 
> > > > 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?
> > 
> > It was my original idea. However it is not completely trivial:
> > ao_ref_init_from_ptr_and_size does bit something else since it is trying
> > to keep the original ref inside ADDR_EXPR (if one is found) to feed it
> > into oracle rather then stripping them all.  If refs are present it does
> > not need to build MEM_REF.
> 
> You've also coded other subtle differences - is it really better
> to have p_1 with unknown offset for in p_3 = p_1 + i_2 instead of
> having p_3?  I suppose the idea is to look for an offsetted
> default def (aka parameter)?

Yes this was intentional (for modref).
Modref primarily cares about the based pointer (for PTA). if it knows
offset it is better.
> 
> Frankly I don't like the loop - unbounded walk over unoptimized ptr += 4;
> ptr += 4; ... chain, possibly quadratic if it's like
> 
> void foo (void *p, int i)
> {
>   void *p1 = p + i;
>   void *p2 = p1 + i;
>   void *p3 = p2 + i;
>   bar (p1, p2, p3);
> }
> 
> so I wonder if the propagation engine instead wants to cache
> the discovered base + offset for a (pointer?) SSA name?  Not
> sure if this recursion when the offset becomes unknown is
> of any help to ipa-prop - IPA prop is also interested in
> other than pointer parameters I guess.

I would like to extend ipa-prop to also record anscessor jump functions
without offset known (to help modref propagate), so in both cases this
should work.

ipa-prop currently handles arith/simple and unary apass through
separately and only then it does the ancessor.

Concerning the loop, maybe stopping after few iterations is better
solution, since ealry pases should eliminate the chains, but I can add
the cache.

Honza
> 
> > static void
> > ao_ref_init_from_ptr_and_range (ao_ref *ref, tree ptr,
> > 				bool range_known,
> > 				poly_int64 offset,
> > 				poly_int64 size,
> > 				poly_int64 max_size)
> > {
> >   poly_int64 t, extra_offset = 0;
> > 
> >   ref->ref = NULL_TREE;
> >   if (TREE_CODE (ptr) == SSA_NAME)
> >     {
> >       gimple *stmt = SSA_NAME_DEF_STMT (ptr);
> >       if (gimple_assign_single_p (stmt)
> > 	  && gimple_assign_rhs_code (stmt) == ADDR_EXPR)
> > 	ptr = gimple_assign_rhs1 (stmt);
> >       else if (is_gimple_assign (stmt)
> > 	       && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
> > 	       && ptrdiff_tree_p (gimple_assign_rhs2 (stmt), &extra_offset))
> > 	{
> > 	  ptr = gimple_assign_rhs1 (stmt);
> > 	  extra_offset *= BITS_PER_UNIT;
> > 	}
> >     }
> > 
> >   if (TREE_CODE (ptr) == ADDR_EXPR)
> >     {
> >       ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0), &t);
> >       if (ref->base)
> > 	ref->offset = BITS_PER_UNIT * t;
> >       else
> > 	{
> > 	  range_known = false;
> > 	  ref->offset = 0;
> > 	  ref->base = get_base_address (TREE_OPERAND (ptr, 0));
> > 	}
> >     }
> >   else
> >     {
> >       gcc_assert (POINTER_TYPE_P (TREE_TYPE (ptr)));
> >       ref->base = build2 (MEM_REF, char_type_node,
> > 			  ptr, null_pointer_node);
> >       ref->offset = 0;
> >     }
> > 
> > I could add a flag if I am looking for ADDR_EXPR or just the base
> > pointer but it seemed more for incremental change.  The new helper
> > should be immediately useful for ipa-modref, ipa-prop and
> > ipa-polymorphic-call though.
> > 
> > Honza
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imend
Jan Hubicka Oct. 14, 2020, 9:13 a.m. UTC | #6
Hi,
here is updated patch with cap on number of iterations.
I set the limit to 8 and bootstrapped it with additional assert that the
limit is not met, it did not fire.

Bootstrapped/regtested x86_64-linux, OK?

gcc/ChangeLog:

2020-10-14  Jan Hubicka  <hubicka@ucw.cz>

	* doc/invoke.texi: (ipa-jump-function-lookups): Document param.
	* ipa-modref.c (merge_call_side_effects): Use
	unadjusted_ptr_and_unit_offset.
	* ipa-prop.c (unadjusted_ptr_and_unit_offset): New function.
	* ipa-prop.h (unadjusted_ptr_and_unit_offset): Declare.
	* params.opt: (-param-ipa-jump-function-lookups): New.

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index c8281ecf502..47aa69530ab 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -13456,6 +13456,9 @@ loop in the loop nest by a given number of iterations.  The strip
 length can be changed using the @option{loop-block-tile-size}
 parameter.
 
+@item ipa-jump-function-lookups
+Specifies number of statements visited during jump function offset discovery.
+
 @item ipa-cp-value-list-size
 IPA-CP attempts to track all possible values and types passed to a function's
 parameter in order to propagate them and perform devirtualization.
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 771a0a88f9a..a6dfe1fc401 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -531,6 +531,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 +551,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..cf3da6a6568 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -52,6 +52,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "domwalk.h"
 #include "builtins.h"
 #include "tree-cfgcleanup.h"
+#include "options.h"
 
 /* Function summary where the parameter infos are actually stored. */
 ipa_node_params_t *ipa_node_params_sum = NULL;
@@ -1222,6 +1223,73 @@ 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;
+  int i;
+
+  for (i = 0; i < param_ipa_jump_function_lookups; i++)
+    {
+      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,
diff --git a/gcc/params.opt b/gcc/params.opt
index d770c55143b..ec69ba04eaf 100644
--- a/gcc/params.opt
+++ b/gcc/params.opt
@@ -253,6 +253,10 @@ The size of translation unit that IPA-CP pass considers large.
 Common Joined UInteger Var(param_ipa_cp_value_list_size) Init(8) Param Optimization
 Maximum size of a list of values associated with each parameter for interprocedural constant propagation.
 
+-param-ipa-jump-function-lookups=
+Common Joined UInteger Var(param_ipa_jump_function_lookups) Init(8) Param Optimization
+Maximum number of statements visited during jump function offset discovery
+
 -param=ipa-max-aa-steps=
 Common Joined UInteger Var(param_ipa_max_aa_steps) Init(25000) Param Optimization
 Maximum number of statements that will be visited by IPA formal parameter analysis based on alias analysis in any given function.
Richard Biener Oct. 14, 2020, 9:30 a.m. UTC | #7
On Wed, 14 Oct 2020, Jan Hubicka wrote:

> Hi,
> here is updated patch with cap on number of iterations.
> I set the limit to 8 and bootstrapped it with additional assert that the
> limit is not met, it did not fire.
> 
> Bootstrapped/regtested x86_64-linux, OK?

OK.

Richard.

> gcc/ChangeLog:
> 
> 2020-10-14  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* doc/invoke.texi: (ipa-jump-function-lookups): Document param.
> 	* ipa-modref.c (merge_call_side_effects): Use
> 	unadjusted_ptr_and_unit_offset.
> 	* ipa-prop.c (unadjusted_ptr_and_unit_offset): New function.
> 	* ipa-prop.h (unadjusted_ptr_and_unit_offset): Declare.
> 	* params.opt: (-param-ipa-jump-function-lookups): New.
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index c8281ecf502..47aa69530ab 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -13456,6 +13456,9 @@ loop in the loop nest by a given number of iterations.  The strip
>  length can be changed using the @option{loop-block-tile-size}
>  parameter.
>  
> +@item ipa-jump-function-lookups
> +Specifies number of statements visited during jump function offset discovery.
> +
>  @item ipa-cp-value-list-size
>  IPA-CP attempts to track all possible values and types passed to a function's
>  parameter in order to propagate them and perform devirtualization.
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 771a0a88f9a..a6dfe1fc401 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -531,6 +531,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 +551,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..cf3da6a6568 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -52,6 +52,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "domwalk.h"
>  #include "builtins.h"
>  #include "tree-cfgcleanup.h"
> +#include "options.h"
>  
>  /* Function summary where the parameter infos are actually stored. */
>  ipa_node_params_t *ipa_node_params_sum = NULL;
> @@ -1222,6 +1223,73 @@ 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;
> +  int i;
> +
> +  for (i = 0; i < param_ipa_jump_function_lookups; i++)
> +    {
> +      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,
> diff --git a/gcc/params.opt b/gcc/params.opt
> index d770c55143b..ec69ba04eaf 100644
> --- a/gcc/params.opt
> +++ b/gcc/params.opt
> @@ -253,6 +253,10 @@ The size of translation unit that IPA-CP pass considers large.
>  Common Joined UInteger Var(param_ipa_cp_value_list_size) Init(8) Param Optimization
>  Maximum size of a list of values associated with each parameter for interprocedural constant propagation.
>  
> +-param-ipa-jump-function-lookups=
> +Common Joined UInteger Var(param_ipa_jump_function_lookups) Init(8) Param Optimization
> +Maximum number of statements visited during jump function offset discovery
> +
>  -param=ipa-max-aa-steps=
>  Common Joined UInteger Var(param_ipa_max_aa_steps) Init(25000) Param Optimization
>  Maximum number of statements that will be visited by IPA formal parameter analysis based on alias analysis in any given function.
>
Martin Jambor Oct. 14, 2020, 10:09 a.m. UTC | #8
Hi,

On Wed, Oct 14 2020, Jan Hubicka wrote:
> Hi,
> here is updated patch with cap on number of iterations.
> I set the limit to 8 and bootstrapped it with additional assert that the
> limit is not met, it did not fire.
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> gcc/ChangeLog:
>
> 2020-10-14  Jan Hubicka  <hubicka@ucw.cz>
>
> 	* doc/invoke.texi: (ipa-jump-function-lookups): Document param.
> 	* ipa-modref.c (merge_call_side_effects): Use
> 	unadjusted_ptr_and_unit_offset.
> 	* ipa-prop.c (unadjusted_ptr_and_unit_offset): New function.
> 	* ipa-prop.h (unadjusted_ptr_and_unit_offset): Declare.
> 	* params.opt: (-param-ipa-jump-function-lookups): New.
>
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 2d09d913051..cf3da6a6568 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -52,6 +52,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "domwalk.h"
>  #include "builtins.h"
>  #include "tree-cfgcleanup.h"
> +#include "options.h"
>  
>  /* Function summary where the parameter infos are actually stored. */
>  ipa_node_params_t *ipa_node_params_sum = NULL;
> @@ -1222,6 +1223,73 @@ 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;
> +  int i;
> +
> +  for (i = 0; i < param_ipa_jump_function_lookups; i++)
> +    {
> +      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;

Umm, did you really intend to clear offset_known only after the break?
(I may not understand the nuances of the get_base... functions but it
strikes me as odd.)

Thanks,

Martin
Jan Hubicka Oct. 14, 2020, 10:18 a.m. UTC | #9
> Hi,
> 
> On Wed, Oct 14 2020, Jan Hubicka wrote:
> > Hi,
> > here is updated patch with cap on number of iterations.
> > I set the limit to 8 and bootstrapped it with additional assert that the
> > limit is not met, it did not fire.
> >
> > Bootstrapped/regtested x86_64-linux, OK?
> >
> > gcc/ChangeLog:
> >
> > 2020-10-14  Jan Hubicka  <hubicka@ucw.cz>
> >
> > 	* doc/invoke.texi: (ipa-jump-function-lookups): Document param.
> > 	* ipa-modref.c (merge_call_side_effects): Use
> > 	unadjusted_ptr_and_unit_offset.
> > 	* ipa-prop.c (unadjusted_ptr_and_unit_offset): New function.
> > 	* ipa-prop.h (unadjusted_ptr_and_unit_offset): Declare.
> > 	* params.opt: (-param-ipa-jump-function-lookups): New.
> >
> > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> > index 2d09d913051..cf3da6a6568 100644
> > --- a/gcc/ipa-prop.c
> > +++ b/gcc/ipa-prop.c
> > @@ -52,6 +52,7 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "domwalk.h"
> >  #include "builtins.h"
> >  #include "tree-cfgcleanup.h"
> > +#include "options.h"
> >  
> >  /* Function summary where the parameter infos are actually stored. */
> >  ipa_node_params_t *ipa_node_params_sum = NULL;
> > @@ -1222,6 +1223,73 @@ 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;
> > +  int i;
> > +
> > +  for (i = 0; i < param_ipa_jump_function_lookups; i++)
> > +    {
> > +      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;
> 
> Umm, did you really intend to clear offset_known only after the break?
> (I may not understand the nuances of the get_base... functions but it
> strikes me as odd.)

Yes, offset_known relates to op.

We try tolookup base and if it is MEM_REF we will be able to update op.
If this is a decl or something else we thus need to give up (since we
are required to return pointer). So in first case we will return current
op for which offset_known stil lapplies, however if we found MEM_REF we
will update op but lose track of offset, since
get_addr_base_and_unit_offset failed.

Honza
> 
> Thanks,
> 
> Martin
diff mbox series

Patch

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,