* [PATCH, PR 61160] Artificial thunks need combined_args_to_skip
@ 2014-05-30 23:08 Martin Jambor
2014-06-17 19:42 ` Martin Jambor
2014-06-27 13:31 ` Martin Jambor
0 siblings, 2 replies; 3+ messages in thread
From: Martin Jambor @ 2014-05-30 23:08 UTC (permalink / raw)
To: GCC Patches; +Cc: Jan Hubicka
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?
Thanks,
Martin
2014-05-29 Martin Jambor <mjambor@suse.cz>
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 4387b99..91cc13c 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));
@@ -348,6 +348,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);
@@ -364,12 +366,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);
}
@@ -422,9 +423,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)
@@ -449,10 +462,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);
@@ -561,7 +573,6 @@ cgraph_create_virtual_clone (struct cgraph_node *old_node,
DECL_SECTION_NAME (new_node->decl) = NULL;
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)
@@ -573,32 +584,7 @@ cgraph_create_virtual_clone (struct cgraph_node *old_node,
FOR_EACH_VEC_SAFE_ELT (tree_map, i, map)
ipa_maybe_record_reference (new_node, 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);
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH, PR 61160] Artificial thunks need combined_args_to_skip
2014-05-30 23:08 [PATCH, PR 61160] Artificial thunks need combined_args_to_skip Martin Jambor
@ 2014-06-17 19:42 ` Martin Jambor
2014-06-27 13:31 ` Martin Jambor
1 sibling, 0 replies; 3+ messages in thread
From: Martin Jambor @ 2014-06-17 19:42 UTC (permalink / raw)
To: GCC Patches, Jan Hubicka
Hi,
Ping.
Thanks,
Martin
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?
>
> Thanks,
>
> Martin
>
>
>
> 2014-05-29 Martin Jambor <mjambor@suse.cz>
>
> 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 4387b99..91cc13c 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));
> @@ -348,6 +348,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);
> @@ -364,12 +366,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);
> }
> @@ -422,9 +423,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)
> @@ -449,10 +462,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);
> @@ -561,7 +573,6 @@ cgraph_create_virtual_clone (struct cgraph_node *old_node,
> DECL_SECTION_NAME (new_node->decl) = NULL;
> 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)
> @@ -573,32 +584,7 @@ cgraph_create_virtual_clone (struct cgraph_node *old_node,
> FOR_EACH_VEC_SAFE_ELT (tree_map, i, map)
> ipa_maybe_record_reference (new_node, 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);
> +}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH, PR 61160] Artificial thunks need combined_args_to_skip
2014-05-30 23:08 [PATCH, PR 61160] Artificial thunks need combined_args_to_skip Martin Jambor
2014-06-17 19:42 ` Martin Jambor
@ 2014-06-27 13:31 ` Martin Jambor
1 sibling, 0 replies; 3+ messages in thread
From: Martin Jambor @ 2014-06-27 13:31 UTC (permalink / raw)
To: gcc-patches
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 <mjambor@suse.cz>
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);
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-06-27 13:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-30 23:08 [PATCH, PR 61160] Artificial thunks need combined_args_to_skip Martin Jambor
2014-06-17 19:42 ` Martin Jambor
2014-06-27 13:31 ` Martin Jambor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).