* ICF on PowerPC Bug
@ 2014-04-01 22:06 Sriraman Tallam
2014-04-02 6:54 ` Alan Modra
0 siblings, 1 reply; 6+ messages in thread
From: Sriraman Tallam @ 2014-04-01 22:06 UTC (permalink / raw)
To: Alan Modra, binutils, Ian Lance Taylor
Hi,
This program fails with ICF on powerpc:
__attribute__((noinline))
int foo () { return 1; }
__attribute__((noinline))
int bar () { return 0; }
__attribute__((noinline))
int fold1() {
return foo ();
}
__attribute__((noinline))
int fold2() {
return bar();
}
int main()
{
assert (fold1() != fold2());
}
because fold2 is folded onto fold1. The only way to differentiate
fold1 and fold2 which have the same object code is via the relocation
type and the Info value is different. However, with powerpc this is
rewritten here in icf.cc:
// Look through function descriptors
parameters->target().function_location(&loc);
if (loc.shndx != it_v->second)
{
it_v->second = loc.shndx;
// Modify symvalue/addend to the code entry.
it_a->first = loc.offset;
it_a->second = 0;
}
I am not sure how to fix it.
Thanks
Sri
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: ICF on PowerPC Bug 2014-04-01 22:06 ICF on PowerPC Bug Sriraman Tallam @ 2014-04-02 6:54 ` Alan Modra 2014-04-02 16:49 ` Sriraman Tallam 0 siblings, 1 reply; 6+ messages in thread From: Alan Modra @ 2014-04-02 6:54 UTC (permalink / raw) To: Sriraman Tallam; +Cc: binutils, Ian Lance Taylor On Tue, Apr 01, 2014 at 03:06:37PM -0700, Sriraman Tallam wrote: > because fold2 is folded onto fold1. The only way to differentiate > fold1 and fold2 which have the same object code is via the relocation > type and the Info value is different. However, with powerpc this is > rewritten here in icf.cc: > > // Look through function descriptors > parameters->target().function_location(&loc); > if (loc.shndx != it_v->second) > { > it_v->second = loc.shndx; > // Modify symvalue/addend to the code entry. > it_a->first = loc.offset; > it_a->second = 0; > } > > I am not sure how to fix it. The following patch fixes the problem, but did you really mean to copy all the vectors here? Icf::Sections_reachable_info v = (it_reloc_info_list->second).section_info; // Stores the information of the symbol pointed to by the reloc. Icf::Symbol_info s = (it_reloc_info_list->second).symbol_info; // Stores the addend and the symbol value. Icf::Addend_info a = (it_reloc_info_list->second).addend_info; // Stores the offset of the reloc. Icf::Offset_info o = (it_reloc_info_list->second).offset_info; Icf::Reloc_addend_size_info reloc_addend_size_info = (it_reloc_info_list->second).reloc_addend_size_info; diff --git a/gold/icf.cc b/gold/icf.cc index f30eb41..920514c 100644 --- a/gold/icf.cc +++ b/gold/icf.cc @@ -288,8 +288,7 @@ get_section_contents(bool first_iteration, for (; it_v != v.end(); ++it_v, ++it_s, ++it_a, ++it_o, ++it_addend_size) { - if (first_iteration - && it_v->first != NULL) + if (it_v->first != NULL) { Symbol_location loc; loc.object = it_v->first; -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICF on PowerPC Bug 2014-04-02 6:54 ` Alan Modra @ 2014-04-02 16:49 ` Sriraman Tallam 2014-04-02 19:11 ` Sriraman Tallam 0 siblings, 1 reply; 6+ messages in thread From: Sriraman Tallam @ 2014-04-02 16:49 UTC (permalink / raw) To: Sriraman Tallam, binutils, Ian Lance Taylor On Tue, Apr 1, 2014 at 11:54 PM, Alan Modra <amodra@gmail.com> wrote: > On Tue, Apr 01, 2014 at 03:06:37PM -0700, Sriraman Tallam wrote: >> because fold2 is folded onto fold1. The only way to differentiate >> fold1 and fold2 which have the same object code is via the relocation >> type and the Info value is different. However, with powerpc this is >> rewritten here in icf.cc: >> >> // Look through function descriptors >> parameters->target().function_location(&loc); >> if (loc.shndx != it_v->second) >> { >> it_v->second = loc.shndx; >> // Modify symvalue/addend to the code entry. >> it_a->first = loc.offset; >> it_a->second = 0; >> } >> >> I am not sure how to fix it. > > The following patch fixes the problem, but did you really mean to copy > all the vectors here? Yikes!, no did not intend that. I will make these references and send a patch. Thanks Sri > > Icf::Sections_reachable_info v = > (it_reloc_info_list->second).section_info; > // Stores the information of the symbol pointed to by the reloc. > Icf::Symbol_info s = (it_reloc_info_list->second).symbol_info; > // Stores the addend and the symbol value. > Icf::Addend_info a = (it_reloc_info_list->second).addend_info; > // Stores the offset of the reloc. > Icf::Offset_info o = (it_reloc_info_list->second).offset_info; > Icf::Reloc_addend_size_info reloc_addend_size_info = > (it_reloc_info_list->second).reloc_addend_size_info; > > > diff --git a/gold/icf.cc b/gold/icf.cc > index f30eb41..920514c 100644 > --- a/gold/icf.cc > +++ b/gold/icf.cc > @@ -288,8 +288,7 @@ get_section_contents(bool first_iteration, > > for (; it_v != v.end(); ++it_v, ++it_s, ++it_a, ++it_o, ++it_addend_size) > { > - if (first_iteration > - && it_v->first != NULL) > + if (it_v->first != NULL) > { > Symbol_location loc; > loc.object = it_v->first; > > -- > Alan Modra > Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICF on PowerPC Bug 2014-04-02 16:49 ` Sriraman Tallam @ 2014-04-02 19:11 ` Sriraman Tallam 2014-04-02 21:12 ` Cary Coutant 0 siblings, 1 reply; 6+ messages in thread From: Sriraman Tallam @ 2014-04-02 19:11 UTC (permalink / raw) To: Sriraman Tallam, binutils, Ian Lance Taylor, Cary Coutant [-- Attachment #1: Type: text/plain, Size: 2270 bytes --] Attached patch to fix the PPC ICF problem by using references to the reloc info vectors. Ok to commit? Thanks Sri On Wed, Apr 2, 2014 at 9:49 AM, Sriraman Tallam <tmsriram@google.com> wrote: > On Tue, Apr 1, 2014 at 11:54 PM, Alan Modra <amodra@gmail.com> wrote: >> On Tue, Apr 01, 2014 at 03:06:37PM -0700, Sriraman Tallam wrote: >>> because fold2 is folded onto fold1. The only way to differentiate >>> fold1 and fold2 which have the same object code is via the relocation >>> type and the Info value is different. However, with powerpc this is >>> rewritten here in icf.cc: >>> >>> // Look through function descriptors >>> parameters->target().function_location(&loc); >>> if (loc.shndx != it_v->second) >>> { >>> it_v->second = loc.shndx; >>> // Modify symvalue/addend to the code entry. >>> it_a->first = loc.offset; >>> it_a->second = 0; >>> } >>> >>> I am not sure how to fix it. >> >> The following patch fixes the problem, but did you really mean to copy >> all the vectors here? > > Yikes!, no did not intend that. I will make these references and send a patch. > > Thanks > Sri > >> >> Icf::Sections_reachable_info v = >> (it_reloc_info_list->second).section_info; >> // Stores the information of the symbol pointed to by the reloc. >> Icf::Symbol_info s = (it_reloc_info_list->second).symbol_info; >> // Stores the addend and the symbol value. >> Icf::Addend_info a = (it_reloc_info_list->second).addend_info; >> // Stores the offset of the reloc. >> Icf::Offset_info o = (it_reloc_info_list->second).offset_info; >> Icf::Reloc_addend_size_info reloc_addend_size_info = >> (it_reloc_info_list->second).reloc_addend_size_info; >> >> >> diff --git a/gold/icf.cc b/gold/icf.cc >> index f30eb41..920514c 100644 >> --- a/gold/icf.cc >> +++ b/gold/icf.cc >> @@ -288,8 +288,7 @@ get_section_contents(bool first_iteration, >> >> for (; it_v != v.end(); ++it_v, ++it_s, ++it_a, ++it_o, ++it_addend_size) >> { >> - if (first_iteration >> - && it_v->first != NULL) >> + if (it_v->first != NULL) >> { >> Symbol_location loc; >> loc.object = it_v->first; >> >> -- >> Alan Modra >> Australia Development Lab, IBM [-- Attachment #2: icf_fix.txt --] [-- Type: text/plain, Size: 1482 bytes --] * icf.cc (get_section_contents): Use references to reloc_info vectors to avoid copies. Index: icf.cc =================================================================== RCS file: /cvs/src/src/gold/icf.cc,v retrieving revision 1.21 diff -u -p -r1.21 icf.cc --- icf.cc 15 Mar 2013 07:51:32 -0000 1.21 +++ icf.cc 2 Apr 2014 19:08:28 -0000 @@ -269,15 +269,15 @@ get_section_contents(bool first_iteratio if (it_reloc_info_list != reloc_info_list.end()) { - Icf::Sections_reachable_info v = + Icf::Sections_reachable_info &v = (it_reloc_info_list->second).section_info; // Stores the information of the symbol pointed to by the reloc. - Icf::Symbol_info s = (it_reloc_info_list->second).symbol_info; + Icf::Symbol_info &s = (it_reloc_info_list->second).symbol_info; // Stores the addend and the symbol value. - Icf::Addend_info a = (it_reloc_info_list->second).addend_info; + Icf::Addend_info &a = (it_reloc_info_list->second).addend_info; // Stores the offset of the reloc. - Icf::Offset_info o = (it_reloc_info_list->second).offset_info; - Icf::Reloc_addend_size_info reloc_addend_size_info = + Icf::Offset_info &o = (it_reloc_info_list->second).offset_info; + Icf::Reloc_addend_size_info &reloc_addend_size_info = (it_reloc_info_list->second).reloc_addend_size_info; Icf::Sections_reachable_info::iterator it_v = v.begin(); Icf::Symbol_info::iterator it_s = s.begin(); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICF on PowerPC Bug 2014-04-02 19:11 ` Sriraman Tallam @ 2014-04-02 21:12 ` Cary Coutant 2014-04-03 0:11 ` Sriraman Tallam 0 siblings, 1 reply; 6+ messages in thread From: Cary Coutant @ 2014-04-02 21:12 UTC (permalink / raw) To: Sriraman Tallam; +Cc: binutils, Ian Lance Taylor > Attached patch to fix the PPC ICF problem by using references to the > reloc info vectors. Ok to commit? - Icf::Sections_reachable_info v = + Icf::Sections_reachable_info &v = (it_reloc_info_list->second).section_info; // Stores the information of the symbol pointed to by the reloc. - Icf::Symbol_info s = (it_reloc_info_list->second).symbol_info; + Icf::Symbol_info &s = (it_reloc_info_list->second).symbol_info; // Stores the addend and the symbol value. - Icf::Addend_info a = (it_reloc_info_list->second).addend_info; + Icf::Addend_info &a = (it_reloc_info_list->second).addend_info; // Stores the offset of the reloc. - Icf::Offset_info o = (it_reloc_info_list->second).offset_info; - Icf::Reloc_addend_size_info reloc_addend_size_info = + Icf::Offset_info &o = (it_reloc_info_list->second).offset_info; + Icf::Reloc_addend_size_info &reloc_addend_size_info = (it_reloc_info_list->second).reloc_addend_size_info; Icf::Sections_reachable_info::iterator it_v = v.begin(); Icf::Symbol_info::iterator it_s = s.begin(); Except for v and a, I think these can all be const, and their iterators can be const_iterators. OK with those changes. Thanks! -cary ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICF on PowerPC Bug 2014-04-02 21:12 ` Cary Coutant @ 2014-04-03 0:11 ` Sriraman Tallam 0 siblings, 0 replies; 6+ messages in thread From: Sriraman Tallam @ 2014-04-03 0:11 UTC (permalink / raw) To: Cary Coutant; +Cc: binutils, Ian Lance Taylor Committed with those changes. Thanks Sri On Wed, Apr 2, 2014 at 2:12 PM, Cary Coutant <ccoutant@google.com> wrote: >> Attached patch to fix the PPC ICF problem by using references to the >> reloc info vectors. Ok to commit? > > - Icf::Sections_reachable_info v = > + Icf::Sections_reachable_info &v = > (it_reloc_info_list->second).section_info; > // Stores the information of the symbol pointed to by the reloc. > - Icf::Symbol_info s = (it_reloc_info_list->second).symbol_info; > + Icf::Symbol_info &s = (it_reloc_info_list->second).symbol_info; > // Stores the addend and the symbol value. > - Icf::Addend_info a = (it_reloc_info_list->second).addend_info; > + Icf::Addend_info &a = (it_reloc_info_list->second).addend_info; > // Stores the offset of the reloc. > - Icf::Offset_info o = (it_reloc_info_list->second).offset_info; > - Icf::Reloc_addend_size_info reloc_addend_size_info = > + Icf::Offset_info &o = (it_reloc_info_list->second).offset_info; > + Icf::Reloc_addend_size_info &reloc_addend_size_info = > (it_reloc_info_list->second).reloc_addend_size_info; > Icf::Sections_reachable_info::iterator it_v = v.begin(); > Icf::Symbol_info::iterator it_s = s.begin(); > > Except for v and a, I think these can all be const, and their > iterators can be const_iterators. OK with those changes. > > Thanks! > > -cary ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-03 0:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-04-01 22:06 ICF on PowerPC Bug Sriraman Tallam 2014-04-02 6:54 ` Alan Modra 2014-04-02 16:49 ` Sriraman Tallam 2014-04-02 19:11 ` Sriraman Tallam 2014-04-02 21:12 ` Cary Coutant 2014-04-03 0:11 ` Sriraman Tallam
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).