On Fri, Sep 2, 2011 at 2:27 PM, Martin Jambor <mjam...@suse.cz> wrote:
> Hi,
>
> currently IPA-CP switches itself off on functions with are called with
> variable number of arguments or on those whose signature cannot be
> changed because of type attributes. This patch turns it on for both,
> not changing the parameters of its clones in the latter case.
>
> Whether the function's signature can be changed is determined by
> looking at node->local.can_change_signature which is set appropriately
> already since my last patch (rev 178386).
>
> This patch also replaces ipa-prop's own node_versionable flag with
> inline_summary (node)->versionable. Frankly, I find it very confusing
> that this flag is in the inline summary even though it is set in
> ipa-prop (except for thunks - a case we currently never care about
> anyway) and used only in ipa-cp. I also think this is a property of
> the node (rather than an inlining parameter) as much as
> can_change_signature is - would a patch moving it to node->local be
> acceptable?
I think so, or even node->global?
> The patch passes bootstrap and testsuite on x86_64-linux. It also
> successfully LTO-builds 465.tonto where it creates 647 clones (498 of
> them replacing the original function completely) without changing the
> signature of a single one. (No change in the run-time on this
> benchmark, though).
>
> OK for trunk?
Ok.
Thanks,
Richard.
> Thanks,
>
> Martin
>
>
>
> 2011-09-01 Martin Jambor <mjam...@suse.cz>
>
> * ipa-prop.h (ipa_node_params): Removed fields
> called_with_var_arguments and node_versionable.
> (ipa_set_called_with_variable_arg): Removed.
> (ipa_is_called_with_var_arguments): Likewise.
> * ipa-cp.c (ipa_get_lattice): Fixed index check in an assert.
> (determine_versionability): Do not check for type attributes and va
> builtins. Record versionability into inline summary.
> (initialize_node_lattices): Do not check
> ipa_is_called_with_var_arguments.
> (propagate_constants_accross_call): Likewise, ignore arguments we do
> not have PARM_DECLs for, set variable flag for parameters that were
> not passed a value.
> (create_specialized_node): Dump info that we cannot change signature.
> * ipa-prop.c (ipa_compute_jump_functions): Do not care about variable
> number of arguments.
> (ipa_make_edge_direct_to_target): Likewise.
> (ipa_update_after_lto_read): Likewise.
> (ipa_node_duplication_hook): Do not copy called_with_var_arguments
> flag.
> * tree-inline.c (copy_arguments_for_versioning): Copy PARM_DECLs if
> they were remapped.
>
> * testsuite/gcc.dg/ipa/ipcp-3.c: New test.
>
> Index: src/gcc/ipa-cp.c
> ===================================================================
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -221,7 +221,7 @@ static struct ipcp_value *values_topo;
> static inline struct ipcp_lattice *
> ipa_get_lattice (struct ipa_node_params *info, int i)
> {
> - gcc_assert (i >= 0 && i <= ipa_get_param_count (info));
> + gcc_assert (i >= 0 && i < ipa_get_param_count (info));
> gcc_checking_assert (!info->ipcp_orig_node);
> gcc_checking_assert (info->lattices);
> return &(info->lattices[i]);
> @@ -360,7 +360,6 @@ print_all_lattices (FILE * f, bool dump_
> static void
> determine_versionability (struct cgraph_node *node)
> {
> - struct cgraph_edge *edge;
> const char *reason = NULL;
>
> /* There are a number of generic reasons functions cannot be versioned. We
> @@ -369,32 +368,15 @@ determine_versionability (struct cgraph_
> if (node->alias || node->thunk.thunk_p)
> reason = "alias or thunk";
> else if (!inline_summary (node)->versionable)
> - reason = "inliner claims it is so";
> - else if (TYPE_ATTRIBUTES (TREE_TYPE (node->decl)))
> - reason = "there are type attributes";
> + reason = "not a tree_versionable_function";
> else if (cgraph_function_body_availability (node) <= AVAIL_OVERWRITABLE)
> reason = "insufficient body availability";
> - else
> - /* Removing arguments doesn't work if the function takes varargs
> - or use __builtin_apply_args.
> - FIXME: handle this together with can_change_signature flag. */
> - for (edge = node->callees; edge; edge = edge->next_callee)
> - {
> - tree t = edge->callee->decl;
> - if (DECL_BUILT_IN_CLASS (t) == BUILT_IN_NORMAL
> - && (DECL_FUNCTION_CODE (t) == BUILT_IN_APPLY_ARGS
> - || DECL_FUNCTION_CODE (t) == BUILT_IN_VA_START))
> - {
> - reason = "prohibitive builtins called";
> - break;
> - };
> - }
>
> if (reason && dump_file && !node->alias && !node->thunk.thunk_p)
> fprintf (dump_file, "Function %s/%i is not versionable, reason: %s.\n",
> cgraph_node_name (node), node->uid, reason);
>
> - IPA_NODE_REF (node)->node_versionable = (reason == NULL);
> + inline_summary (node)->versionable = (reason == NULL);
> }
>
> /* Return true if it is at all technically possible to create clones of a
> @@ -403,7 +385,7 @@ determine_versionability (struct cgraph_
> static bool
> ipcp_versionable_function_p (struct cgraph_node *node)
> {
> - return IPA_NODE_REF (node)->node_versionable;
> + return inline_summary (node)->versionable;
> }
>
> /* Structure holding accumulated information about callers of a node. */
> @@ -610,9 +592,7 @@ initialize_node_lattices (struct cgraph_
> int i;
>
> gcc_checking_assert (cgraph_function_with_gimple_body_p (node));
> - if (ipa_is_called_with_var_arguments (info))
> - disable = true;
> - else if (!node->local.local)
> + if (!node->local.local)
> {
> /* When cloning is allowed, we can assume that externally visible
> functions are not called. We will compensate this by cloning
> @@ -1068,18 +1048,17 @@ propagate_constants_accross_call (struct
> struct cgraph_node *callee, *alias_or_thunk;
> struct ipa_edge_args *args;
> bool ret = false;
> - int i, count;
> + int i, args_count, parms_count;
>
> callee = cgraph_function_node (cs->callee, &availability);
> if (!callee->analyzed)
> return false;
> gcc_checking_assert (cgraph_function_with_gimple_body_p (callee));
> callee_info = IPA_NODE_REF (callee);
> - if (ipa_is_called_with_var_arguments (callee_info))
> - return false;
>
> args = IPA_EDGE_REF (cs);
> - count = ipa_get_cs_argument_count (args);
> + args_count = ipa_get_cs_argument_count (args);
> + parms_count = ipa_get_param_count (callee_info);
>
> /* If this call goes through a thunk we must not propagate to the first
> (0th)
> parameter. However, we might need to uncover a thunk from below a series
> @@ -1095,7 +1074,7 @@ propagate_constants_accross_call (struct
> else
> i = 0;
>
> - for (; i < count; i++)
> + for (; (i < args_count) && (i < parms_count); i++)
> {
> struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i);
> struct ipcp_lattice *dest_lat = ipa_get_lattice (callee_info, i);
> @@ -1105,6 +1084,9 @@ propagate_constants_accross_call (struct
> else
> ret |= propagate_accross_jump_function (cs, jump_func, dest_lat);
> }
> + for (; i < parms_count; i++)
> + ret |= set_lattice_contains_variable (ipa_get_lattice (callee_info, i));
> +
> return ret;
> }
>
> @@ -2004,7 +1986,11 @@ create_specialized_node (struct cgraph_n
> }
> }
> else
> - args_to_skip = NULL;
> + {
> + args_to_skip = NULL;
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + fprintf (dump_file, " cannot change function signature\n");
> + }
>
> for (i = 0; i < count ; i++)
> {
> Index: src/gcc/ipa-prop.h
> ===================================================================
> --- src.orig/gcc/ipa-prop.h
> +++ src/gcc/ipa-prop.h
> @@ -168,11 +168,6 @@ struct ipa_node_params
> /* If this node is an ipa-cp clone, these are the known values that describe
> what it has been specialized for. */
> VEC (tree, heap) *known_vals;
> - /* Whether this function is called with variable number of actual
> - arguments. */
> - unsigned called_with_var_arguments : 1;
> - /* Set when it is possible to create specialized versions of this node. */
> - unsigned node_versionable : 1;
> /* Whether the param uses analysis has already been performed. */
> unsigned uses_analysis_done : 1;
> /* Whether the function is enqueued in ipa-cp propagation stack. */
> @@ -224,22 +219,6 @@ ipa_is_param_used (struct ipa_node_param
> return VEC_index (ipa_param_descriptor_t, info->descriptors, i)->used;
> }
>
> -/* Flag this node as having callers with variable number of arguments. */
> -
> -static inline void
> -ipa_set_called_with_variable_arg (struct ipa_node_params *info)
> -{
> - info->called_with_var_arguments = 1;
> -}
> -
> -/* Have we detected this node was called with variable number of arguments?
> */
> -
> -static inline bool
> -ipa_is_called_with_var_arguments (struct ipa_node_params *info)
> -{
> - return info->called_with_var_arguments;
> -}
> -
> /* ipa_edge_args stores information related to a callsite and particularly
> its
> arguments. It can be accessed by the IPA_EDGE_REF macro. */
> typedef struct GTY(()) ipa_edge_args
> Index: src/gcc/tree-inline.c
> ===================================================================
> --- src.orig/gcc/tree-inline.c
> +++ src/gcc/tree-inline.c
> @@ -4869,6 +4869,8 @@ copy_arguments_for_versioning (tree orig
> if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
> {
> tree new_tree = remap_decl (arg, id);
> + if (TREE_CODE (new_tree) != PARM_DECL)
> + new_tree = id->copy_decl (arg, id);
> lang_hooks.dup_lang_specific_decl (new_tree);
> *parg = new_tree;
> parg = &DECL_CHAIN (new_tree);
> Index: src/gcc/testsuite/gcc.dg/ipa/ipcp-3.c
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/ipa/ipcp-3.c
> @@ -0,0 +1,70 @@
> +/* Verify that IPA-CP can clone mark_cell without miscompiling it despite its
> + type_attributes. */
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fdump-ipa-cp" } */
> +
> +
> +struct PMC {
> + unsigned flags;
> +};
> +
> +typedef struct Pcc_cell
> +{
> + struct PMC *p;
> + long bla;
> + long type;
> +} Pcc_cell;
> +
> +int gi;
> +
> +extern void abort ();
> +extern void never_ever(int * interp, struct PMC *pmc)
> + __attribute__((noinline));
> +
> +void never_ever (int * interp, struct PMC *pmc)
> +{
> + abort ();
> +}
> +
> +static void mark_cell(int * interp, Pcc_cell *c)
> + __attribute__((__nonnull__(1)))
> + __attribute__((noinline));
> +
> +static void
> +mark_cell(int * interp, Pcc_cell *c)
> +{
> + if (c && c->type == 4 && c->p
> + && !(c->p->flags & (1<<18)))
> + never_ever(interp, c->p);
> +}
> +
> +static void foo(int * interp, Pcc_cell *c)
> + __attribute__((noinline));
> +
> +static void
> +foo(int * interp, Pcc_cell *c)
> +{
> + mark_cell(interp, c);
> +}
> +
> +static struct Pcc_cell *
> +__attribute__((noinline,noclone))
> +getnull(void)
> +{
> + return (struct Pcc_cell *) 0;
> +}
> +
> +
> +int main()
> +{
> + int i;
> +
> + for (i = 0; i < 100; i++)
> + foo (&gi, getnull ());
> + return 0;
> +}
> +
> +
> +/* { dg-final { scan-ipa-dump "Creating a specialized node of mark_cell"
> "cp" } } */
> +/* { dg-final { cleanup-ipa-dump "cp" } } */
> +
> Index: src/gcc/ipa-prop.c
> ===================================================================
> --- src.orig/gcc/ipa-prop.c
> +++ src/gcc/ipa-prop.c
> @@ -1032,19 +1032,13 @@ ipa_compute_jump_functions (struct cgrap
>
> for (cs = node->callees; cs; cs = cs->next_callee)
> {
> - struct cgraph_node *callee = cgraph_function_or_thunk_node
> (cs->callee, NULL);
> + struct cgraph_node *callee = cgraph_function_or_thunk_node (cs->callee,
> + NULL);
> /* We do not need to bother analyzing calls to unknown
> functions unless they may become known during lto/whopr. */
> - if (!cs->callee->analyzed && !flag_lto)
> + if (!callee->analyzed && !flag_lto)
> continue;
> ipa_count_arguments (cs);
> - /* If the descriptor of the callee is not initialized yet, we have to
> do
> - it now. */
> - if (callee->analyzed)
> - ipa_initialize_node_params (callee);
> - if (ipa_get_cs_argument_count (IPA_EDGE_REF (cs))
> - != ipa_get_param_count (IPA_NODE_REF (callee)))
> - ipa_set_called_with_variable_arg (IPA_NODE_REF (callee));
> ipa_compute_jump_functions_for_edge (parms_info, cs);
> }
>
> @@ -1649,10 +1643,6 @@ ipa_make_edge_direct_to_target (struct c
> }
> callee = cgraph_function_or_thunk_node (callee, NULL);
>
> - if (ipa_get_cs_argument_count (IPA_EDGE_REF (ie))
> - != ipa_get_param_count (IPA_NODE_REF (callee)))
> - ipa_set_called_with_variable_arg (IPA_NODE_REF (callee));
> -
> return ie;
> }
>
> @@ -1964,7 +1954,6 @@ ipa_node_duplication_hook (struct cgraph
> new_info->lattices = NULL;
> new_info->ipcp_orig_node = old_info->ipcp_orig_node;
>
> - new_info->called_with_var_arguments = old_info->called_with_var_arguments;
> new_info->uses_analysis_done = old_info->uses_analysis_done;
> new_info->node_enqueued = old_info->node_enqueued;
> }
> @@ -2949,7 +2938,6 @@ void
> ipa_update_after_lto_read (void)
> {
> struct cgraph_node *node;
> - struct cgraph_edge *cs;
>
> ipa_check_create_node_params ();
> ipa_check_create_edge_args ();
> @@ -2957,17 +2945,4 @@ ipa_update_after_lto_read (void)
> for (node = cgraph_nodes; node; node = node->next)
> if (node->analyzed)
> ipa_initialize_node_params (node);
> -
> - for (node = cgraph_nodes; node; node = node->next)
> - if (node->analyzed)
> - for (cs = node->callees; cs; cs = cs->next_callee)
> - {
> - struct cgraph_node *callee;
> -
> - callee = cgraph_function_or_thunk_node (cs->callee, NULL);
> - if (ipa_get_cs_argument_count (IPA_EDGE_REF (cs))
> - != ipa_get_param_count (IPA_NODE_REF (callee)))
> - ipa_set_called_with_variable_arg (IPA_NODE_REF (callee));
> - }
> }
> -
>
