From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10063 invoked by alias); 2 Sep 2011 12:27:23 -0000 Received: (qmail 10051 invoked by uid 22791); 2 Sep 2011 12:27:21 -0000 X-SWARE-Spam-Status: No, hits=-3.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 02 Sep 2011 12:27:03 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 0E8EC890B6; Fri, 2 Sep 2011 14:27:02 +0200 (CEST) Date: Fri, 02 Sep 2011 12:27:00 -0000 From: Martin Jambor To: GCC Patches Cc: Jan Hubicka Subject: [PATCH] Enable IPA-CP on functions with variable number of arguments or type attributes Message-ID: <20110902122701.GA21553@virgil.arch.suse.de> Mail-Followup-To: GCC Patches , Jan Hubicka MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-09/txt/msg00139.txt.bz2 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? 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? Thanks, Martin 2011-09-01 Martin Jambor * 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)); - } } -