| Message ID | yddei9peduf.fsf@manam.CeBiTec.Uni-Bielefeld.DE |
|---|---|
| State | New |
| Headers | show |
On Fri, Dec 10, 2010 at 06:29:12PM +0100, Rainer Orth wrote: > 2010-12-09 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> > > PR middle-end/46671 > * varasm.c (default_function_section): Return NULL unless > targetm.have_named_sections. > This: > * regrename.c (check_new_reg_p): Mark reg with ATTRIBUTE_UNUSED. hunk is PR46844, fixed for 2 days already... > > diff -r 0168da119e76 gcc/regrename.c > --- a/gcc/regrename.c Fri Dec 10 17:50:52 2010 +0100 > +++ b/gcc/regrename.c Fri Dec 10 17:52:57 2010 +0100 > @@ -309,8 +309,8 @@ > registers. */ > > static bool > -check_new_reg_p (int reg, int new_reg, struct du_head *this_head, > - HARD_REG_SET this_unavailable) > +check_new_reg_p (int reg ATTRIBUTE_UNUSED, int new_reg, > + struct du_head *this_head, HARD_REG_SET this_unavailable) > { > enum machine_mode mode = GET_MODE (*this_head->first->loc); > int nregs = hard_regno_nregs[new_reg][mode]; Jakub
> Jan's patch > > Re: Group static constructors and destructors in specific subsections, take 2 > http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00983.html > > broke bootstrap on every target without named sections since varasm.c > (default_function_section) assumes they exist. While two port > maintainers so far have worked around the breakage in their ports > (pdp11, hppa) since Jan completely failed to address (or even > acknowledge) the breakage he's caused, I think the fix belongs into > default_function_section to make that function an appropriate default. Sorry, I was offline for last week and didn't noticed the problem before leaving. Yes, it is indeed problem introduce by my patch. > * varasm.c (default_function_section): Return NULL unless > targetm.have_named_sections. > --- a/gcc/varasm.c Fri Dec 10 17:50:52 2010 +0100 > +++ b/gcc/varasm.c Fri Dec 10 17:52:57 2010 +0100 > @@ -533,6 +533,9 @@ > default_function_section (tree decl, enum node_frequency freq, > bool startup, bool exit) > { > + if (!targetm.have_named_sections) > + return NULL; Yes, this seems sane default for me. Thanks! Honza > + > /* Startup code should go to startup subsection unless it is > unlikely executed (this happens especially with function splitting > where we can split away unnecesary parts of static constructors. */ > > > -- > ----------------------------------------------------------------------------- > Rainer Orth, Center for Biotechnology, Bielefeld University
> > > * varasm.c (default_function_section): Return NULL unless > > > targetm.have_named_sections. > > > --- a/gcc/varasm.c Fri Dec 10 17:50:52 2010 +0100 > > > +++ b/gcc/varasm.c Fri Dec 10 17:52:57 2010 +0100 > > > @@ -533,6 +533,9 @@ > > > default_function_section (tree decl, enum node_frequency freq, > > > bool startup, bool exit) > > > { > > > + if (!targetm.have_named_sections) > > > + return NULL; > > > > Yes, this seems sane default for me. > > I think this doesn't go far enough in restoring previous behaviour. > There may be targets with named sections that aren't ELF. > > The current implementation of default_function_section is really > ELF/GNU ld specific. I think it would be better if it were changed > to elf_default_function_section selected in elfos.h. The default for OK, I see that currently only GNU linker does the ordering, but the previous code was enabled by default on non-elf targets too? choose_function was called with flag_reorder_function and set DECL_NAMED_SECTION that in my understanding affects all targets, elf or non-elf. I lost the flag_reorder_functions check in my patch, will add it back into default_function_section and dwarf2_function_section code. Honza > TARGET_ASM_FUNCTION_SECTION could be NULL so the hook isn't called > at all for the default case. The code would then fall through to > hot_function_section. This probably could be further simplified. > > I am probably stuck in defining TARGET_ASM_FUNCTION_SECTION on PA > because of a problem in handling nested functions when they are in > different sections... > > Dave > -- > J. David Anglin dave.anglin@nrc-cnrc.gc.ca > National Research Council of Canada (613) 990-0752 (FAX: 952-6602)
diff -r 0168da119e76 gcc/regrename.c --- a/gcc/regrename.c Fri Dec 10 17:50:52 2010 +0100 +++ b/gcc/regrename.c Fri Dec 10 17:52:57 2010 +0100 @@ -309,8 +309,8 @@ registers. */ static bool -check_new_reg_p (int reg, int new_reg, struct du_head *this_head, - HARD_REG_SET this_unavailable) +check_new_reg_p (int reg ATTRIBUTE_UNUSED, int new_reg, + struct du_head *this_head, HARD_REG_SET this_unavailable) { enum machine_mode mode = GET_MODE (*this_head->first->loc); int nregs = hard_regno_nregs[new_reg][mode]; diff -r 0168da119e76 gcc/varasm.c --- a/gcc/varasm.c Fri Dec 10 17:50:52 2010 +0100 +++ b/gcc/varasm.c Fri Dec 10 17:52:57 2010 +0100 @@ -533,6 +533,9 @@ default_function_section (tree decl, enum node_frequency freq, bool startup, bool exit) { + if (!targetm.have_named_sections) + return NULL; + /* Startup code should go to startup subsection unless it is unlikely executed (this happens especially with function splitting where we can split away unnecesary parts of static constructors. */
Jan's patch Re: Group static constructors and destructors in specific subsections, take 2 http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00983.html broke bootstrap on every target without named sections since varasm.c (default_function_section) assumes they exist. While two port maintainers so far have worked around the breakage in their ports (pdp11, hppa) since Jan completely failed to address (or even acknowledge) the breakage he's caused, I think the fix belongs into default_function_section to make that function an appropriate default. The patch below does this, and also includes another obvious fix to allow the bootstrap to continue. Bootstrap has finished with the patch. While testing is still underway, there were no unusual testsuite failures so far. Ok for mainline? Rainer 2010-12-09 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> PR middle-end/46671 * varasm.c (default_function_section): Return NULL unless targetm.have_named_sections. * regrename.c (check_new_reg_p): Mark reg with ATTRIBUTE_UNUSED.