| Message ID | b2c492eb-31ad-4071-8666-3a569479b424@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Hi, On Fri, Dec 09, 2016 at 03:36:44PM +1100, kugan wrote: > On 07/12/16 21:08, Martin Jambor wrote: > > On Mon, Nov 28, 2016 at 04:25:00PM +1100, kugan wrote: > > > > ... > > > > > Here is a patch that does this. To fox PR78365, in > > > ipa_get_callee_param_type, I am now checking DECL_ARGUMENTS first. I lto > > > bootstrapped and regression tested on x86_64-linux-gnu and ppc64le-linux > > > with no new regressions. I will build Firefox and measure the memory usage > > > as Honza suggested based on the feedback. > > > > > > > So, do you have any numbers? > I will do it. How do you measure the gcc's memory usage while building > Firefox. I saw Honza's LTO blog talks about using vmstat result. Any tips > please? I asked Martin Liska how he does it and he replied with: Steps: 1) clone git repository: https://github.com/marxin/script-misc 2) Run command that does an LTO linking 3) ./system_top.py > /tmp/log Run that in a separate terminal, terminate the script after the LTO linking finishes. 4) ./vmstat_parser.py /tmp/log Plot the collected data. so try that :-) As far as I know, it uses vmstat. Or maybe Honza has some other method. > > ... > > > > > -tree > > > +static tree > > > ipa_get_callee_param_type (struct cgraph_edge *e, int i) > > > { > > > int n; > > > + tree t = e->callee ? DECL_ARGUMENTS (e->callee->decl) : NULL_TREE; > > > + if (t) > > > + for (n = 0; n < i; n++) > > > + { > > > + if (!t) > > > + return NULL; > > > + t = TREE_CHAIN (t); > > > + } > > > + if (t) > > > + return TREE_TYPE (t); > > > tree type = (e->callee > > > ? TREE_TYPE (e->callee->decl) > > > : gimple_call_fntype (e->call_stmt)); > > > - tree t = TYPE_ARG_TYPES (type); > > > + t = TYPE_ARG_TYPES (type); > > > > If TYPE_ARG_TYPES is inherently unreliable then I am afraid you must > > not fallback on it but rather return NULL if cs->callee is not > > available and adjust the caller to give up in that case. > > > > (I have checked both testcases that we hope to fix with types in jump > > functions and the gimple_call_fntype is the same as > > TREE_TYPE(e->callee->decl) so that does not help either). > > Do you like the attached patch where I have removed it. > I am fine with the new patch but you'll need an approval from Honza or Richi. I find it a bit saddening that we cannot really rely on gimple_call_fntype but at least I do not see any other way. Thanks, Martin
On Fri, Dec 9, 2016 at 11:51 AM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > On Fri, Dec 09, 2016 at 03:36:44PM +1100, kugan wrote: >> On 07/12/16 21:08, Martin Jambor wrote: >> > On Mon, Nov 28, 2016 at 04:25:00PM +1100, kugan wrote: >> > >> > ... >> > >> > > Here is a patch that does this. To fox PR78365, in >> > > ipa_get_callee_param_type, I am now checking DECL_ARGUMENTS first. I lto >> > > bootstrapped and regression tested on x86_64-linux-gnu and ppc64le-linux >> > > with no new regressions. I will build Firefox and measure the memory usage >> > > as Honza suggested based on the feedback. >> > > >> > >> > So, do you have any numbers? >> I will do it. How do you measure the gcc's memory usage while building >> Firefox. I saw Honza's LTO blog talks about using vmstat result. Any tips >> please? > > I asked Martin Liska how he does it and he replied with: > > Steps: > > 1) clone git repository: https://github.com/marxin/script-misc > > 2) Run command that does an LTO linking > > 3) ./system_top.py > /tmp/log > Run that in a separate terminal, terminate the script after the LTO linking > finishes. > > 4) ./vmstat_parser.py /tmp/log > Plot the collected data. > > so try that :-) As far as I know, it uses vmstat. Or maybe Honza > has some other method. > > >> > ... >> > >> > > -tree >> > > +static tree >> > > ipa_get_callee_param_type (struct cgraph_edge *e, int i) >> > > { >> > > int n; >> > > + tree t = e->callee ? DECL_ARGUMENTS (e->callee->decl) : NULL_TREE; >> > > + if (t) >> > > + for (n = 0; n < i; n++) >> > > + { >> > > + if (!t) >> > > + return NULL; >> > > + t = TREE_CHAIN (t); >> > > + } >> > > + if (t) >> > > + return TREE_TYPE (t); >> > > tree type = (e->callee >> > > ? TREE_TYPE (e->callee->decl) >> > > : gimple_call_fntype (e->call_stmt)); >> > > - tree t = TYPE_ARG_TYPES (type); >> > > + t = TYPE_ARG_TYPES (type); >> > >> > If TYPE_ARG_TYPES is inherently unreliable then I am afraid you must >> > not fallback on it but rather return NULL if cs->callee is not >> > available and adjust the caller to give up in that case. >> > >> > (I have checked both testcases that we hope to fix with types in jump >> > functions and the gimple_call_fntype is the same as >> > TREE_TYPE(e->callee->decl) so that does not help either). >> >> Do you like the attached patch where I have removed it. >> > > I am fine with the new patch but you'll need an approval from Honza > or Richi. > > I find it a bit saddening that we cannot really rely on > gimple_call_fntype but at least I do not see any other way. The patch looks somewhat backward. It populates the param type from the callee but the only thing we really know is the _originating_ type from the callers DECL_ARGUMENTS (if the JF is based on a parameter which is the only case where we do not know the type of the actual value statically -- we only know the type we expect). So the type would be present only on IPA_JF_PASS_THROUGH JFs (? does that also cover modified params?). At propagation time when applying the jump-function we have to make sure to first convert the actual parameter value to that expected type (or drop to BOTTOM if we can't do that conversion due to excess mismatches). Richard. > Thanks, > > Martin
On Wed, Dec 14, 2016 at 11:15 AM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > On Fri, Dec 09, 2016 at 01:18:25PM +0100, Richard Biener wrote: >> >> The patch looks somewhat backward. It populates the param type from >> the callee but the only thing we really know is the _originating_ type >> from the callers DECL_ARGUMENTS (if the JF is based on a parameter >> which is the only case where we do not know the type of the actual >> value statically -- we only know the type we expect). >> >> So the type would be present only on IPA_JF_PASS_THROUGH JFs >> (? does that also cover modified params?). >> >> At propagation time when applying the jump-function we have to make >> sure to first convert the actual parameter value to that expected type >> (or drop to BOTTOM if we can't do that conversion due to excess mismatches). >> > > I wanted to have a look at this myself for some time but the omp-low > splitting took far more time than I had anticipated and so it took my > until yesterday evening to come up with "my fix" which you can find > below. It has the added benefit of also fixing PR 78599. > > The patch removes the LTO WPA attempt to figure out types of actual > arguments from TYPE_ARG_TYPES and instead it adds streaming them into > node summaries, i.e. they come from the compile time unit with the > actual function definition (as opposed to the information we have at > call-sites when building jump functions). > > Because in various corner cases and plain-garbage-code cases the types > of the actual and formal arguments can be quite different, especially > with LTO, I have reorganized propagate_vr_accross_jump_function so > that it uses extract_range_from_unary_expr in all but the > IPA_JF_PASS_THROUGH cases, because those rely on fold_convert. > > In order to have all the information for > extract_range_from_unary_expr, I needed to stream also the type of the > actual argument, which hitherto we did not have for the cases when the > jump function was IPA_JF_UNKNOWN but still had vr_known set (and thus > did contain any info for constant propagation but still some info > about VR). I am not happy about adding another field to this data > structure but at this point see not way around it. > > Perhaps propagate_vr_accross_jump_function can now be simplified > further, running extract_range_from_unary_expr pretty much always (or > only when the types are not useless_type_conversion_p). But I think I > should post what I have now. The patch passes bootstrap, lto-boostrap > (of C, C++ and Fortran) and testing on x86_64-linux. > > What do you think? > > Martin > > 2016-12-13 Martin Jambor <mjambor@suse.cz> > > PR ipa/78365 > * ipa-prop.h (ipa_jump_func): New field pased_type. > * ipa-cp.c (ipa_vr_operation_and_type_effects): New function. > (propagate_vr_accross_jump_function): Use the above function for all > value range computations for pass-through jump functions and type > converasion from explicit value range values. > (ipcp_propagate_stage): Do not attempt to deduce types of formal > parameters from TYPE_ARG_TYPES. > * ipa-prop.c (ipa_compute_jump_functions_for_edge): Set > jfunc->passed_type whenever setting jfunc->vr_known to true. > (ipa_write_jump_function): Remove trailing whitespace, stream > passed_type. > (ipa_read_jump_function): Stream passed_type. > (ipa_write_node_info): Stream type of the actual argument. > (ipa_read_node_info): Likewise. Also remove trailing whitespace. > > testsuite/ > * gcc.dg/torture/pr78365.c: New test. > --- > gcc/ipa-cp.c | 70 +++++++++++++++++----------------- > gcc/ipa-prop.c | 22 ++++++++--- > gcc/ipa-prop.h | 3 ++ > gcc/testsuite/gcc.dg/torture/pr78365.c | 21 ++++++++++ > 4 files changed, 75 insertions(+), 41 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr78365.c > > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > index 4ec7cc5..7e6fc9a 100644 > --- a/gcc/ipa-cp.c > +++ b/gcc/ipa-cp.c > @@ -1839,6 +1839,23 @@ propagate_bits_accross_jump_function (cgraph_edge *cs, int idx, ipa_jump_func *j > return dest_lattice->set_to_bottom (); > } > > +/* Emulate effects of unary OPERATION and/or conversion from SRC_TYPE to > + DST_TYPE on value range in SRC_VR and store it to DST_VR. Return true if > + the result is a range or an anti-range. */ > + > +static bool > +ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr, > + enum tree_code operation, > + tree dst_type, tree src_type) > +{ > + memset (dst_vr, 0, sizeof (*dst_vr)); The memset is not necessary. > + extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type); > + if (dst_vr->type == VR_RANGE || dst_vr->type == VR_ANTI_RANGE) > + return true; > + else > + return false; > +} > + > /* Propagate value range across jump function JFUNC that is associated with > edge CS with param of callee of PARAM_TYPE and update DEST_PLATS > accordingly. */ > @@ -1849,7 +1866,6 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, > struct ipcp_param_lattices *dest_plats, > tree param_type) > { > - struct ipcp_param_lattices *src_lats; > ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range; > > if (dest_lat->bottom_p ()) > @@ -1862,31 +1878,23 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, > > if (jfunc->type == IPA_JF_PASS_THROUGH) > { > - struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); > - int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); > - src_lats = ipa_get_parm_lattices (caller_info, src_idx); > + enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); > > - if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR) > - return dest_lat->meet_with (src_lats->m_value_range); > - else if (param_type > - && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc)) > - == tcc_unary)) > + if (TREE_CODE_CLASS (operation) == tcc_unary) > { > - value_range vr; > - memset (&vr, 0, sizeof (vr)); > + struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); > + int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); > tree operand_type = ipa_get_type (caller_info, src_idx); > - enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); > + struct ipcp_param_lattices *src_lats > + = ipa_get_parm_lattices (caller_info, src_idx); > > if (src_lats->m_value_range.bottom_p ()) > return dest_lat->set_to_bottom (); > - > - extract_range_from_unary_expr (&vr, > - operation, > - param_type, > - &src_lats->m_value_range.m_vr, > - operand_type); > - if (vr.type == VR_RANGE > - || vr.type == VR_ANTI_RANGE) > + value_range vr; > + if (ipa_vr_operation_and_type_effects (&vr, > + &src_lats->m_value_range.m_vr, > + operation, param_type, > + operand_type)) > return dest_lat->meet_with (&vr); > } > } > @@ -1906,8 +1914,11 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, > } > } > > - if (jfunc->vr_known) > - return dest_lat->meet_with (&jfunc->m_vr); > + value_range vr; > + if (jfunc->vr_known > + && ipa_vr_operation_and_type_effects (&vr, &jfunc->m_vr, NOP_EXPR, > + param_type, jfunc->passed_type)) but instead of a new jfunc->passed_type you can use TREE_TYPE (jfunc->m_vr.min) for example. I notice that ipa_jump_func is badly laid out: struct GTY (()) ipa_jump_func { /* Aggregate contants description. See struct ipa_agg_jump_function and its description. */ struct ipa_agg_jump_function agg; /* Information about zero/non-zero bits. */ struct ipa_bits bits; /* Information about value range. */ bool vr_known; value_range m_vr; enum jump_func_type type; /* Represents a value of a jump function. pass_through is used only in jump function context. constant represents the actual constant in constant jump functions and member_cst holds constant c++ member functions. */ union jump_func_value { struct ipa_constant_data GTY ((tag ("IPA_JF_CONST"))) constant; struct ipa_pass_through_data GTY ((tag ("IPA_JF_PASS_THROUGH"))) pass_through; struct ipa_ancestor_jf_data GTY ((tag ("IPA_JF_ANCESTOR"))) ancestor; } GTY ((desc ("%1.type"))) value; }; vr_known should be moved to pack with type. and ipa_bits::known should be moved out of it as well. I also think we do not use the equiv member of m_vr and thus that is a waste. Not sure if memory use of this struct is an issue. Richard. > + return dest_lat->meet_with (&vr); > else > return dest_lat->set_to_bottom (); > } > @@ -2247,7 +2258,7 @@ propagate_constants_accross_call (struct cgraph_edge *cs) > { > struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i); > struct ipcp_param_lattices *dest_plats; > - tree param_type = ipa_get_callee_param_type (cs, i); > + tree param_type = ipa_get_type (callee_info, i); > > dest_plats = ipa_get_parm_lattices (callee_info, i); > if (availability == AVAIL_INTERPOSABLE) > @@ -3234,19 +3245,6 @@ ipcp_propagate_stage (struct ipa_topo_info *topo) > { > struct ipa_node_params *info = IPA_NODE_REF (node); > > - /* In LTO we do not have PARM_DECLs but we would still like to be able to > - look at types of parameters. */ > - if (in_lto_p) > - { > - tree t = TYPE_ARG_TYPES (TREE_TYPE (node->decl)); > - for (int k = 0; k < ipa_get_param_count (info) && t; k++) > - { > - gcc_assert (t != void_list_node); > - info->descriptors[k].decl_or_type = TREE_VALUE (t); > - t = t ? TREE_CHAIN (t) : NULL; > - } > - } > - > determine_versionability (node, info); > if (node->has_gimple_body_p ()) > { > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c > index 642111d..123d422 100644 > --- a/gcc/ipa-prop.c > +++ b/gcc/ipa-prop.c > @@ -1751,6 +1751,7 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi, > jfunc->m_vr.min = build_int_cst (TREE_TYPE (arg), 0); > jfunc->m_vr.max = build_int_cst (TREE_TYPE (arg), 0); > jfunc->m_vr.equiv = NULL; > + jfunc->passed_type = TREE_TYPE (arg); > } > else > gcc_assert (!jfunc->vr_known); > @@ -1776,7 +1777,10 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi, > &vr, TREE_TYPE (arg)); > if (jfunc->m_vr.type == VR_RANGE > || jfunc->m_vr.type == VR_ANTI_RANGE) > - jfunc->vr_known = true; > + { > + jfunc->vr_known = true; > + jfunc->passed_type = TREE_TYPE (arg); > + } > else > jfunc->vr_known = false; > } > @@ -4775,7 +4779,7 @@ ipa_write_jump_function (struct output_block *ob, > { > streamer_write_widest_int (ob, jump_func->bits.value); > streamer_write_widest_int (ob, jump_func->bits.mask); > - } > + } > bp_pack_value (&bp, jump_func->vr_known, 1); > streamer_write_bitpack (&bp); > if (jump_func->vr_known) > @@ -4784,6 +4788,7 @@ ipa_write_jump_function (struct output_block *ob, > VR_LAST, jump_func->m_vr.type); > stream_write_tree (ob, jump_func->m_vr.min, true); > stream_write_tree (ob, jump_func->m_vr.max, true); > + stream_write_tree (ob, jump_func->passed_type, true); > } > } > > @@ -4877,6 +4882,7 @@ ipa_read_jump_function (struct lto_input_block *ib, > VR_LAST); > jump_func->m_vr.min = stream_read_tree (ib, data_in); > jump_func->m_vr.max = stream_read_tree (ib, data_in); > + jump_func->passed_type = stream_read_tree (ib, data_in); > } > else > jump_func->vr_known = false; > @@ -4973,7 +4979,10 @@ ipa_write_node_info (struct output_block *ob, struct cgraph_node *node) > bp_pack_value (&bp, ipa_is_param_used (info, j), 1); > streamer_write_bitpack (&bp); > for (j = 0; j < ipa_get_param_count (info); j++) > - streamer_write_hwi (ob, ipa_get_controlled_uses (info, j)); > + { > + streamer_write_hwi (ob, ipa_get_controlled_uses (info, j)); > + stream_write_tree (ob, ipa_get_type (info, j), true); > + } > for (e = node->callees; e; e = e->next_callee) > { > struct ipa_edge_args *args = IPA_EDGE_REF (e); > @@ -5020,7 +5029,7 @@ ipa_read_node_info (struct lto_input_block *ib, struct cgraph_node *node, > > for (k = 0; k < ipa_get_param_count (info); k++) > info->descriptors[k].move_cost = streamer_read_uhwi (ib); > - > + > bp = streamer_read_bitpack (ib); > if (ipa_get_param_count (info) != 0) > info->analysis_done = true; > @@ -5028,7 +5037,10 @@ ipa_read_node_info (struct lto_input_block *ib, struct cgraph_node *node, > for (k = 0; k < ipa_get_param_count (info); k++) > ipa_set_param_used (info, k, bp_unpack_value (&bp, 1)); > for (k = 0; k < ipa_get_param_count (info); k++) > - ipa_set_controlled_uses (info, k, streamer_read_hwi (ib)); > + { > + ipa_set_controlled_uses (info, k, streamer_read_hwi (ib)); > + info->descriptors[k].decl_or_type = stream_read_tree (ib, data_in); > + } > for (e = node->callees; e; e = e->next_callee) > { > struct ipa_edge_args *args = IPA_EDGE_REF (e); > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index 0e75cf4..e4de8ed 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -181,6 +181,9 @@ struct GTY (()) ipa_jump_func > /* Information about value range. */ > bool vr_known; > value_range m_vr; > + /* If value range is known, this is the type in which we pass the date at the > + caller side. */ > + tree passed_type; > > enum jump_func_type type; > /* Represents a value of a jump function. pass_through is used only in jump > diff --git a/gcc/testsuite/gcc.dg/torture/pr78365.c b/gcc/testsuite/gcc.dg/torture/pr78365.c > new file mode 100644 > index 0000000..5180a01 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr78365.c > @@ -0,0 +1,21 @@ > +/* { dg-do compile } */ > + > +int a, b, c; > +char d; > +static void fn1 (void *, int); > +int fn2 (int); > + > +void fn1 (cc, yh) void *cc; > +char yh; > +{ > + char y; > + a = fn2(c - b + 1); > + for (; y <= yh; y++) > + ; > +} > + > +void fn3() > +{ > + fn1((void *)fn3, 1); > + fn1((void *)fn3, d - 1); > +} > -- > 2.10.2 >
On Fri, Jan 6, 2017 at 7:00 PM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > On Wed, Dec 14, 2016 at 01:12:11PM +0100, Richard Biener wrote: >> On Wed, Dec 14, 2016 at 11:15 AM, Martin Jambor <mjambor@suse.cz> wrote: > >> > ... > >> > +/* Emulate effects of unary OPERATION and/or conversion from SRC_TYPE to >> > + DST_TYPE on value range in SRC_VR and store it to DST_VR. Return true if >> > + the result is a range or an anti-range. */ >> > + >> > +static bool >> > +ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr, >> > + enum tree_code operation, >> > + tree dst_type, tree src_type) >> > +{ >> > + memset (dst_vr, 0, sizeof (*dst_vr)); >> >> The memset is not necessary. > > Apparently it is. Without it, I ended up with corrupted > dst->vr_bitmup. I got ICEs when I removed the memset and tracked it > down to: > > (gdb) p dst_vr->equiv->first->next > $14 = (bitmap_element *) 0x16 > > after extract_range_from_unary_expr returns. Ah, I see that set_value_range_to_* expect properly initialized ->equiv. >> >> > + extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type); >> > + if (dst_vr->type == VR_RANGE || dst_vr->type == VR_ANTI_RANGE) >> > + return true; >> > + else >> > + return false; >> > +} >> > + >> > /* Propagate value range across jump function JFUNC that is associated with >> > edge CS with param of callee of PARAM_TYPE and update DEST_PLATS >> > accordingly. */ >> > @@ -1849,7 +1866,6 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, >> > struct ipcp_param_lattices *dest_plats, >> > tree param_type) >> > { >> > - struct ipcp_param_lattices *src_lats; >> > ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range; >> > >> > if (dest_lat->bottom_p ()) >> > @@ -1862,31 +1878,23 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, >> > >> > if (jfunc->type == IPA_JF_PASS_THROUGH) >> > { >> > - struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); >> > - int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); >> > - src_lats = ipa_get_parm_lattices (caller_info, src_idx); >> > + enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); >> > >> > - if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR) >> > - return dest_lat->meet_with (src_lats->m_value_range); >> > - else if (param_type >> > - && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc)) >> > - == tcc_unary)) >> > + if (TREE_CODE_CLASS (operation) == tcc_unary) >> > { >> > - value_range vr; >> > - memset (&vr, 0, sizeof (vr)); >> > + struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); >> > + int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); >> > tree operand_type = ipa_get_type (caller_info, src_idx); >> > - enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); >> > + struct ipcp_param_lattices *src_lats >> > + = ipa_get_parm_lattices (caller_info, src_idx); >> > >> > if (src_lats->m_value_range.bottom_p ()) >> > return dest_lat->set_to_bottom (); >> > - >> > - extract_range_from_unary_expr (&vr, >> > - operation, >> > - param_type, >> > - &src_lats->m_value_range.m_vr, >> > - operand_type); >> > - if (vr.type == VR_RANGE >> > - || vr.type == VR_ANTI_RANGE) >> > + value_range vr; >> > + if (ipa_vr_operation_and_type_effects (&vr, >> > + &src_lats->m_value_range.m_vr, >> > + operation, param_type, >> > + operand_type)) >> > return dest_lat->meet_with (&vr); >> > } >> > } >> > @@ -1906,8 +1914,11 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, >> > } >> > } >> > >> > - if (jfunc->vr_known) >> > - return dest_lat->meet_with (&jfunc->m_vr); >> > + value_range vr; >> > + if (jfunc->vr_known >> > + && ipa_vr_operation_and_type_effects (&vr, &jfunc->m_vr, NOP_EXPR, >> > + param_type, jfunc->passed_type)) >> >> but instead of a new jfunc->passed_type you can use TREE_TYPE (jfunc->m_vr.min) >> for example. > > Great, thanks a lot for this suggestion. I have used that and removed > the new field addition from the patch and used your suggestion > instead. > >> >> I notice that ipa_jump_func is badly laid out: >> >> struct GTY (()) ipa_jump_func >> { >> /* Aggregate contants description. See struct ipa_agg_jump_function and its >> description. */ >> struct ipa_agg_jump_function agg; >> >> /* Information about zero/non-zero bits. */ >> struct ipa_bits bits; >> >> /* Information about value range. */ >> bool vr_known; >> value_range m_vr; >> >> enum jump_func_type type; >> /* Represents a value of a jump function. pass_through is used only in jump >> function context. constant represents the actual constant in constant jump >> functions and member_cst holds constant c++ member functions. */ >> union jump_func_value >> { >> struct ipa_constant_data GTY ((tag ("IPA_JF_CONST"))) constant; >> struct ipa_pass_through_data GTY ((tag ("IPA_JF_PASS_THROUGH"))) >> pass_through; >> struct ipa_ancestor_jf_data GTY ((tag ("IPA_JF_ANCESTOR"))) ancestor; >> } GTY ((desc ("%1.type"))) value; >> }; >> >> vr_known should be moved to pack with type. > > I have swapped their position in this patch because it does not affect > anything else. > >> and ipa_bits::known should be moved out of it as well. > > ...and will try to come up with a patch doing this soon. > >> I also think we do not use the equiv member of m_vr and thus that is >> a waste. >> >> Not sure if memory use of this struct is an issue. > > We create it for each and every actual argument in every call > statement we track with IPA-CP et al, so it is visible in mem-stats of > LTO WPA of big programs. > > I have bootstrapped, lto-bootstrapped and tested the following on > x86_64-linux. OK for trunk? Ok. Thanks for fixing this. Richard. > Thanks, > > Martin > > > 2017-01-04 Martin Jambor <mjambor@suse.cz> > > PR ipa/78365 > PR ipa/78599 > * ipa-prop.h (ipa_jump_func): Swap positions of vr_known and m_vr. > * ipa-cp.c (ipa_vr_operation_and_type_effects): New function. > (propagate_vr_accross_jump_function): Use the above function for all > value range computations for pass-through jump functions and type > converasion from explicit value range values. > (ipcp_propagate_stage): Do not attempt to deduce types of formal > parameters from TYPE_ARG_TYPES. > * ipa-prop.c (ipa_write_jump_function): Remove trailing whitespace. > (ipa_write_node_info): Stream type of the actual argument. > (ipa_read_node_info): Likewise. Also remove trailing whitespace. > > testsuite/ > * gcc.dg/torture/pr78365.c: New test. > --- > gcc/ipa-cp.c | 71 +++++++++++++++++----------------- > gcc/ipa-prop.c | 14 +++++-- > gcc/ipa-prop.h | 5 ++- > gcc/testsuite/gcc.dg/torture/pr78365.c | 21 ++++++++++ > 4 files changed, 69 insertions(+), 42 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr78365.c > > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > index 82bf35084b6..9cc903769e8 100644 > --- a/gcc/ipa-cp.c > +++ b/gcc/ipa-cp.c > @@ -1837,6 +1837,23 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int idx, > return dest_lattice->set_to_bottom (); > } > > +/* Emulate effects of unary OPERATION and/or conversion from SRC_TYPE to > + DST_TYPE on value range in SRC_VR and store it to DST_VR. Return true if > + the result is a range or an anti-range. */ > + > +static bool > +ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr, > + enum tree_code operation, > + tree dst_type, tree src_type) > +{ > + memset (dst_vr, 0, sizeof (*dst_vr)); > + extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type); > + if (dst_vr->type == VR_RANGE || dst_vr->type == VR_ANTI_RANGE) > + return true; > + else > + return false; > +} > + > /* Propagate value range across jump function JFUNC that is associated with > edge CS with param of callee of PARAM_TYPE and update DEST_PLATS > accordingly. */ > @@ -1846,7 +1863,6 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, > struct ipcp_param_lattices *dest_plats, > tree param_type) > { > - struct ipcp_param_lattices *src_lats; > ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range; > > if (dest_lat->bottom_p ()) > @@ -1859,31 +1875,23 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, > > if (jfunc->type == IPA_JF_PASS_THROUGH) > { > - struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); > - int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); > - src_lats = ipa_get_parm_lattices (caller_info, src_idx); > + enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); > > - if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR) > - return dest_lat->meet_with (src_lats->m_value_range); > - else if (param_type > - && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc)) > - == tcc_unary)) > + if (TREE_CODE_CLASS (operation) == tcc_unary) > { > - value_range vr; > - memset (&vr, 0, sizeof (vr)); > + struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller); > + int src_idx = ipa_get_jf_pass_through_formal_id (jfunc); > tree operand_type = ipa_get_type (caller_info, src_idx); > - enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc); > + struct ipcp_param_lattices *src_lats > + = ipa_get_parm_lattices (caller_info, src_idx); > > if (src_lats->m_value_range.bottom_p ()) > return dest_lat->set_to_bottom (); > - > - extract_range_from_unary_expr (&vr, > - operation, > - param_type, > - &src_lats->m_value_range.m_vr, > - operand_type); > - if (vr.type == VR_RANGE > - || vr.type == VR_ANTI_RANGE) > + value_range vr; > + if (ipa_vr_operation_and_type_effects (&vr, > + &src_lats->m_value_range.m_vr, > + operation, param_type, > + operand_type)) > return dest_lat->meet_with (&vr); > } > } > @@ -1903,8 +1911,12 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, > } > } > > - if (jfunc->vr_known) > - return dest_lat->meet_with (&jfunc->m_vr); > + value_range vr; > + if (jfunc->vr_known > + && ipa_vr_operation_and_type_effects (&vr, &jfunc->m_vr, NOP_EXPR, > + param_type, > + TREE_TYPE (jfunc->m_vr.min))) > + return dest_lat->meet_with (&vr); > else > return dest_lat->set_to_bottom (); > } > @@ -2244,7 +2256,7 @@ propagate_constants_across_call (struct cgraph_edge *cs) > { > struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i); > struct ipcp_param_lattices *dest_plats; > - tree param_type = ipa_get_callee_param_type (cs, i); > + tree param_type = ipa_get_type (callee_info, i); > > dest_plats = ipa_get_parm_lattices (callee_info, i); > if (availability == AVAIL_INTERPOSABLE) > @@ -3230,19 +3242,6 @@ ipcp_propagate_stage (struct ipa_topo_info *topo) > { > struct ipa_node_params *info = IPA_NODE_REF (node); > > - /* In LTO we do not have PARM_DECLs but we would still like to be able to > - look at types of parameters. */ > - if (in_lto_p) > - { > - tree t = TYPE_ARG_TYPES (TREE_TYPE (node->decl)); > - for (int k = 0; k < ipa_get_param_count (info) && t; k++) > - { > - gcc_assert (t != void_list_node); > - info->descriptors[k].decl_or_type = TREE_VALUE (t); > - t = t ? TREE_CHAIN (t) : NULL; > - } > - } > - > determine_versionability (node, info); > if (node->has_gimple_body_p ()) > { > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c > index 1afa8fc7a05..1f68f736e46 100644 > --- a/gcc/ipa-prop.c > +++ b/gcc/ipa-prop.c > @@ -4775,7 +4775,7 @@ ipa_write_jump_function (struct output_block *ob, > { > streamer_write_widest_int (ob, jump_func->bits.value); > streamer_write_widest_int (ob, jump_func->bits.mask); > - } > + } > bp_pack_value (&bp, jump_func->vr_known, 1); > streamer_write_bitpack (&bp); > if (jump_func->vr_known) > @@ -4973,7 +4973,10 @@ ipa_write_node_info (struct output_block *ob, struct cgraph_node *node) > bp_pack_value (&bp, ipa_is_param_used (info, j), 1); > streamer_write_bitpack (&bp); > for (j = 0; j < ipa_get_param_count (info); j++) > - streamer_write_hwi (ob, ipa_get_controlled_uses (info, j)); > + { > + streamer_write_hwi (ob, ipa_get_controlled_uses (info, j)); > + stream_write_tree (ob, ipa_get_type (info, j), true); > + } > for (e = node->callees; e; e = e->next_callee) > { > struct ipa_edge_args *args = IPA_EDGE_REF (e); > @@ -5020,7 +5023,7 @@ ipa_read_node_info (struct lto_input_block *ib, struct cgraph_node *node, > > for (k = 0; k < ipa_get_param_count (info); k++) > info->descriptors[k].move_cost = streamer_read_uhwi (ib); > - > + > bp = streamer_read_bitpack (ib); > if (ipa_get_param_count (info) != 0) > info->analysis_done = true; > @@ -5028,7 +5031,10 @@ ipa_read_node_info (struct lto_input_block *ib, struct cgraph_node *node, > for (k = 0; k < ipa_get_param_count (info); k++) > ipa_set_param_used (info, k, bp_unpack_value (&bp, 1)); > for (k = 0; k < ipa_get_param_count (info); k++) > - ipa_set_controlled_uses (info, k, streamer_read_hwi (ib)); > + { > + ipa_set_controlled_uses (info, k, streamer_read_hwi (ib)); > + info->descriptors[k].decl_or_type = stream_read_tree (ib, data_in); > + } > for (e = node->callees; e; e = e->next_callee) > { > struct ipa_edge_args *args = IPA_EDGE_REF (e); > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index 341d9db6c63..c9a69ab1f53 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -178,9 +178,10 @@ struct GTY (()) ipa_jump_func > /* Information about zero/non-zero bits. */ > struct ipa_bits bits; > > - /* Information about value range. */ > - bool vr_known; > + /* Information about value range, containing valid data only when vr_known is > + true. */ > value_range m_vr; > + bool vr_known; > > enum jump_func_type type; > /* Represents a value of a jump function. pass_through is used only in jump > diff --git a/gcc/testsuite/gcc.dg/torture/pr78365.c b/gcc/testsuite/gcc.dg/torture/pr78365.c > new file mode 100644 > index 00000000000..5180a0171ae > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr78365.c > @@ -0,0 +1,21 @@ > +/* { dg-do compile } */ > + > +int a, b, c; > +char d; > +static void fn1 (void *, int); > +int fn2 (int); > + > +void fn1 (cc, yh) void *cc; > +char yh; > +{ > + char y; > + a = fn2(c - b + 1); > + for (; y <= yh; y++) > + ; > +} > + > +void fn3() > +{ > + fn1((void *)fn3, 1); > + fn1((void *)fn3, d - 1); > +} > -- > 2.11.0 >
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 2ec671f..9853467 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -1846,11 +1846,11 @@ propagate_bits_accross_jump_function (cgraph_edge *cs, int idx, ipa_jump_func *j static bool propagate_vr_accross_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, - struct ipcp_param_lattices *dest_plats, - tree param_type) + struct ipcp_param_lattices *dest_plats) { struct ipcp_param_lattices *src_lats; ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range; + tree param_type = jfunc->param_type; if (dest_lat->bottom_p ()) return false; @@ -2247,7 +2247,6 @@ propagate_constants_accross_call (struct cgraph_edge *cs) { struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i); struct ipcp_param_lattices *dest_plats; - tree param_type = ipa_get_callee_param_type (cs, i); dest_plats = ipa_get_parm_lattices (callee_info, i); if (availability == AVAIL_INTERPOSABLE) @@ -2264,8 +2263,7 @@ propagate_constants_accross_call (struct cgraph_edge *cs) dest_plats); if (opt_for_fn (callee->decl, flag_ipa_vrp)) ret |= propagate_vr_accross_jump_function (cs, - jump_func, dest_plats, - param_type); + jump_func, dest_plats); else ret |= dest_plats->m_value_range.set_to_bottom (); } diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index 642111d..21ee251 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -1659,26 +1659,13 @@ determine_locally_known_aggregate_parts (gcall *call, tree arg, /* Return the Ith param type of callee associated with call graph edge E. */ -tree +static tree ipa_get_callee_param_type (struct cgraph_edge *e, int i) { int n; - tree type = (e->callee - ? TREE_TYPE (e->callee->decl) - : gimple_call_fntype (e->call_stmt)); - tree t = TYPE_ARG_TYPES (type); - - for (n = 0; n < i; n++) - { - if (!t) - break; - t = TREE_CHAIN (t); - } - if (t) - return TREE_VALUE (t); if (!e->callee) - return NULL; - t = DECL_ARGUMENTS (e->callee->decl); + return NULL; + tree t = DECL_ARGUMENTS (e->callee->decl); for (n = 0; n < i; n++) { if (!t) @@ -1732,6 +1719,7 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi, useful_context = true; } + jfunc->param_type = param_type; if (POINTER_TYPE_P (TREE_TYPE (arg))) { bool addr_nonzero = false; @@ -4717,6 +4705,7 @@ ipa_write_jump_function (struct output_block *ob, int i, count; streamer_write_uhwi (ob, jump_func->type); + stream_write_tree (ob, jump_func->param_type, true); switch (jump_func->type) { case IPA_JF_UNKNOWN: @@ -4800,6 +4789,7 @@ ipa_read_jump_function (struct lto_input_block *ib, int i, count; jftype = (enum jump_func_type) streamer_read_uhwi (ib); + jump_func->param_type = stream_read_tree (ib, data_in); switch (jftype) { case IPA_JF_UNKNOWN: diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 0e75cf4..eeb0f6b 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -182,6 +182,9 @@ struct GTY (()) ipa_jump_func bool vr_known; value_range m_vr; + /* Type of callee param corresponding to this jump_func. */ + tree param_type; + enum jump_func_type type; /* Represents a value of a jump function. pass_through is used only in jump function context. constant represents the actual constant in constant jump @@ -818,7 +821,6 @@ ipa_parm_adjustment *ipa_get_adjustment_candidate (tree **, bool *, ipa_parm_adjustment_vec, bool); void ipa_release_body_info (struct ipa_func_body_info *); -tree ipa_get_callee_param_type (struct cgraph_edge *e, int i); /* From tree-sra.c: */ tree build_ref_for_offset (location_t, tree, HOST_WIDE_INT, bool, tree, diff --git a/gcc/testsuite/gcc.dg/torture/pr78365.c b/gcc/testsuite/gcc.dg/torture/pr78365.c index e69de29..5180a01 100644 --- a/gcc/testsuite/gcc.dg/torture/pr78365.c +++ b/gcc/testsuite/gcc.dg/torture/pr78365.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ + +int a, b, c; +char d; +static void fn1 (void *, int); +int fn2 (int); + +void fn1 (cc, yh) void *cc; +char yh; +{ + char y; + a = fn2(c - b + 1); + for (; y <= yh; y++) + ; +} + +void fn3() +{ + fn1((void *)fn3, 1); + fn1((void *)fn3, d - 1); +}