| Message ID | 1421721072-6962-1-git-send-email-tbsaunde+gcc@tbsaunde.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 20, 2015 at 3:31 AM, <tbsaunde+gcc@tbsaunde.org> wrote: > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > Hi, > > when doing an lto link we can have some symbols be ir only and others be > machine code, which trips the assert here. Just adjust the assert to handle > that. > > bootstrapped + regtested x86_64-linux-gnu, ok? Testcase? It's hard to understand what "machine code" is otherwise or why this assert would fail. Thanks, Richard. > Trev > > gcc/ > > * ipa-visibility.c (update_visibility_by_resolution_info): Only > assert when not in lto mode. > --- > gcc/ipa-visibility.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c > index 71894af..0791a1c 100644 > --- a/gcc/ipa-visibility.c > +++ b/gcc/ipa-visibility.c > @@ -425,11 +425,19 @@ update_visibility_by_resolution_info (symtab_node * node) > if (node->same_comdat_group) > for (symtab_node *next = node->same_comdat_group; > next != node; next = next->same_comdat_group) > - gcc_assert (!next->externally_visible > - || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY > - || next->resolution == LDPR_PREVAILING_DEF > - || next->resolution == LDPR_UNDEF > - || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)); > + { > + if (!next->externally_visible) > + continue; > + > + bool same_def > + = define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY > + || next->resolution == LDPR_PREVAILING_DEF > + || next->resolution == LDPR_UNDEF > + || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP); > + gcc_assert (in_lto_p || same_def); > + if (!same_def) > + return; > + } > > if (node->same_comdat_group) > for (symtab_node *next = node->same_comdat_group; > -- > 2.1.4 >
On Tue, Jan 20, 2015 at 10:18:25AM +0100, Richard Biener wrote: > On Tue, Jan 20, 2015 at 3:31 AM, <tbsaunde+gcc@tbsaunde.org> wrote: > > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > > > Hi, > > > > when doing an lto link we can have some symbols be ir only and others be > > machine code, which trips the assert here. Just adjust the assert to handle > > that. > > > > bootstrapped + regtested x86_64-linux-gnu, ok? > > Testcase? It's hard to understand what "machine code" is otherwise > or why this assert would fail. > > Thanks, the one in the pr, basically one file is compiled without -flto and then linked with -flto that + an odr violation making the two object files different sets of thunks. does the lto test suite support not passing -flto to one compilation? it wasn't clear to me how to put the test case in the testsuite. Trev > Richard. > > > Trev > > > > gcc/ > > > > * ipa-visibility.c (update_visibility_by_resolution_info): Only > > assert when not in lto mode. > > --- > > gcc/ipa-visibility.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c > > index 71894af..0791a1c 100644 > > --- a/gcc/ipa-visibility.c > > +++ b/gcc/ipa-visibility.c > > @@ -425,11 +425,19 @@ update_visibility_by_resolution_info (symtab_node * node) > > if (node->same_comdat_group) > > for (symtab_node *next = node->same_comdat_group; > > next != node; next = next->same_comdat_group) > > - gcc_assert (!next->externally_visible > > - || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY > > - || next->resolution == LDPR_PREVAILING_DEF > > - || next->resolution == LDPR_UNDEF > > - || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)); > > + { > > + if (!next->externally_visible) > > + continue; > > + > > + bool same_def > > + = define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY > > + || next->resolution == LDPR_PREVAILING_DEF > > + || next->resolution == LDPR_UNDEF > > + || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP); > > + gcc_assert (in_lto_p || same_def); > > + if (!same_def) > > + return; > > + } > > > > if (node->same_comdat_group) > > for (symtab_node *next = node->same_comdat_group; > > -- > > 2.1.4 > >
On Tue, Jan 20, 2015 at 3:15 PM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote: > On Tue, Jan 20, 2015 at 10:18:25AM +0100, Richard Biener wrote: >> On Tue, Jan 20, 2015 at 3:31 AM, <tbsaunde+gcc@tbsaunde.org> wrote: >> > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> >> > >> > Hi, >> > >> > when doing an lto link we can have some symbols be ir only and others be >> > machine code, which trips the assert here. Just adjust the assert to handle >> > that. >> > >> > bootstrapped + regtested x86_64-linux-gnu, ok? >> >> Testcase? It's hard to understand what "machine code" is otherwise >> or why this assert would fail. >> >> Thanks, > > the one in the pr, basically one file is compiled without -flto and then > linked with -flto that + an odr violation making the two object files > different sets of thunks. > > does the lto test suite support not passing -flto to one compilation? it > wasn't clear to me how to put the test case in the testsuite. Yes, simply add { dg-options -fno-lto } to one of the secondary files. Richard. > Trev > >> Richard. >> >> > Trev >> > >> > gcc/ >> > >> > * ipa-visibility.c (update_visibility_by_resolution_info): Only >> > assert when not in lto mode. >> > --- >> > gcc/ipa-visibility.c | 18 +++++++++++++----- >> > 1 file changed, 13 insertions(+), 5 deletions(-) >> > >> > diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c >> > index 71894af..0791a1c 100644 >> > --- a/gcc/ipa-visibility.c >> > +++ b/gcc/ipa-visibility.c >> > @@ -425,11 +425,19 @@ update_visibility_by_resolution_info (symtab_node * node) >> > if (node->same_comdat_group) >> > for (symtab_node *next = node->same_comdat_group; >> > next != node; next = next->same_comdat_group) >> > - gcc_assert (!next->externally_visible >> > - || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY >> > - || next->resolution == LDPR_PREVAILING_DEF >> > - || next->resolution == LDPR_UNDEF >> > - || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)); >> > + { >> > + if (!next->externally_visible) >> > + continue; >> > + >> > + bool same_def >> > + = define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY >> > + || next->resolution == LDPR_PREVAILING_DEF >> > + || next->resolution == LDPR_UNDEF >> > + || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP); >> > + gcc_assert (in_lto_p || same_def); >> > + if (!same_def) >> > + return; >> > + } >> > >> > if (node->same_comdat_group) >> > for (symtab_node *next = node->same_comdat_group; >> > -- >> > 2.1.4 >> >
diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c index 71894af..0791a1c 100644 --- a/gcc/ipa-visibility.c +++ b/gcc/ipa-visibility.c @@ -425,11 +425,19 @@ update_visibility_by_resolution_info (symtab_node * node) if (node->same_comdat_group) for (symtab_node *next = node->same_comdat_group; next != node; next = next->same_comdat_group) - gcc_assert (!next->externally_visible - || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY - || next->resolution == LDPR_PREVAILING_DEF - || next->resolution == LDPR_UNDEF - || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)); + { + if (!next->externally_visible) + continue; + + bool same_def + = define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY + || next->resolution == LDPR_PREVAILING_DEF + || next->resolution == LDPR_UNDEF + || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP); + gcc_assert (in_lto_p || same_def); + if (!same_def) + return; + } if (node->same_comdat_group) for (symtab_node *next = node->same_comdat_group;
From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> Hi, when doing an lto link we can have some symbols be ir only and others be machine code, which trips the assert here. Just adjust the assert to handle that. bootstrapped + regtested x86_64-linux-gnu, ok? Trev gcc/ * ipa-visibility.c (update_visibility_by_resolution_info): Only assert when not in lto mode. --- gcc/ipa-visibility.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)