[RFC] Handle unary pass-through jump functions for ipa-vrp

Hi Honza,

I reverted this patch after it was reported that it resulted in 
bootstrap compare failure in some targets.

I reproduced it and tracked to a mistake in the patch that introduced it.

That is, in propagate_vr_accross_jump_function, I had:

	  if (src_lats->m_value_range.bottom_p ())
	    return false;

which should have been:

	  if (src_lats->m_value_range.bottom_p ())
	    return dest_lat->set_to_bottom ();


I also fixed update_jump_functions_after_inlining as reported in pr78268.

I now bootstrapped the patch (lto and normal) on two affected targets 
aarch64-none-linux-gnu and powerpc64le-unknown-linux-gnu. I also tested 
it on x86_64-linux-gnu with no new regressions. Is this OK?


Thanks,
Kugan


gcc/testsuite/ChangeLog:

2016-11-13  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* g++.dg/torture/pr78268.C: New test.


gcc/ChangeLog:

2016-11-13  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* ipa-cp.c (ipa_get_jf_pass_through_result): Skip unary expressions.
	(propagate_vr_accross_jump_function): Handle unary expressions.
	* ipa-prop.c (ipa_set_jf_unary_pass_through): New.
	(load_from_param_1): New.
	(load_from_unmodified_param): Factor common part into load_from_param_1.
	(load_from_param): New.
	(compute_complex_assign_jump_func): Handle unary expressions.
	(update_jump_functions_after_inlining): Likewise.
	(ipa_write_jump_function): Likewise.
	(ipa_read_jump_function): Likewise.

> Hi Honza,
> 
> I reverted this patch after it was reported that it resulted in
> bootstrap compare failure in some targets.
> 
> I reproduced it and tracked to a mistake in the patch that introduced it.
> 
> That is, in propagate_vr_accross_jump_function, I had:
> 
> 	  if (src_lats->m_value_range.bottom_p ())
> 	    return false;
> 
> which should have been:
> 
> 	  if (src_lats->m_value_range.bottom_p ())
> 	    return dest_lat->set_to_bottom ();

Oops, sorry for missing that :)
> 
> 
> I also fixed update_jump_functions_after_inlining as reported in pr78268.
> 
> I now bootstrapped the patch (lto and normal) on two affected
> targets aarch64-none-linux-gnu and powerpc64le-unknown-linux-gnu. I
> also tested it on x86_64-linux-gnu with no new regressions. Is this
> OK?

Yes, thanks for fixing the issue!

Can you, please, also send patch to https://gcc.gnu.org/gcc-7/changes.html
which mention the new features and command line options?

Honza

> 
> 
> Thanks,
> Kugan
> 
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-11-13  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* g++.dg/torture/pr78268.C: New test.
> 
> 
> gcc/ChangeLog:
> 
> 2016-11-13  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* ipa-cp.c (ipa_get_jf_pass_through_result): Skip unary expressions.
> 	(propagate_vr_accross_jump_function): Handle unary expressions.
> 	* ipa-prop.c (ipa_set_jf_unary_pass_through): New.
> 	(load_from_param_1): New.
> 	(load_from_unmodified_param): Factor common part into load_from_param_1.
> 	(load_from_param): New.
> 	(compute_complex_assign_jump_func): Handle unary expressions.
> 	(update_jump_functions_after_inlining): Likewise.
> 	(ipa_write_jump_function): Likewise.
> 	(ipa_read_jump_function): Likewise.
> 

> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 79e621a..2ec671f 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -1219,13 +1219,19 @@ ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input)
>      return NULL_TREE;
>  
>    if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
> -      == tcc_comparison)
> -    restype = boolean_type_node;
> +      == tcc_unary)
> +    res = fold_unary (ipa_get_jf_pass_through_operation (jfunc),
> +		      TREE_TYPE (input), input);
>    else
> -    restype = TREE_TYPE (input);
> -  res = fold_binary (ipa_get_jf_pass_through_operation (jfunc), restype,
> -		     input, ipa_get_jf_pass_through_operand (jfunc));
> -
> +    {
> +      if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
> +	  == tcc_comparison)
> +	restype = boolean_type_node;
> +      else
> +	restype = TREE_TYPE (input);
> +      res = fold_binary (ipa_get_jf_pass_through_operation (jfunc), restype,
> +			 input, ipa_get_jf_pass_through_operand (jfunc));
> +    }
>    if (res && !is_gimple_ip_invariant (res))
>      return NULL_TREE;
>  
> @@ -1857,13 +1863,32 @@ propagate_vr_accross_jump_function (cgraph_edge *cs,
>    if (jfunc->type == IPA_JF_PASS_THROUGH)
>      {
>        struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
> -      if (dest_lat->bottom_p ())
> -	return false;
>        int src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
>        src_lats = ipa_get_parm_lattices (caller_info, src_idx);
>  
>        if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
>  	return dest_lat->meet_with (src_lats->m_value_range);
> +      else if (param_type
> +	       && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
> +		   == tcc_unary))
> +	{
> +	  value_range vr;
> +	  memset (&vr, 0, sizeof (vr));
> +	  tree operand_type = ipa_get_type (caller_info, src_idx);
> +	  enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc);
> +
> +	  if (src_lats->m_value_range.bottom_p ())
> +	    return dest_lat->set_to_bottom ();
> +
> +	  extract_range_from_unary_expr (&vr,
> +					 operation,
> +					 param_type,
> +					 &src_lats->m_value_range.m_vr,
> +					 operand_type);
> +	  if (vr.type == VR_RANGE
> +	      || vr.type == VR_ANTI_RANGE)
> +	    return dest_lat->meet_with (&vr);
> +	}
>      }
>    else if (jfunc->type == IPA_JF_CONST)
>      {
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 74fe199..6321fdd 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -446,6 +446,18 @@ ipa_set_jf_simple_pass_through (struct ipa_jump_func *jfunc, int formal_id,
>    jfunc->value.pass_through.agg_preserved = agg_preserved;
>  }
>  
> +/* Set JFUNC to be an unary pass through jump function.  */
> +
> +static void
> +ipa_set_jf_unary_pass_through (struct ipa_jump_func *jfunc, int formal_id,
> +			       enum tree_code operation)
> +{
> +  jfunc->type = IPA_JF_PASS_THROUGH;
> +  jfunc->value.pass_through.operand = NULL_TREE;
> +  jfunc->value.pass_through.formal_id = formal_id;
> +  jfunc->value.pass_through.operation = operation;
> +  jfunc->value.pass_through.agg_preserved = false;
> +}
>  /* Set JFUNC to be an arithmetic pass through jump function.  */
>  
>  static void
> @@ -849,21 +861,19 @@ parm_preserved_before_stmt_p (struct ipa_func_body_info *fbi, int index,
>    return !modified;
>  }
>  
> -/* If STMT is an assignment that loads a value from an parameter declaration,
> -   return the index of the parameter in ipa_node_params which has not been
> -   modified.  Otherwise return -1.  */
> +/* Main worker for load_from_unmodified_param and load_from_param.
> +   If STMT is an assignment that loads a value from an parameter declaration,
> +   return the index of the parameter in ipa_node_params.  Otherwise return -1.  */
>  
>  static int
> -load_from_unmodified_param (struct ipa_func_body_info *fbi,
> -			    vec<ipa_param_descriptor> descriptors,
> -			    gimple *stmt)
> +load_from_param_1 (struct ipa_func_body_info *fbi,
> +		   vec<ipa_param_descriptor> descriptors,
> +		   gimple *stmt)
>  {
>    int index;
>    tree op1;
>  
> -  if (!gimple_assign_single_p (stmt))
> -    return -1;
> -
> +  gcc_checking_assert (is_gimple_assign (stmt));
>    op1 = gimple_assign_rhs1 (stmt);
>    if (TREE_CODE (op1) != PARM_DECL)
>      return -1;
> @@ -876,6 +886,40 @@ load_from_unmodified_param (struct ipa_func_body_info *fbi,
>    return index;
>  }
>  
> +/* If STMT is an assignment that loads a value from an parameter declaration,
> +   return the index of the parameter in ipa_node_params which has not been
> +   modified.  Otherwise return -1.  */
> +
> +static int
> +load_from_unmodified_param (struct ipa_func_body_info *fbi,
> +			    vec<ipa_param_descriptor> descriptors,
> +			    gimple *stmt)
> +{
> +  if (!gimple_assign_single_p (stmt))
> +    return -1;
> +
> +  return load_from_param_1 (fbi, descriptors, stmt);
> +}
> +
> +/* If STMT is an assignment that loads a value from an parameter declaration,
> +   return the index of the parameter in ipa_node_params.  Otherwise return -1.  */
> +
> +static int
> +load_from_param (struct ipa_func_body_info *fbi,
> +		 vec<ipa_param_descriptor> descriptors,
> +		 gimple *stmt)
> +{
> +  if (!is_gimple_assign (stmt))
> +    return -1;
> +
> +  enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
> +  if ((get_gimple_rhs_class (rhs_code) != GIMPLE_SINGLE_RHS)
> +      && (get_gimple_rhs_class (rhs_code) != GIMPLE_UNARY_RHS))
> +    return -1;
> +
> +  return load_from_param_1 (fbi, descriptors, stmt);
> +}
> +
>  /* Return true if memory reference REF (which must be a load through parameter
>     with INDEX) loads data that are known to be unmodified in this function
>     before reaching statement STMT.  */
> @@ -1109,6 +1153,7 @@ compute_complex_assign_jump_func (struct ipa_func_body_info *fbi,
>    tree op1, tc_ssa, base, ssa;
>    bool reverse;
>    int index;
> +  gimple *stmt2 = stmt;
>  
>    op1 = gimple_assign_rhs1 (stmt);
>  
> @@ -1117,13 +1162,16 @@ compute_complex_assign_jump_func (struct ipa_func_body_info *fbi,
>        if (SSA_NAME_IS_DEFAULT_DEF (op1))
>  	index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
>        else
> -	index = load_from_unmodified_param (fbi, info->descriptors,
> -					    SSA_NAME_DEF_STMT (op1));
> +	{
> +	  index = load_from_param (fbi, info->descriptors,
> +				   SSA_NAME_DEF_STMT (op1));
> +	  stmt2 = SSA_NAME_DEF_STMT (op1);
> +	}
>        tc_ssa = op1;
>      }
>    else
>      {
> -      index = load_from_unmodified_param (fbi, info->descriptors, stmt);
> +      index = load_from_param (fbi, info->descriptors, stmt);
>        tc_ssa = gimple_assign_lhs (stmt);
>      }
>  
> @@ -1147,6 +1195,11 @@ compute_complex_assign_jump_func (struct ipa_func_body_info *fbi,
>  	  bool agg_p = parm_ref_data_pass_through_p (fbi, index, call, tc_ssa);
>  	  ipa_set_jf_simple_pass_through (jfunc, index, agg_p);
>  	}
> +      else if (is_gimple_assign (stmt2)
> +	       && (gimple_expr_code (stmt2) != NOP_EXPR)
> +	       && (TREE_CODE_CLASS (gimple_expr_code (stmt2)) == tcc_unary))
> +	ipa_set_jf_unary_pass_through (jfunc, index,
> +				       gimple_assign_rhs_code (stmt2));
>        return;
>      }
>  
> @@ -2518,6 +2571,12 @@ update_jump_functions_after_inlining (struct cgraph_edge *cs,
>  	      dst->value.ancestor.agg_preserved &=
>  		src->value.pass_through.agg_preserved;
>  	    }
> +	  else if (src->type == IPA_JF_PASS_THROUGH
> +		   && TREE_CODE_CLASS (src->value.pass_through.operation) == tcc_unary)
> +	    {
> +	      dst->value.ancestor.formal_id = src->value.pass_through.formal_id;
> +	      dst->value.ancestor.agg_preserved = false;
> +	    }
>  	  else if (src->type == IPA_JF_ANCESTOR)
>  	    {
>  	      dst->value.ancestor.formal_id = src->value.ancestor.formal_id;
> @@ -2583,6 +2642,8 @@ update_jump_functions_after_inlining (struct cgraph_edge *cs,
>  			  && ipa_get_jf_pass_through_agg_preserved (src);
>  			ipa_set_jf_simple_pass_through (dst, formal_id, agg_p);
>  		      }
> +		    else if (TREE_CODE_CLASS (operation) == tcc_unary)
> +		      ipa_set_jf_unary_pass_through (dst, formal_id, operation);
>  		    else
>  		      {
>  			tree operand = ipa_get_jf_pass_through_operand (src);
> @@ -4666,6 +4727,9 @@ ipa_write_jump_function (struct output_block *ob,
>  	  bp_pack_value (&bp, jump_func->value.pass_through.agg_preserved, 1);
>  	  streamer_write_bitpack (&bp);
>  	}
> +      else if (TREE_CODE_CLASS (jump_func->value.pass_through.operation)
> +	       == tcc_unary)
> +	streamer_write_uhwi (ob, jump_func->value.pass_through.formal_id);
>        else
>  	{
>  	  stream_write_tree (ob, jump_func->value.pass_through.operand, true);
> @@ -4745,6 +4809,11 @@ ipa_read_jump_function (struct lto_input_block *ib,
>  	  bool agg_preserved = bp_unpack_value (&bp, 1);
>  	  ipa_set_jf_simple_pass_through (jump_func, formal_id, agg_preserved);
>  	}
> +      else if (TREE_CODE_CLASS (operation) == tcc_unary)
> +	{
> +	  int formal_id =  streamer_read_uhwi (ib);
> +	  ipa_set_jf_unary_pass_through (jump_func, formal_id, operation);
> +	}
>        else
>  	{
>  	  tree operand = stream_read_tree (ib, data_in);
> diff --git a/gcc/testsuite/g++.dg/torture/pr78268.C b/gcc/testsuite/g++.dg/torture/pr78268.C
> index e69de29..ef4547c 100644
> --- a/gcc/testsuite/g++.dg/torture/pr78268.C
> +++ b/gcc/testsuite/g++.dg/torture/pr78268.C
> @@ -0,0 +1,25 @@
> +// { dg-do compile }
> +typedef enum {} nsresult;
> +
> +struct A {
> +      virtual nsresult m_fn1(bool);
> +};
> +
> +struct B {
> +      A *operator[](int);
> +};
> +
> +struct C {
> +      nsresult m_fn2(bool);
> +        bool m_fn3(bool);
> +	  B mDataSources;
> +};
> +nsresult C::m_fn2(bool p1)
> +{
> +  m_fn3(!p1);
> +}
> +
> +bool C::m_fn3(bool p1)
> +{
> +  mDataSources[0]->m_fn1(p1);
> +}