On 9 August 2016 at 23:43, Martin Jambor wrote: > Hi, > > On Tue, Aug 09, 2016 at 05:17:31PM +0530, Prathamesh Kulkarni wrote: >> On 9 August 2016 at 16:39, Martin Jambor wrote: >> >> ... >> >> >> Instead of storing arg's precision and sign, we should store >> >> parameter's precision and sign in ipa_compute_jump_functions_for_edge (). >> >> Diff with respect to previous patch: >> >> >> >> @@ -1688,9 +1690,9 @@ ipa_compute_jump_functions_for_edge (struct >> >> ipa_func_body_info *fbi, >> >> && (TREE_CODE (arg) == SSA_NAME || TREE_CODE (arg) == INTEGER_CST)) >> >> { >> >> jfunc->bits.known = true; >> >> - jfunc->bits.sgn = TYPE_SIGN (TREE_TYPE (arg)); >> >> - jfunc->bits.precision = TYPE_PRECISION (TREE_TYPE (arg)); >> >> - >> >> + jfunc->bits.sgn = TYPE_SIGN (param_type); >> >> + jfunc->bits.precision = TYPE_PRECISION (param_type); >> >> + >> > >> > If you want to use the precision of the formal parameter then you do >> > not need to store it to jump functions. Parameter DECLs along with >> > their types are readily accessible in IPA (even with LTO). It would >> > also be much clearer what is going on, IMHO. >> Could you please point out how to access parameter decl in wpa ? >> The only reason I ended up putting this in jump function was because >> I couldn't figure out how to access param decl during WPA. >> I see there's ipa_get_param() in ipa-prop.h however it's gated on >> gcc_checking_assert (!flag_wpa), so I suppose I can't use this >> during WPA ? >> >> Alternatively I think I could access cs->callee->decl and get to the param decl >> by walking DECL_ARGUMENTS ? > > Actually, we no longer have DECL_ARGUMENTS during LTO WPA. But in > most cases, you can still get at the type with something like the > following (only very lightly tested) patch, if Honza does not think it > is too crazy. > > Note that= for old K&R C sources we do not have TYPE_ARG_TYPES and so > ipa_get_type can return NULL(!) ...however I wonder whether for such > programs the type assumptions made in callers when constructing jump > functions can be trusted either. > > I have to run, we will continue the discussion later. Thanks for the patch. In this version, I updated the patch to use ipa_get_type, remove precision and sgn from ipcp_bits_lattice and ipa_bits, and renamed member variables to add m_ prefix. Does it look OK ? I am looking for test-case that affects precision and hopefully add that along with other test-cases in follow-up patch. Bootstrap+test in progress on x86_64-unknown-linux-gnu. Thanks, Prathamesh > > Martin > > > 2016-08-09 Martin Jambor > > * ipa-prop.h (ipa_param_descriptor): Renamed decl to decl_or_type. > Update comment. > (ipa_get_param): Updated comment, added assert that we have a > PARM_DECL. > (ipa_get_type): New function. > * ipa-cp.c (ipcp_propagate_stage): Fill in argument types in LTO mode. > * ipa-prop.c (ipa_get_param_decl_index_1): Use decl_or_type > instead of decl; > (ipa_populate_param_decls): Likewise. > (ipa_dump_param): Likewise. > > > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > index 5b6cb9a..3465da5 100644 > --- a/gcc/ipa-cp.c > +++ b/gcc/ipa-cp.c > @@ -1952,11 +1952,21 @@ propagate_constants_accross_call (struct cgraph_edge *cs) > else > i = 0; > > + /* !!! The following dump is of course only a demonstration that it works: */ > + debug_generic_expr (callee->decl); > + fprintf (stderr, "\n"); > + > for (; (i < args_count) && (i < parms_count); i++) > { > struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i); > struct ipcp_param_lattices *dest_plats; > > + /* !!! The following dump is of course only a demonstration that it > + works: */ > + fprintf (stderr, " The type of parameter %i is: ", i); > + debug_generic_expr (ipa_get_type (callee_info, i)); > + fprintf (stderr, "\n"); > + > dest_plats = ipa_get_parm_lattices (callee_info, i); > if (availability == AVAIL_INTERPOSABLE) > ret |= set_all_contains_variable (dest_plats); > @@ -2936,6 +2946,19 @@ 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); 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 132b622..1eaccdf 100644 > --- a/gcc/ipa-prop.c > +++ b/gcc/ipa-prop.c > @@ -103,9 +103,10 @@ ipa_get_param_decl_index_1 (vec descriptors, tree ptree) > { > int i, count; > > + gcc_checking_assert (!flag_wpa); > count = descriptors.length (); > for (i = 0; i < count; i++) > - if (descriptors[i].decl == ptree) > + if (descriptors[i].decl_or_type == ptree) > return i; > > return -1; > @@ -138,7 +139,7 @@ ipa_populate_param_decls (struct cgraph_node *node, > param_num = 0; > for (parm = fnargs; parm; parm = DECL_CHAIN (parm)) > { > - descriptors[param_num].decl = parm; > + descriptors[param_num].decl_or_type = parm; > descriptors[param_num].move_cost = estimate_move_cost (TREE_TYPE (parm), > true); > param_num++; > @@ -168,10 +169,10 @@ void > ipa_dump_param (FILE *file, struct ipa_node_params *info, int i) > { > fprintf (file, "param #%i", i); > - if (info->descriptors[i].decl) > + if (info->descriptors[i].decl_or_type) > { > fprintf (file, " "); > - print_generic_expr (file, info->descriptors[i].decl, 0); > + print_generic_expr (file, info->descriptors[i].decl_or_type, 0); > } > } > > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index e32d078..1d5ce0b 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -283,8 +283,11 @@ ipa_get_jf_ancestor_type_preserved (struct ipa_jump_func *jfunc) > > struct ipa_param_descriptor > { > - /* PARAM_DECL of this parameter. */ > - tree decl; > + /* In analysis and modification phase, this is the PARAM_DECL of this > + parameter, in IPA LTO phase, this is the type of the the described > + parameter or NULL if not known. Do not read this field directly but > + through ipa_get_param and ipa_get_type as appropriate. */ > + tree decl_or_type; > /* If all uses of the parameter are described by ipa-prop structures, this > says how many there are. If any use could not be described by means of > ipa-prop structures, this is IPA_UNDESCRIBED_USE. */ > @@ -402,13 +405,31 @@ ipa_get_param_count (struct ipa_node_params *info) > > /* Return the declaration of Ith formal parameter of the function corresponding > to INFO. Note there is no setter function as this array is built just once > - using ipa_initialize_node_params. */ > + using ipa_initialize_node_params. This function should not be called in > + WPA. */ > > static inline tree > ipa_get_param (struct ipa_node_params *info, int i) > { > gcc_checking_assert (!flag_wpa); > - return info->descriptors[i].decl; > + tree t = info->descriptors[i].decl_or_type; > + gcc_checking_assert (TREE_CODE (t) == PARM_DECL); > + return t; > +} > + > +/* Return the type of Ith formal parameter of the function corresponding > + to INFO if it is known or NULL if not. */ > + > +static inline tree > +ipa_get_type (struct ipa_node_params *info, int i) > +{ > + tree t = info->descriptors[i].decl_or_type; > + if (!t) > + return NULL; > + if (TYPE_P (t)) > + return t; > + gcc_checking_assert (TREE_CODE (t) == PARM_DECL); > + return TREE_TYPE (t); > } > > /* Return the move cost of Ith formal parameter of the function corresponding >