[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);

> +}