PR82808

Message ID CAAgBjMmAJPxAYomqv6tx0rdnrpbH=ak7=91A+n05x-mSjddXMA@mail.gmail.com
State New
Headers show
Series
  • PR82808
Related show

Commit Message

Prathamesh Kulkarni Nov. 3, 2017, 5:15 a.m.
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 ?

Thanks,
Prathamesh
2017-11-03  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

	* ipa-cp.c (ipa_get_jf_pass_through_result): Add new parameter
	parm_type with default value set to NULL. Call fold_convert if jfunc
	operation is FLOAT_EXPR or FIX_TRUNC_EXPR or CONVERT_EXPR.
	(propagate_vals_across_pass_through): Add parameter parm_type.
	(propagate_scalar_across_jump_function): Add parameter parm_type and
	pass it to propagate_vals_across_pass_through.
	(propagate_constants_across_call): Pass param_type to
	propagate_scalar_across_jump_function.

testsuite/
	* gcc.dg/ipa/pr82808.c: New test.

Comments

Richard Biener Nov. 3, 2017, 10:08 a.m. | #1
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.

Thanks,
Richard.

> Thanks,

> Prathamesh
Prathamesh Kulkarni Nov. 14, 2017, 9:31 a.m. | #2
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 ?
Patch is bootstrapped+tested on x86_64-unknown-linux-gnu.

Thanks,
Prathamesh
>

> Thanks,

> Richard.

>

>> Thanks,

>> Prathamesh
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index bc1e3ae799d..c7b67e6d007 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1224,7 +1224,8 @@ initialize_node_lattices (struct cgraph_node *node)
    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 parm_type)
 {
   tree restype, res;
 
@@ -1233,10 +1234,12 @@ ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input)
   if (!is_gimple_ip_invariant (input))
     return NULL_TREE;
 
+  gcc_assert (parm_type != 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);
+		      parm_type, input);
   else
     {
       if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
@@ -1312,7 +1315,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, TREE_TYPE (input));
       else
 	return ipa_get_jf_ancestor_result (jfunc, input);
     }
@@ -1567,7 +1570,8 @@ ipcp_lattice<valtype>::add_value (valtype newval, cgraph_edge *cs,
 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 +1585,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);
@@ -1627,7 +1632,8 @@ propagate_vals_across_ancestor (struct cgraph_edge *cs,
 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 +1668,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 +2285,7 @@ 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
diff --git a/gcc/testsuite/gcc.dg/ipa/pr82808.c b/gcc/testsuite/gcc.dg/ipa/pr82808.c
new file mode 100644
index 00000000000..e9a90e3ce2e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr82808.c
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -flto -fno-inline" } */
+
+void foo(double *a, double x)
+{
+  *a = x;
+}
+
+double 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;
+}
Richard Biener Nov. 14, 2017, 12:47 p.m. | #3
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
to only support known-good operations.  There's the ipa_node_params->descriptors
array where the type should be attached to, no?

So use TREE_TYPE (ipa_get_param (info, idx))?

Other than that Martin should have a look as I'm not really familiar
with this IPA API.

Richard.

> Patch is bootstrapped+tested on x86_64-unknown-linux-gnu.

>

> Thanks,

> Prathamesh

>>

>> Thanks,

>> Richard.

>>

>>> Thanks,

>>> Prathamesh
Richard Biener Nov. 28, 2017, 11:46 a.m. | #4
On Tue, Nov 28, 2017 at 12:35 AM, Martin Jambor <mjambor@suse.cz> wrote:
> 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?


Ok with nits below...

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


expr_type_first_operand_type_p ()

> +{

> +  gcc_checking_assert (TREE_CODE_CLASS (code) == tcc_unary

> +                      || TREE_CODE_CLASS (code) == tcc_binary);


Drop this assert, there are at least tcc_expression codes we might want
to handle in the future, the default: return false should be a good fallback.
Adjust the function comment accordingly.

Ok with those changes.

Thanks,
Richard.

> +  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

>

>

>
Martin Jambor Nov. 28, 2017, 6:54 p.m. | #5
Hi,

On Tue, Nov 28 2017, Richard Biener wrote:
> On Tue, Nov 28, 2017 at 12:35 AM, Martin Jambor <mjambor@suse.cz> wrote:


...

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

>

> expr_type_first_operand_type_p ()


Done.

>

>> +{

>> +  gcc_checking_assert (TREE_CODE_CLASS (code) == tcc_unary

>> +                      || TREE_CODE_CLASS (code) == tcc_binary);

>

> Drop this assert, there are at least tcc_expression codes we might want

> to handle in the future, the default: return false should be a good fallback.

> Adjust the function comment accordingly.


Done, this is what I have just committed.  I will prepare a conservative
fix for gcc 7 with only the expr_type_first_operand_type_p part.

Thanks a lot,

Martin

[PR 82808] Use proper result types for arithmetic jump functions

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

	PR ipa/82808
	* tree.h (expr_type_first_operand_type_p): Declare
	* tree.c (expr_type_first_operand_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                         | 44 +++++++++++++++++++++++++
 gcc/tree.h                         |  1 +
 7 files changed, 138 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..aa9e300d378 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 (expr_type_first_operand_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..fcb2bb02a5f 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -13893,6 +13893,50 @@ arg_size_in_bytes (const_tree type)
   return TYPE_EMPTY_P (type) ? size_zero_node : size_in_bytes (type);
 }
 
+/* Return true if an expression with CODE has to have the same result type as
+   its first operand.  */
+
+bool
+expr_type_first_operand_type_p (tree_code code)
+{
+  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..e579cbc0097 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 expr_type_first_operand_type_p (tree_code);
 
 extern location_t
 set_source_range (tree expr, location_t start, location_t finish);
-- 
2.15.0
Martin Jambor Nov. 29, 2017, 10:17 p.m. | #6
On Tue, Nov 28 2017, Martin Jambor wrote:
>

...
>

> Done, this is what I have just committed.  I will prepare a conservative

> fix for gcc 7 with only the expr_type_first_operand_type_p part.

>


The following is what I have committed to the gcc-7-branch after a round
of bootstrap and testing on an x86_64-linux.

Thanks,

Martin


2017-11-29  Martin Jambor  <mjambor@suse.cz>

	PR ipa/82808
	* tree.c (expr_type_first_operand_type_p): New function.
	* tree.h (expr_type_first_operand_type_p): Declare it.
	* ipa-cp.c (ipa_get_jf_pass_through_result): Use it.

testsuite/
	* gcc.dg/ipa/pr82808.c: New test.
---
 gcc/ipa-cp.c                       | 26 +++++++++++-----------
 gcc/testsuite/gcc.dg/ipa/pr82808.c | 27 +++++++++++++++++++++++
 gcc/tree.c                         | 44 ++++++++++++++++++++++++++++++++++++++
 gcc/tree.h                         |  1 +
 4 files changed, 85 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr82808.c

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index fa3d5fd7548..716c8cc3a1f 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1224,20 +1224,20 @@ ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree 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);
+  tree_code opcode = ipa_get_jf_pass_through_operation (jfunc);
+  if (TREE_CODE_CLASS (opcode) == tcc_comparison)
+    restype = boolean_type_node;
+  else if (expr_type_first_operand_type_p (opcode))
+    restype = TREE_TYPE (input);
   else
-    {
-      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));
-    }
+    return NULL_TREE;
+
+  if (TREE_CODE_CLASS (opcode) == tcc_unary)
+    res = fold_unary (opcode, restype, input);
+  else
+    res = fold_binary (opcode, restype, input,
+		       ipa_get_jf_pass_through_operand (jfunc));
+
   if (res && !is_gimple_ip_invariant (res))
     return NULL_TREE;
 
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 69425ab59ee..698213c3501 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -14515,6 +14515,50 @@ get_nonnull_args (const_tree fntype)
   return argmap;
 }
 
+/* Return true if an expression with CODE has to have the same result type as
+   its first operand.  */
+
+bool
+expr_type_first_operand_type_p (tree_code code)
+{
+  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;
+    }
+}
+
 #if CHECKING_P
 
 namespace selftest {
diff --git a/gcc/tree.h b/gcc/tree.h
index 375edcd4ba6..f20b77f17e4 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5471,6 +5471,7 @@ extern void gt_pch_nx (tree &, gt_pointer_operator, void *);
 
 extern bool nonnull_arg_p (const_tree);
 extern bool is_redundant_typedef (const_tree);
+extern bool expr_type_first_operand_type_p (tree_code);
 
 extern location_t
 set_source_range (tree expr, location_t start, location_t finish);
-- 
2.15.0

Patch

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 6b3d8d7364c..20328a43f9b 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1224,7 +1224,8 @@  initialize_node_lattices (struct cgraph_node *node)
    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 parm_type = NULL_TREE)
 {
   tree restype, res;
 
@@ -1233,7 +1234,17 @@  ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input)
   if (!is_gimple_ip_invariant (input))
     return NULL_TREE;
 
-  if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
+  if (ipa_get_jf_pass_through_operation (jfunc) == FLOAT_EXPR
+      || ipa_get_jf_pass_through_operation (jfunc) == FIX_TRUNC_EXPR
+      || ipa_get_jf_pass_through_operation (jfunc) == CONVERT_EXPR)
+    {
+      if (!parm_type)
+	return NULL_TREE;
+
+      res = fold_convert (parm_type, input);
+      restype = parm_type;
+    }
+  else 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);
@@ -1567,7 +1578,8 @@  ipcp_lattice<valtype>::add_value (valtype newval, cgraph_edge *cs,
 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 +1593,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);
@@ -1627,7 +1640,8 @@  propagate_vals_across_ancestor (struct cgraph_edge *cs,
 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 +1676,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 +2293,7 @@  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
diff --git a/gcc/testsuite/gcc.dg/ipa/pr82808.c b/gcc/testsuite/gcc.dg/ipa/pr82808.c
new file mode 100644
index 00000000000..e9a90e3ce2e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr82808.c
@@ -0,0 +1,24 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -flto -fno-inline" } */
+
+void foo(double *a, double x)
+{
+  *a = x;
+}
+
+double 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;
+}