Patchwork Restore Tru64 UNIX bootstrap (PR middle-end/46671)

login
register
mail settings
Submitter Rainer Orth
Date Dec. 10, 2010, 5:29 p.m.
Message ID <yddei9peduf.fsf@manam.CeBiTec.Uni-Bielefeld.DE>
Download mbox | patch
Permalink /patch/75113/
State New
Headers show

Comments

Rainer Orth - Dec. 10, 2010, 5:29 p.m.
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.
Jakub Jelinek - Dec. 10, 2010, 5:32 p.m.
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 Hubicka - Dec. 11, 2010, 2:03 p.m.
> 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
Jan Hubicka - Dec. 11, 2010, 4:04 p.m.
> > > 	* 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)

Patch

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.  */