| Message ID | 545CC0E4.7050602@suse.cz |
|---|---|
| State | New |
| Headers | show |
> >> jeff > > Hello. > > I think the bug is a duplicate of PR63580 and there's working patch that can bootstrap on x86_64-linux and no regression has been seen. > > Ready for trunk? > Thanks, > Martin > gcc/ChangeLog: > > 2014-11-07 Martin Liska <mliska@suse.cz> > > * cgraphunit.c (cgraph_node::create_wrapper): > TREE_ADDRESSABLE is set to false for a newly created thunk. OK, thanks! Honza > > gcc/testsuite/ChangeLog: > > 2014-11-07 Martin Liska <mliska@suse.cz> > > * g++.dg/ipa/pr63580.C: New test. > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > index 6f61f5c..89c96dc 100644 > --- a/gcc/cgraphunit.c > +++ b/gcc/cgraphunit.c > @@ -2342,6 +2342,14 @@ cgraph_node::create_wrapper (cgraph_node *target) > > cgraph_edge *e = create_edge (target, NULL, 0, CGRAPH_FREQ_BASE); > > + tree arguments = DECL_ARGUMENTS (decl); > + > + while (arguments) > + { > + TREE_ADDRESSABLE (arguments) = false; > + arguments = TREE_CHAIN (arguments); > + } > + > expand_thunk (false, true); > e->call_stmt_cannot_inline_p = true; > > diff --git a/gcc/testsuite/g++.dg/ipa/pr63580.C b/gcc/testsuite/g++.dg/ipa/pr63580.C > new file mode 100644 > index 0000000..904195a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ipa/pr63580.C > @@ -0,0 +1,26 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-ipa-icf" } */ > + > +struct A > +{ > +}; > +template <class L, class R> A operator%(L, R); > +template <class A0, class A1, class A2, class A3> > +void make_tuple (A0 &, A1, A2, A3); > +A > +bar (int p1, char p2, int p3, double p4) > +{ > + A a; > + make_tuple (p1, p2, p3, p4); > + return "int; char; string; double; " % a; > +} > +A > +foo (int p1, char p2, int p3, double p4) > +{ > + A b; > + make_tuple (p1, p2, p3, p4); > + return "int; char; string; double; " % b; > +} > + > +/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */ > +/* { dg-final { cleanup-ipa-dump "icf" } } */
On 07/11/14 12:53, Martin Liška wrote: > On 11/07/2014 10:52 AM, Jan Hubicka wrote: >>> On 11/05/14 07:09, Jiong Wang wrote: >>>> the same ICE will happen on x86-64, if compile with -O2 -fPIC. >>>> >>>> the reason is for the following two functions, they are identical, so >>>> IPA-ICF >>>> pass try to transform the second function to call the first one directly. >>>> >>>> int >>>> atomic_compare_exchange_STRONG_RELEASE_ACQUIRE (int a, int b) >>>> { >>>> return __atomic_compare_exchange (&v, &a, &b, >>>> STRONG, __ATOMIC_RELEASE, >>>> __ATOMIC_ACQUIRE); >>>> } >>>> >>>> int >>>> atomic_compare_exchange_n_STRONG_RELEASE_ACQUIRE (int a, int b) >>>> { >>>> return __atomic_compare_exchange_n (&v, &a, b, >>>> STRONG, __ATOMIC_RELEASE, >>>> __ATOMIC_ACQUIRE); >>>> } >>>> >>>> while during this transformation, looks like there are something wrong >>>> with the >>>> function argument handling. take "a" for example, because later there >>>> are "&a", >>>> so it's marked as addressable. while after transformation, if we turn the >>>> second function into >>>> >>>> int atomic_compare_exchange_n_STRONG_RELEASE_ACQUIRE (int a, int b) >>>> { >>>> return atomic_compare_exchange_STRONG_RELEASE_ACQUIRE (a, b) >>>> } >>>> >>>> then argument a is no longer addressable. >>>> >>>> so, in cgraph_node::release_body, when making the wrapper, except >>>> clearing the >>>> function body, we should also clear the addressable flag for function args >>>> because they are decided by the function body which is cleared. >>>> >>>> bootstrap ok on x86-64 and no regression. >>>> bootstrap ok on aarch64 juno. >>>> ICE gone away on arm & x86-64 >>>> >>>> ok for trunk? >>>> >>>> gcc/ >>>> >>>> PR tree-optimization/63721 >>>> * cgraph.c (cgraph_node::release_body): Clear addressable flag for >>>> function args. >>> While I understand the need to clear the addressable flag, I think >>> release_body probably isn't the best place to do this. Seems to me >>> that ought to happen when we emit the thunk or otherwise transform >>> the body into something that doesn't take the address of those >>> parameters. >> Yep, I would just move it into expand_thunk - the TREE_ADDRESSABLE bits are not really >> well defined before we build the gimple body. >> >> Honza >>> jeff > Hello. > > I think the bug is a duplicate of PR63580 and there's working patch that can bootstrap on x86_64-linux and no regression has been seen. thanks. looks good to me, although I think expand_thunk is a better place to fix as there is a loop on arguments already, for (arg = a; arg; arg = DECL_CHAIN (arg)) nargs++; > > Ready for trunk? > Thanks, > Martin
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 6f61f5c..89c96dc 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -2342,6 +2342,14 @@ cgraph_node::create_wrapper (cgraph_node *target) cgraph_edge *e = create_edge (target, NULL, 0, CGRAPH_FREQ_BASE); + tree arguments = DECL_ARGUMENTS (decl); + + while (arguments) + { + TREE_ADDRESSABLE (arguments) = false; + arguments = TREE_CHAIN (arguments); + } + expand_thunk (false, true); e->call_stmt_cannot_inline_p = true; diff --git a/gcc/testsuite/g++.dg/ipa/pr63580.C b/gcc/testsuite/g++.dg/ipa/pr63580.C new file mode 100644 index 0000000..904195a --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr63580.C @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf" } */ + +struct A +{ +}; +template <class L, class R> A operator%(L, R); +template <class A0, class A1, class A2, class A3> +void make_tuple (A0 &, A1, A2, A3); +A +bar (int p1, char p2, int p3, double p4) +{ + A a; + make_tuple (p1, p2, p3, p4); + return "int; char; string; double; " % a; +} +A +foo (int p1, char p2, int p3, double p4) +{ + A b; + make_tuple (p1, p2, p3, p4); + return "int; char; string; double; " % b; +} + +/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */ +/* { dg-final { cleanup-ipa-dump "icf" } } */