From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24858 invoked by alias); 27 Jun 2014 13:31:32 -0000 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 Received: (qmail 24844 invoked by uid 89); 27 Jun 2014 13:31:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 27 Jun 2014 13:31:30 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id A0A17AC89 for ; Fri, 27 Jun 2014 13:31:26 +0000 (UTC) Date: Fri, 27 Jun 2014 13:31:00 -0000 From: Martin Jambor To: gcc-patches@gcc.gnu.org Subject: Re: [PATCH, PR 61160] Artificial thunks need combined_args_to_skip Message-ID: <20140627133126.GC26334@virgil.suse> Mail-Followup-To: gcc-patches@gcc.gnu.org References: <20140530230831.GC23341@virgil.suse> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140530230831.GC23341@virgil.suse> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg02231.txt.bz2 On Sat, May 31, 2014 at 01:08:31AM +0200, Martin Jambor wrote: > Hi, > > the second issue in PR 61160 is that because artificial thunks > (produced by duplicate_thunk_for_node) do not have > combined_args_to_skip, calls to them do not get actual arguments > removed, while the actual functions do loose their formal parameters, > leading to mismatches. > > Currently, the combined_args_to_skip is computed in of > cgraph_create_virtual_clone only after all the edge redirection and > thunk duplication is done so it had to be moved to a spot before > that. Since we already pass args_to_skip to cgraph_clone_node, I > moved the computation there (otherwise it would have to duplicate the > old value and also pass the new one to the redirection routine). > > I have also noticed that the code producing combined_args_to_skip from > an old value and new args_to_skip cannot work in LTO because we do not > have DECL_ARGUMENTS available at WPA in LTO. The wrong code is > however never executed and so I replaced it with a simple bitmap_ior. > This changes the semantics of args_to_skip for any user of > cgraph_create_virtual_clone that would like to remove some parameters > from something which is already a clone. However, currently there are > no such users and the new semantics is saner because WPA code will be > happier using the old indices rather than remapping everything the > whole time. > > I am still in the process of bootstrapping and testing this patch on > trunk, I will test it on the 4.9 branch too. OK if it passes > everywhere? > The patch has been approved by Honza on IRC. Since then, a conflict has occured on the trunk so I committed this variant after re-testing. Thanks, Martin 2014-06-27 Martin Jambor PR ipa/61160 * cgraphclones.c (duplicate_thunk_for_node): Removed parameter args_to_skip, use those from node instead. Copy args_to_skip and combined_args_to_skip from node to the new thunk. (redirect_edge_duplicating_thunks): Removed parameter args_to_skip. (cgraph_create_virtual_clone): Moved computation of combined_args_to_skip... (cgraph_clone_node): ...here, simplify it to bitmap_ior.. testsuite/ * g++.dg/ipa/pr61160-2.C: New test. * g++.dg/ipa/pr61160-3.C: Likewise. diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index d57cd9f..2e7dc90 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -301,14 +301,13 @@ set_new_clone_decl_and_node_flags (cgraph_node *new_node) thunk is this_adjusting but we are removing this parameter. */ static cgraph_node * -duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node, - bitmap args_to_skip) +duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node) { cgraph_node *new_thunk, *thunk_of; thunk_of = cgraph_function_or_thunk_node (thunk->callees->callee); if (thunk_of->thunk.thunk_p) - node = duplicate_thunk_for_node (thunk_of, node, args_to_skip); + node = duplicate_thunk_for_node (thunk_of, node); struct cgraph_edge *cs; for (cs = node->callers; cs; cs = cs->next_caller) @@ -320,17 +319,18 @@ duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node, return cs->caller; tree new_decl; - if (!args_to_skip) + if (!node->clone.args_to_skip) new_decl = copy_node (thunk->decl); else { /* We do not need to duplicate this_adjusting thunks if we have removed this. */ if (thunk->thunk.this_adjusting - && bitmap_bit_p (args_to_skip, 0)) + && bitmap_bit_p (node->clone.args_to_skip, 0)) return node; - new_decl = build_function_decl_skip_args (thunk->decl, args_to_skip, + new_decl = build_function_decl_skip_args (thunk->decl, + node->clone.args_to_skip, false); } gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl)); @@ -347,6 +347,8 @@ duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node, new_thunk->thunk = thunk->thunk; new_thunk->unique_name = in_lto_p; new_thunk->former_clone_of = thunk->decl; + new_thunk->clone.args_to_skip = node->clone.args_to_skip; + new_thunk->clone.combined_args_to_skip = node->clone.combined_args_to_skip; struct cgraph_edge *e = cgraph_create_edge (new_thunk, node, NULL, 0, CGRAPH_FREQ_BASE); @@ -363,12 +365,11 @@ duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node, chain. */ void -redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n, - bitmap args_to_skip) +redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n) { cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee); if (orig_to->thunk.thunk_p) - n = duplicate_thunk_for_node (orig_to, n, args_to_skip); + n = duplicate_thunk_for_node (orig_to, n); cgraph_redirect_edge_callee (e, n); } @@ -421,9 +422,21 @@ cgraph_clone_node (struct cgraph_node *n, tree decl, gcov_type count, int freq, new_node->rtl = n->rtl; new_node->count = count; new_node->frequency = n->frequency; - new_node->clone = n->clone; - new_node->clone.tree_map = NULL; new_node->tp_first_run = n->tp_first_run; + + new_node->clone.tree_map = NULL; + new_node->clone.args_to_skip = args_to_skip; + if (!args_to_skip) + new_node->clone.combined_args_to_skip = n->clone.combined_args_to_skip; + else if (n->clone.combined_args_to_skip) + { + new_node->clone.combined_args_to_skip = BITMAP_GGC_ALLOC (); + bitmap_ior (new_node->clone.combined_args_to_skip, + n->clone.combined_args_to_skip, args_to_skip); + } + else + new_node->clone.combined_args_to_skip = args_to_skip; + if (n->count) { if (new_node->count > n->count) @@ -448,10 +461,9 @@ cgraph_clone_node (struct cgraph_node *n, tree decl, gcov_type count, int freq, if (!e->callee || DECL_BUILT_IN_CLASS (e->callee->decl) != BUILT_IN_NORMAL || DECL_FUNCTION_CODE (e->callee->decl) != BUILT_IN_UNREACHABLE) - redirect_edge_duplicating_thunks (e, new_node, args_to_skip); + redirect_edge_duplicating_thunks (e, new_node); } - for (e = n->callees;e; e=e->next_callee) cgraph_clone_edge (e, new_node, e->call_stmt, e->lto_stmt_uid, count_scale, freq, update_original); @@ -558,7 +570,6 @@ cgraph_create_virtual_clone (struct cgraph_node *old_node, ABI support for this. */ set_new_clone_decl_and_node_flags (new_node); new_node->clone.tree_map = tree_map; - new_node->clone.args_to_skip = args_to_skip; /* Clones of global symbols or symbols with unique names are unique. */ if ((TREE_PUBLIC (old_decl) @@ -569,32 +580,7 @@ cgraph_create_virtual_clone (struct cgraph_node *old_node, new_node->unique_name = true; FOR_EACH_VEC_SAFE_ELT (tree_map, i, map) new_node->maybe_add_reference (map->new_tree, IPA_REF_ADDR, NULL); - if (!args_to_skip) - new_node->clone.combined_args_to_skip = old_node->clone.combined_args_to_skip; - else if (old_node->clone.combined_args_to_skip) - { - int newi = 0, oldi = 0; - tree arg; - bitmap new_args_to_skip = BITMAP_GGC_ALLOC (); - struct cgraph_node *orig_node; - for (orig_node = old_node; orig_node->clone_of; orig_node = orig_node->clone_of) - ; - for (arg = DECL_ARGUMENTS (orig_node->decl); - arg; arg = DECL_CHAIN (arg), oldi++) - { - if (bitmap_bit_p (old_node->clone.combined_args_to_skip, oldi)) - { - bitmap_set_bit (new_args_to_skip, oldi); - continue; - } - if (bitmap_bit_p (args_to_skip, newi)) - bitmap_set_bit (new_args_to_skip, oldi); - newi++; - } - new_node->clone.combined_args_to_skip = new_args_to_skip; - } - else - new_node->clone.combined_args_to_skip = args_to_skip; + if (old_node->ipa_transforms_to_apply.exists ()) new_node->ipa_transforms_to_apply = old_node->ipa_transforms_to_apply.copy (); diff --git a/gcc/testsuite/g++.dg/ipa/pr61160-2.C b/gcc/testsuite/g++.dg/ipa/pr61160-2.C new file mode 100644 index 0000000..1011bd1 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr61160-2.C @@ -0,0 +1,43 @@ +/* { dg-do run } */ +/* { dg-options "-O3 --param ipa-cp-eval-threshold=1" } */ + +extern "C" void abort (void); + +struct CBase { + virtual void BaseFunc () {} +}; + +struct MMixin { + virtual void * MixinFunc (int, void *) = 0; +}; + +struct CExample: CBase, public MMixin +{ + int stuff, magic, more_stuff; + + CExample () + { + stuff = 0; + magic = 0xbeef; + more_stuff = 0; + } + void *MixinFunc (int arg, void *arg2) + { + if (arg != 1 || arg2) + return 0; + if (magic != 0xbeef) + abort(); + return this; + } +}; + +void *test (MMixin & anExample) +{ + return anExample.MixinFunc (1, 0); +} + +int main () +{ + CExample c; + return (test (c) != &c); +} diff --git a/gcc/testsuite/g++.dg/ipa/pr61160-3.C b/gcc/testsuite/g++.dg/ipa/pr61160-3.C new file mode 100644 index 0000000..8184ec2 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr61160-3.C @@ -0,0 +1,37 @@ +/* { dg-do run } */ +/* { dg-options "-O3" } */ + +struct A { + void *p; + A (void *q) : p (q) {} + A (const A &) : p () {} +}; + +struct CBase { + virtual void BaseFunc () {} +}; + +struct MMixin { + virtual A MixinFunc (int, A) = 0; +}; + +struct CExample: CBase, public MMixin +{ + A MixinFunc (int arg, A arg2) + { + if (arg != 1 || arg2.p) + return 0; + return this; + } +}; + +void *test (MMixin & anExample) +{ + return anExample.MixinFunc (1, (0)).p; +} + +int main () +{ + CExample c; + return (test (c) != &c); +}