Message ID | ri6o95cpoaa.fsf@suse.cz |
---|---|
State | New |
Headers | show |
Series | [PR,89693] Reorganize cgraph_node::clone_of_p | expand |
Dne 2019-04-11 17:14, Martin Jambor napsal: > Hi, > > this reorganization of cgraph_node:clone_of_p() prevents verifier > falsely ICEing because it cannot recognize that a former thunk > (expanded > even before reaching pass pipeline) was cloned by IPA-CP. > > It basically traverses the clone_of chain at each step of thunk chain > traversal, rather than just after it. This is only done when checking > is on, so the extra little overhead should be of little concern. > > Bootstrapped, LTO-bootstrapped and tested on x86_64, OK for trunk? If > so, I will check if we need it in GCC 8 and if so, backport it there > too. > > Thanks, > > Martin > > > 2019-04-11 Martin Jambor <mjambor@suse.cz> > > PR ipa/89693 > * cgraph.c (clone_of_p): Loop over clone chain for each step in > the thunk chain. > > testsuite/ > * g++.dg/ipa/pr89693.C: New test. Thanks, the patch is OK. If this verification turns out to be too much trouble, i would be for removing it (it really need to second guess all sane transformations do by all passes we have) Honza > --- > gcc/cgraph.c | 30 ++++++++++------- > gcc/testsuite/g++.dg/ipa/pr89693.C | 52 ++++++++++++++++++++++++++++++ > 2 files changed, 70 insertions(+), 12 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ipa/pr89693.C > > diff --git a/gcc/cgraph.c b/gcc/cgraph.c > index 49d80ad1e28..b1b0b4c42d5 100644 > --- a/gcc/cgraph.c > +++ b/gcc/cgraph.c > @@ -2977,17 +2977,25 @@ cgraph_node::collect_callers (void) > static bool > clone_of_p (cgraph_node *node, cgraph_node *node2) > { > - bool skipped_thunk = false; > node = node->ultimate_alias_target (); > node2 = node2->ultimate_alias_target (); > > + if (node2->clone_of == node > + || node2->former_clone_of == node->decl) > + return true; > + > + if (!node->thunk.thunk_p && !node->former_thunk_p ()) > + { > + while (node2 && node->decl != node2->decl) > + node2 = node2->clone_of; > + return node2 != NULL; > + } > + > /* There are no virtual clones of thunks so check former_clone_of or > if we > might have skipped thunks because this adjustments are no longer > necessary. */ > while (node->thunk.thunk_p || node->former_thunk_p ()) > { > - if (node2->former_clone_of == node->decl) > - return true; > if (!node->thunk.this_adjusting) > return false; > /* In case of instrumented expanded thunks, which can have > multiple calls > @@ -2996,23 +3004,21 @@ clone_of_p (cgraph_node *node, cgraph_node > *node2) > if (node->callees->next_callee) > return true; > node = node->callees->callee->ultimate_alias_target (); > - skipped_thunk = true; > - } > > - if (skipped_thunk) > - { > if (!node2->clone.args_to_skip > || !bitmap_bit_p (node2->clone.args_to_skip, 0)) > return false; > if (node2->former_clone_of == node->decl) > return true; > - else if (!node2->clone_of) > - return false; > + > + cgraph_node *n2 = node2; > + while (n2 && node->decl != n2->decl) > + n2 = n2->clone_of; > + if (n2) > + return true; > } > > - while (node2 && node->decl != node2->decl) > - node2 = node2->clone_of; > - return node2 != NULL; > + return false; > } > > /* Verify edge count and frequency. */ > diff --git a/gcc/testsuite/g++.dg/ipa/pr89693.C > b/gcc/testsuite/g++.dg/ipa/pr89693.C > new file mode 100644 > index 00000000000..4ac83eeeb3a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ipa/pr89693.C > @@ -0,0 +1,52 @@ > +// Copyright (C) 2005 Free Software Foundation, Inc. > +// Contributed by Nathan Sidwell 4 Apr 2005 <nathan@codesourcery.com> > +// Re-purposed to check for re-rurgesnce of PR 89693 in 2019. > + > +// { dg-do compile } > +// { dg-options "-O3 -fno-ipa-icf-functions" } > + > +// Origin: yanliu@ca.ibm.com > +// nathan@codesourcery.com > + > +struct A { > + virtual void One (); > +}; > +struct B { > + virtual B *Two (); > + virtual B &Three (); > +}; > + > +struct C : A, B > +{ > + virtual C *Two (); > + virtual C &Three (); > +}; > +void A::One () {} > +B *B::Two() {return this;} > +B &B::Three() {return *this;} > +C *C::Two () {return 0;} > +C &C::Three () {return *(C *)0;} > + > +B *Foo (B *b) > +{ > + return b->Two (); > +} > + > +B &Bar (B *b) > +{ > + return b->Three (); > +} > + > +int main () > +{ > + C c; > + > + /* We should not adjust a null pointer. */ > + if (Foo (&c)) > + return 1; > + /* But we should adjust a (bogus) null reference. */ > + if (!&Bar (&c)) > + return 2; > + > + return 0; > +}
diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 49d80ad1e28..b1b0b4c42d5 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -2977,17 +2977,25 @@ cgraph_node::collect_callers (void) static bool clone_of_p (cgraph_node *node, cgraph_node *node2) { - bool skipped_thunk = false; node = node->ultimate_alias_target (); node2 = node2->ultimate_alias_target (); + if (node2->clone_of == node + || node2->former_clone_of == node->decl) + return true; + + if (!node->thunk.thunk_p && !node->former_thunk_p ()) + { + while (node2 && node->decl != node2->decl) + node2 = node2->clone_of; + return node2 != NULL; + } + /* There are no virtual clones of thunks so check former_clone_of or if we might have skipped thunks because this adjustments are no longer necessary. */ while (node->thunk.thunk_p || node->former_thunk_p ()) { - if (node2->former_clone_of == node->decl) - return true; if (!node->thunk.this_adjusting) return false; /* In case of instrumented expanded thunks, which can have multiple calls @@ -2996,23 +3004,21 @@ clone_of_p (cgraph_node *node, cgraph_node *node2) if (node->callees->next_callee) return true; node = node->callees->callee->ultimate_alias_target (); - skipped_thunk = true; - } - if (skipped_thunk) - { if (!node2->clone.args_to_skip || !bitmap_bit_p (node2->clone.args_to_skip, 0)) return false; if (node2->former_clone_of == node->decl) return true; - else if (!node2->clone_of) - return false; + + cgraph_node *n2 = node2; + while (n2 && node->decl != n2->decl) + n2 = n2->clone_of; + if (n2) + return true; } - while (node2 && node->decl != node2->decl) - node2 = node2->clone_of; - return node2 != NULL; + return false; } /* Verify edge count and frequency. */ diff --git a/gcc/testsuite/g++.dg/ipa/pr89693.C b/gcc/testsuite/g++.dg/ipa/pr89693.C new file mode 100644 index 00000000000..4ac83eeeb3a --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr89693.C @@ -0,0 +1,52 @@ +// Copyright (C) 2005 Free Software Foundation, Inc. +// Contributed by Nathan Sidwell 4 Apr 2005 <nathan@codesourcery.com> +// Re-purposed to check for re-rurgesnce of PR 89693 in 2019. + +// { dg-do compile } +// { dg-options "-O3 -fno-ipa-icf-functions" } + +// Origin: yanliu@ca.ibm.com +// nathan@codesourcery.com + +struct A { + virtual void One (); +}; +struct B { + virtual B *Two (); + virtual B &Three (); +}; + +struct C : A, B +{ + virtual C *Two (); + virtual C &Three (); +}; +void A::One () {} +B *B::Two() {return this;} +B &B::Three() {return *this;} +C *C::Two () {return 0;} +C &C::Three () {return *(C *)0;} + +B *Foo (B *b) +{ + return b->Two (); +} + +B &Bar (B *b) +{ + return b->Three (); +} + +int main () +{ + C c; + + /* We should not adjust a null pointer. */ + if (Foo (&c)) + return 1; + /* But we should adjust a (bogus) null reference. */ + if (!&Bar (&c)) + return 2; + + return 0; +}