diff mbox

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

Message ID 20101211170002.GB8468@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Dec. 11, 2010, 5 p.m. UTC 
> > I lost the flag_reorder_functions check in my patch, will add it back into
> > default_function_section and dwarf2_function_section code.
> 
> Ah, that explains why I started seeing the sections associated with
> section reordering in default compilations without options.

-freorder-functions is now declared as { OPT_LEVELS_2_PLUS, OPT_freorder_functions, NULL, 1 },
I do not see why this should not be default for -O1 too - it is cheap
compilation time wise.  I am attaching the fix for flag_reorder_functions I
intend to commit tonight as obvious if there are no complains
> 
> We previously (4.5 branch) had the following code in choose_function_section:
> 
>   if (DECL_SECTION_NAME (current_function_decl)
>       || !targetm.have_named_sections
>       /* Theoretically we can split the gnu.linkonce text section too,
>       but this requires more work as the frequency needs to match
>       for all generated objects so we need to merge the frequency
>       of all instances.  For now just never set frequency for these.  */
>       || DECL_ONE_ONLY (current_function_decl))
>    return;

I see, I missed the targetm.have_named_sections.  I guess the proposed patch still
makes sense in this context.
> 
>    /* If we are doing the partitioning optimization, let the optimization
>       choose the correct section into which to put things.  */
> 
>    if (flag_reorder_blocks_and_partition)
>      return;
> 
> It appears functions with a named section are correctly handled by the
> new code by name concatenation.  Don't know about DECL_ONE_ONLY and
> flag_reorder_blocks_and_partition (latter is disabled on PA).

The concatenation code comes from flag_reorder_blocks_and_partition code.
there is code checking if -freorder-blocks-and-partition is supposed and
it also uses the targetm.have_named_sections.  So I suppose on named section
targets we did the renaming.

I am however not too opposed to making this default for ELF systems only.
We already handle this specially on darwin, that leaves us with cygwin
as a target where we probably really care?

Honza

	* varasm.c (default_function_section): Return NULL when
	flag_reorder_functions is disabled.
	* darwin.c (darwin_function_section): Likewise.

Comments

Jan Hubicka Dec. 11, 2010, 6:09 p.m. UTC | #1 
> 
> In my opinion, Rainer's change should also be committed as obvious.

I agree, I was just concerned bz this thread about non-elf targets.  So I will merge
both patches, re-test and commit.
Thanks!
Honza
> 
> Dave
> -- 
> J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
diff mbox

Patch

Index: varasm.c
===================================================================
--- varasm.c	(revision 167472)
+++ varasm.c	(working copy)
@@ -533,6 +533,8 @@  section *
 default_function_section (tree decl, enum node_frequency freq,
 			  bool startup, bool exit)
 {
+  if (!flag_reorder_functions)
+    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.  */
Index: config/darwin.c
===================================================================
--- config/darwin.c	(revision 167472)
+++ config/darwin.c	(working copy)
@@ -2970,6 +2970,8 @@  section *
 darwin_function_section (tree decl, enum node_frequency freq,
 			  bool startup, bool exit)
 {
+  if (!flag_reorder_functions)
+    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.  */