[PR 82808] Use result types for arithmetic jump functions

[Martin Jambor <mjambor at suse dot cz>]

Hi,

sorry for getting so late to this.  However...

On Tue, Nov 14 2017, Richard Biener wrote:
> On Tue, Nov 14, 2017 at 10:31 AM, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
>> On 3 November 2017 at 15:38, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Fri, Nov 3, 2017 at 6:15 AM, Prathamesh Kulkarni
>>> <prathamesh.kulkarni@linaro.org> wrote:
>>>> Hi Martin,
>>>> As mentioned in PR, the issue here for propagating value of 'm' from
>>>> f_c1 to foo() is that the jump function operation is FLOAT_EXPR, and
>>>> the type of input param 'm' is int, so fold_unary() doesn't do the
>>>> conversion to real_type. The attached patch fixes that by calling
>>>> fold_convert if operation is FLOAT_EXPR / FIX_TRUNC_EXPR /
>>>> CONVERT_EXPR and converts it to the type of corresponding parameter in
>>>> callee.
>>>>
>>>> There are still two issues:
>>>> a) Using NOP_EXPR for early_exit in ipa_get_jf_pass_through_result.
>>>> I suppose we need to change to some other code to indicate that there
>>>> is no operation ?
>>>> b) Patch does not passing param_type from all callers.
>>>> I suppose we could fix these incrementally ?
>>>>
>>>> Bootstrap+tested on x86_64-unknown-linux-gnu.
>>>> OK for trunk ?
>>>
>>> This doesn't look like a well designed fix.  Both fold_unary and fold_binary
>>> calls get a possibly bogus type and you single out only a few ops.
>>>
>>> Either _fully_ list a set of operations that are know to have matching
>>> input/output types or always require param_type to be non-NULL.
>>>
>>> For a) simply remove the special-casing and merge it with CONVERT_EXPR
>>> handling (however it will end up looking).
>>>
>>> Please don't use fold_convert, using fold_unary is fine.
>> Hi Richard,
>> Sorry for the delay. In the attached version, parm_type is made non
>> NULL in ipa_get_jf_pass_through_result().
>>
>> ipa_get_jf_pass_through_result() is called from two
>> places - propagate_vals_across_pass_through() and ipa_value_from_jfunc().
>> However it appears ipa_value_from_jfunc() is called from multiple
>> functions and it's
>> hard to detect parm_type in the individual callers. So I have passed
>> correct parm_type from propagate_vals_across_pass_through(), and kept
>> the old behavior for ipa_value_from_jfunc().
>>
>> Would it be OK to fix that incrementally ?
>
> I don't think it is good to do that.  If we can't get the correct type
> then we have

...I agree with Richi that it is better to fix all the potential type
issues like in the patch below.  Because we now have the types almost
everywhere - notable exceptions are K&R C programs and functions with
variable number of parameters - we might just as well use them, and so I
went over other uses of ipa_get_jf_pass_through_result and made them
look for the appropriate type too.

However, because there are cases where we just do not have the type at
hand, we have to deal with it by only going forward if we know the
operation at hand does not change type.  I have added a special
predicate for this purpose to tree.c but I am opened to suggestions for
better place or name or how/whether to integrate them to gimple
verifier.

Bootstrapped and tested on x86_64-linux.  If the new tree.c predicate is
deemed OK, I'd like to propose to commit this to trunk (and then
backport it to the gcc 7 branch).

What do you think?

Martin


2017-11-27  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>
	    Martin Jambor  <mjambor@suse.cz>

	PR ipa/82808
	* tree.h (tree_operation_preserves_op1_type_p): Declare
	* tree.c (tree_operation_preserves_op1_type_p): New function.
	* ipa-prop.h (ipa_get_type): Allow i to be out of bounds.
	(ipa_value_from_jfunc): Adjust declaration.
	* ipa-cp.c (ipa_get_jf_pass_through_result): New parameter RES_TYPE.
	Use it as result type for arithmetics, unless it is NULL in which case
	be more conservative.
	(ipa_value_from_jfunc): New parameter PARM_TYPE, pass it to
	ipa_get_jf_pass_through_result.
	(propagate_vals_across_pass_through): Likewise.
	(propagate_scalar_across_jump_function): New parameter PARM_TYPE, pass
	is to propagate_vals_across_pass_through.
	(propagate_constants_across_call): Pass PARM_TYPE to
	propagate_scalar_across_jump_function.
	(find_more_scalar_values_for_callers_subset): Pass parameter type to
	ipa_value_from_jfunc.
	(cgraph_edge_brings_all_scalars_for_node): Likewise.
	* ipa-fnsummary.c (evaluate_properties_for_edge): Renamed parms_info
	to caller_parms_info, pass parameter type to ipa_value_from_jfunc.
	* ipa-prop.c (try_make_edge_direct_simple_call): New parameter
	target_type, pass it to ipa_value_from_jfunc.
	(update_indirect_edges_after_inlining): Pass parameter type to
	try_make_edge_direct_simple_call.

testsuite/
	* gcc.dg/ipa/pr82808.c: New test.
---
 gcc/ipa-cp.c                       | 67 +++++++++++++++++++++++---------------
 gcc/ipa-fnsummary.c                | 14 ++++----
 gcc/ipa-prop.c                     | 21 ++++++++----
 gcc/ipa-prop.h                     |  5 +--
 gcc/testsuite/gcc.dg/ipa/pr82808.c | 27 +++++++++++++++
 gcc/tree.c                         | 46 ++++++++++++++++++++++++++
 gcc/tree.h                         |  1 +
 7 files changed, 140 insertions(+), 41 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr82808.c

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 05228d0582d..100a09a46c7 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1220,33 +1220,38 @@ initialize_node_lattices (struct cgraph_node *node)
 }
 
 /* Return the result of a (possibly arithmetic) pass through jump function
-   JFUNC on the constant value INPUT.  Return NULL_TREE if that cannot be
+   JFUNC on the constant value INPUT.  RES_TYPE is the type of the parameter
+   to which the result is passed.  Return NULL_TREE if that cannot be
    determined or be considered an interprocedural invariant.  */
 
 static tree
-ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input)
+ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input,
+				tree res_type)
 {
-  tree restype, res;
+  tree res;
 
   if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
     return input;
   if (!is_gimple_ip_invariant (input))
     return NULL_TREE;
 
-  if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
-      == tcc_unary)
-    res = fold_unary (ipa_get_jf_pass_through_operation (jfunc),
-		      TREE_TYPE (input), input);
-  else
+  tree_code opcode = ipa_get_jf_pass_through_operation (jfunc);
+  if (!res_type)
     {
-      if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
-	  == tcc_comparison)
-	restype = boolean_type_node;
+      if (TREE_CODE_CLASS (opcode) == tcc_comparison)
+	res_type = boolean_type_node;
+      else if (tree_operation_preserves_op1_type_p (opcode))
+	res_type = TREE_TYPE (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));
+	return NULL_TREE;
     }
+
+  if (TREE_CODE_CLASS (opcode) == tcc_unary)
+    res = fold_unary (opcode, res_type, input);
+  else
+    res = fold_binary (opcode, res_type, input,
+		       ipa_get_jf_pass_through_operand (jfunc));
+
   if (res && !is_gimple_ip_invariant (res))
     return NULL_TREE;
 
@@ -1275,10 +1280,12 @@ ipa_get_jf_ancestor_result (struct ipa_jump_func *jfunc, tree input)
 /* Determine whether JFUNC evaluates to a single known constant value and if
    so, return it.  Otherwise return NULL.  INFO describes the caller node or
    the one it is inlined to, so that pass-through jump functions can be
-   evaluated.  */
+   evaluated.  PARM_TYPE is the type of the parameter to which the result is
+   passed.  */
 
 tree
-ipa_value_from_jfunc (struct ipa_node_params *info, struct ipa_jump_func *jfunc)
+ipa_value_from_jfunc (struct ipa_node_params *info, struct ipa_jump_func *jfunc,
+		      tree parm_type)
 {
   if (jfunc->type == IPA_JF_CONST)
     return ipa_get_jf_constant (jfunc);
@@ -1312,7 +1319,7 @@ ipa_value_from_jfunc (struct ipa_node_params *info, struct ipa_jump_func *jfunc)
 	return NULL_TREE;
 
       if (jfunc->type == IPA_JF_PASS_THROUGH)
-	return ipa_get_jf_pass_through_result (jfunc, input);
+	return ipa_get_jf_pass_through_result (jfunc, input, parm_type);
       else
 	return ipa_get_jf_ancestor_result (jfunc, input);
     }
@@ -1562,12 +1569,14 @@ ipcp_lattice<valtype>::add_value (valtype newval, cgraph_edge *cs,
 
 /* Propagate values through a pass-through jump function JFUNC associated with
    edge CS, taking values from SRC_LAT and putting them into DEST_LAT.  SRC_IDX
-   is the index of the source parameter.  */
+   is the index of the source parameter.  PARM_TYPE is the type of the
+   parameter to which the result is passed.  */
 
 static bool
 propagate_vals_across_pass_through (cgraph_edge *cs, ipa_jump_func *jfunc,
 				    ipcp_lattice<tree> *src_lat,
-				    ipcp_lattice<tree> *dest_lat, int src_idx)
+				    ipcp_lattice<tree> *dest_lat, int src_idx,
+				    tree parm_type)
 {
   ipcp_value<tree> *src_val;
   bool ret = false;
@@ -1581,7 +1590,8 @@ propagate_vals_across_pass_through (cgraph_edge *cs, ipa_jump_func *jfunc,
   else
     for (src_val = src_lat->values; src_val; src_val = src_val->next)
       {
-	tree cstval = ipa_get_jf_pass_through_result (jfunc, src_val->value);
+	tree cstval = ipa_get_jf_pass_through_result (jfunc, src_val->value,
+						      parm_type);
 
 	if (cstval)
 	  ret |= dest_lat->add_value (cstval, cs, src_val, src_idx);
@@ -1622,12 +1632,14 @@ propagate_vals_across_ancestor (struct cgraph_edge *cs,
 }
 
 /* Propagate scalar values across jump function JFUNC that is associated with
-   edge CS and put the values into DEST_LAT.  */
+   edge CS and put the values into DEST_LAT.  PARM_TYPE is the type of the
+   parameter to which the result is passed.  */
 
 static bool
 propagate_scalar_across_jump_function (struct cgraph_edge *cs,
 				       struct ipa_jump_func *jfunc,
-				       ipcp_lattice<tree> *dest_lat)
+				       ipcp_lattice<tree> *dest_lat,
+				       tree param_type)
 {
   if (dest_lat->bottom)
     return false;
@@ -1662,7 +1674,7 @@ propagate_scalar_across_jump_function (struct cgraph_edge *cs,
 
       if (jfunc->type == IPA_JF_PASS_THROUGH)
 	ret = propagate_vals_across_pass_through (cs, jfunc, src_lat,
-						  dest_lat, src_idx);
+						  dest_lat, src_idx, param_type);
       else
 	ret = propagate_vals_across_ancestor (cs, jfunc, src_lat, dest_lat,
 					      src_idx);
@@ -2279,7 +2291,8 @@ propagate_constants_across_call (struct cgraph_edge *cs)
       else
 	{
 	  ret |= propagate_scalar_across_jump_function (cs, jump_func,
-							&dest_plats->itself);
+							&dest_plats->itself,
+							param_type);
 	  ret |= propagate_context_across_jump_function (cs, jump_func, i,
 							 &dest_plats->ctxlat);
 	  ret
@@ -3857,6 +3870,7 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
       tree newval = NULL_TREE;
       int j;
       bool first = true;
+      tree type = ipa_get_type (info, i);
 
       if (ipa_get_scalar_lat (info, i)->bottom || known_csts[i])
 	continue;
@@ -3876,7 +3890,7 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
 	      break;
 	    }
 	  jump_func = ipa_get_ith_jump_func (IPA_EDGE_REF (cs), i);
-	  t = ipa_value_from_jfunc (IPA_NODE_REF (cs->caller), jump_func);
+	  t = ipa_value_from_jfunc (IPA_NODE_REF (cs->caller), jump_func, type);
 	  if (!t
 	      || (newval
 		  && !values_equal_for_ipcp_p (t, newval))
@@ -4352,7 +4366,8 @@ cgraph_edge_brings_all_scalars_for_node (struct cgraph_edge *cs,
       if (i >= ipa_get_cs_argument_count (args))
 	return false;
       jump_func = ipa_get_ith_jump_func (args, i);
-      t = ipa_value_from_jfunc (caller_info, jump_func);
+      t = ipa_value_from_jfunc (caller_info, jump_func,
+				ipa_get_type (dest_info, i));
       if (!t || !values_equal_for_ipcp_p (val, t))
 	return false;
     }
diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index 4b5cd70ea85..df8386d9f44 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -444,15 +444,16 @@ evaluate_properties_for_edge (struct cgraph_edge *e, bool inline_p,
       && !e->call_stmt_cannot_inline_p
       && ((clause_ptr && info->conds) || known_vals_ptr || known_contexts_ptr))
     {
-      struct ipa_node_params *parms_info;
+      struct ipa_node_params *caller_parms_info, *callee_pi;
       struct ipa_edge_args *args = IPA_EDGE_REF (e);
       struct ipa_call_summary *es = ipa_call_summaries->get (e);
       int i, count = ipa_get_cs_argument_count (args);
 
       if (e->caller->global.inlined_to)
-	parms_info = IPA_NODE_REF (e->caller->global.inlined_to);
+	caller_parms_info = IPA_NODE_REF (e->caller->global.inlined_to);
       else
-	parms_info = IPA_NODE_REF (e->caller);
+	caller_parms_info = IPA_NODE_REF (e->caller);
+      callee_pi = IPA_NODE_REF (e->callee);
 
       if (count && (info->conds || known_vals_ptr))
 	known_vals.safe_grow_cleared (count);
@@ -464,7 +465,8 @@ evaluate_properties_for_edge (struct cgraph_edge *e, bool inline_p,
       for (i = 0; i < count; i++)
 	{
 	  struct ipa_jump_func *jf = ipa_get_ith_jump_func (args, i);
-	  tree cst = ipa_value_from_jfunc (parms_info, jf);
+	  tree cst = ipa_value_from_jfunc (caller_parms_info, jf,
+					   ipa_get_type (callee_pi, i));
 
 	  if (!cst && e->call_stmt
 	      && i < (int)gimple_call_num_args (e->call_stmt))
@@ -483,8 +485,8 @@ evaluate_properties_for_edge (struct cgraph_edge *e, bool inline_p,
 	    known_vals[i] = error_mark_node;
 
 	  if (known_contexts_ptr)
-	    (*known_contexts_ptr)[i] = ipa_context_from_jfunc (parms_info, e,
-							       i, jf);
+	    (*known_contexts_ptr)[i]
+	      = ipa_context_from_jfunc (caller_parms_info, e, i, jf);
 	  /* TODO: When IPA-CP starts propagating and merging aggregate jump
 	     functions, use its knowledge of the caller too, just like the
 	     scalar case above.  */
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 4b3c2361418..31879a747c0 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3207,19 +3207,20 @@ try_decrement_rdesc_refcount (struct ipa_jump_func *jfunc)
 
 /* Try to find a destination for indirect edge IE that corresponds to a simple
    call or a call of a member function pointer and where the destination is a
-   pointer formal parameter described by jump function JFUNC.  If it can be
-   determined, return the newly direct edge, otherwise return NULL.
+   pointer formal parameter described by jump function JFUNC.  TARGET_TYPE is
+   the type of the parameter to which the result of JFUNC is passed.  If it can
+   be determined, return the newly direct edge, otherwise return NULL.
    NEW_ROOT_INFO is the node info that JFUNC lattices are relative to.  */
 
 static struct cgraph_edge *
 try_make_edge_direct_simple_call (struct cgraph_edge *ie,
-				  struct ipa_jump_func *jfunc,
+				  struct ipa_jump_func *jfunc, tree target_type,
 				  struct ipa_node_params *new_root_info)
 {
   struct cgraph_edge *cs;
   tree target;
   bool agg_contents = ie->indirect_info->agg_contents;
-  tree scalar = ipa_value_from_jfunc (new_root_info, jfunc);
+  tree scalar = ipa_value_from_jfunc (new_root_info, jfunc, target_type);
   if (agg_contents)
     {
       bool from_global_constant;
@@ -3397,7 +3398,7 @@ update_indirect_edges_after_inlining (struct cgraph_edge *cs,
 {
   struct ipa_edge_args *top;
   struct cgraph_edge *ie, *next_ie, *new_direct_edge;
-  struct ipa_node_params *new_root_info;
+  struct ipa_node_params *new_root_info, *inlined_node_info;
   bool res = false;
 
   ipa_check_create_edge_args ();
@@ -3405,6 +3406,7 @@ update_indirect_edges_after_inlining (struct cgraph_edge *cs,
   new_root_info = IPA_NODE_REF (cs->caller->global.inlined_to
 				? cs->caller->global.inlined_to
 				: cs->caller);
+  inlined_node_info = IPA_NODE_REF (cs->callee->function_symbol ());
 
   for (ie = node->indirect_calls; ie; ie = next_ie)
     {
@@ -3445,8 +3447,13 @@ update_indirect_edges_after_inlining (struct cgraph_edge *cs,
 	  new_direct_edge = try_make_edge_direct_virtual_call (ie, jfunc, ctx);
 	}
       else
-	new_direct_edge = try_make_edge_direct_simple_call (ie, jfunc,
-							    new_root_info);
+	{
+	  tree target_type =  ipa_get_type (inlined_node_info, param_index);
+	  new_direct_edge = try_make_edge_direct_simple_call (ie, jfunc,
+							      target_type,
+							      new_root_info);
+	}
+
       /* If speculation was removed, then we need to do nothing.  */
       if (new_direct_edge && new_direct_edge != ie
 	  && new_direct_edge->callee == spec_target)
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index bd8ae3e8dae..c3624052bbc 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -464,7 +464,8 @@ ipa_get_param (struct ipa_node_params *info, int i)
 static inline tree
 ipa_get_type (struct ipa_node_params *info, int i)
 {
-  gcc_checking_assert (info->descriptors);
+  if (vec_safe_length (info->descriptors) <= (unsigned) i)
+    return NULL;
   tree t = (*info->descriptors)[i].decl_or_type;
   if (!t)
     return NULL;
@@ -773,7 +774,7 @@ void ipcp_write_transformation_summaries (void);
 void ipcp_read_transformation_summaries (void);
 int ipa_get_param_decl_index (struct ipa_node_params *, tree);
 tree ipa_value_from_jfunc (struct ipa_node_params *info,
-			   struct ipa_jump_func *jfunc);
+			   struct ipa_jump_func *jfunc, tree type);
 unsigned int ipcp_transform_function (struct cgraph_node *node);
 ipa_polymorphic_call_context ipa_context_from_jfunc (ipa_node_params *,
 						     cgraph_edge *,
diff --git a/gcc/testsuite/gcc.dg/ipa/pr82808.c b/gcc/testsuite/gcc.dg/ipa/pr82808.c
new file mode 100644
index 00000000000..9c95d0b6ed7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr82808.c
@@ -0,0 +1,27 @@
+/* { dg-options "-O2" } */
+/* { dg-do run } */
+
+static void __attribute__((noinline))
+foo (double *a, double x)
+{
+  *a = x;
+}
+
+static double  __attribute__((noinline))
+f_c1 (int m, double *a)
+{
+  foo (a, m);
+  return *a;
+}
+
+int
+main (){
+  double data;
+  double ret = 0 ;
+
+  if ((ret = f_c1 (2, &data)) != 2)
+    {
+      __builtin_abort ();
+    }
+  return 0;
+}
diff --git a/gcc/tree.c b/gcc/tree.c
index 7efd644fb27..2a25c657f8b 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -13893,6 +13893,52 @@ arg_size_in_bytes (const_tree type)
   return TYPE_EMPTY_P (type) ? size_zero_node : size_in_bytes (type);
 }
 
+/* Return true if unary or binary operation specified with CODE has to have the
+   same result type as its first operand.  */
+
+bool
+tree_operation_preserves_op1_type_p (tree_code code)
+{
+  gcc_checking_assert (TREE_CODE_CLASS (code) == tcc_unary
+		       || TREE_CODE_CLASS (code) == tcc_binary);
+  switch (code)
+    {
+    case NEGATE_EXPR:
+    case ABS_EXPR:
+    case BIT_NOT_EXPR:
+    case PAREN_EXPR:
+    case CONJ_EXPR:
+
+    case PLUS_EXPR:
+    case MINUS_EXPR:
+    case MULT_EXPR:
+    case TRUNC_DIV_EXPR:
+    case CEIL_DIV_EXPR:
+    case FLOOR_DIV_EXPR:
+    case ROUND_DIV_EXPR:
+    case TRUNC_MOD_EXPR:
+    case CEIL_MOD_EXPR:
+    case FLOOR_MOD_EXPR:
+    case ROUND_MOD_EXPR:
+    case RDIV_EXPR:
+    case EXACT_DIV_EXPR:
+    case MIN_EXPR:
+    case MAX_EXPR:
+    case BIT_IOR_EXPR:
+    case BIT_XOR_EXPR:
+    case BIT_AND_EXPR:
+
+    case LSHIFT_EXPR:
+    case RSHIFT_EXPR:
+    case LROTATE_EXPR:
+    case RROTATE_EXPR:
+      return true;
+
+    default:
+      return false;
+    }
+}
+
 /* List of pointer types used to declare builtins before we have seen their
    real declaration.
 
diff --git a/gcc/tree.h b/gcc/tree.h
index c2cabfc7529..5183f3da42a 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5457,6 +5457,7 @@ extern bool is_redundant_typedef (const_tree);
 extern bool default_is_empty_record (const_tree);
 extern HOST_WIDE_INT arg_int_size_in_bytes (const_tree);
 extern tree arg_size_in_bytes (const_tree);
+extern bool tree_operation_preserves_op1_type_p (tree_code);
 
 extern location_t
 set_source_range (tree expr, location_t start, location_t finish);
-- 
2.15.0