| Message ID | CAAgBjMmAJPxAYomqv6tx0rdnrpbH=ak7=91A+n05x-mSjddXMA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR82808 | expand |
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
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; +}
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
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 > > >
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
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
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; +}