diff mbox

Group static constructors and destructors in specific subsections, take 2

Message ID 20101123235749.GA13018@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Nov. 23, 2010, 11:57 p.m. UTC
> 
> This caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46628

Hi,
this problem is caused by dwarf2out trying to get into cold section to output
starting and ending label.  This is no longer so easy since one needs a
function declaration to get into cold section, otherwise we end up producing
cold section with data section flags.

This patch makes dwarf2out to behave roughly as before by waiting with
switching to the cold section until first function is output.

I think this is just latent problem since multiple cold section will lead to
bad debug info. (wilth -ffunction-sections -fpartition-functions-and-reorder).
How we should handle this case?  Does this patch seems resonable to fix the
regression?

Bootstrapped/regtested x86_64-linux.  My apologizes for the breakage.

Honza

	* dwarf2out.c (dwarf2out_begin_function): Set cold_text_section
	and output cold_text_section_label.
	(dwarf2out_init): Don't do that there.
	(dwarf2out_finish): Handle cold section end label only if cold
	section was used.

Comments

Jan Hubicka Nov. 24, 2010, 4:09 p.m. UTC | #1
Hi,
as Jakub, Tromey and Jason patiently explained to me, the cold_text_section
code in dwarf2out is just optimizations.  Functions in separate sections are
handled elsewhere. So I take back my claims about dwarf2out being broken in
partitioning and attaching the updated patch.  The only difference is in
dwarf2out_begin_function that waits for first function in text section (that
consequently have standard cold sectoin) and initialize vars correspondingly.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* dwarf2out.c (dwarf2out_begin_function): Initialize cold_text_section
	and emit cold_text_section_label.
	(dwarf2out_init): Don't do that there.
	(dwarf2out_finish): Emit end label only when cold_text_section is
	initialized.
Index: dwarf2out.c
===================================================================
*** dwarf2out.c	(revision 167086)
--- dwarf2out.c	(working copy)
*************** dwarf2out_begin_function (tree fun)
*** 21676,21681 ****
--- 21676,21689 ----
  {
    if (function_section (fun) != text_section)
      have_multiple_function_sections = true;
+   else if (flag_reorder_blocks_and_partition && !cold_text_section)
+     {
+       gcc_assert (current_function_decl == fun);
+       cold_text_section = unlikely_text_section ();
+       switch_to_section (cold_text_section);
+       ASM_OUTPUT_LABEL (asm_out_file, cold_text_section_label);
+       switch_to_section (current_function_section ());
+     }
  
    dwarf2out_note_section_used ();
  }
*************** dwarf2out_init (const char *filename ATT
*** 21996,22008 ****
  
    switch_to_section (text_section);
    ASM_OUTPUT_LABEL (asm_out_file, text_section_label);
-   if (flag_reorder_blocks_and_partition)
-     {
-       cold_text_section = unlikely_text_section ();
-       switch_to_section (cold_text_section);
-       ASM_OUTPUT_LABEL (asm_out_file, cold_text_section_label);
-     }
- 
  }
  
  /* Called before cgraph_optimize starts outputtting functions, variables
--- 22004,22009 ----
*************** dwarf2out_finish (const char *filename)
*** 23108,23116 ****
    /* Output a terminator label for the .text section.  */
    switch_to_section (text_section);
    targetm.asm_out.internal_label (asm_out_file, TEXT_END_LABEL, 0);
!   if (flag_reorder_blocks_and_partition)
      {
!       switch_to_section (unlikely_text_section ());
        targetm.asm_out.internal_label (asm_out_file, COLD_END_LABEL, 0);
      }
  
--- 23109,23117 ----
    /* Output a terminator label for the .text section.  */
    switch_to_section (text_section);
    targetm.asm_out.internal_label (asm_out_file, TEXT_END_LABEL, 0);
!   if (cold_text_section)
      {
!       switch_to_section (cold_text_section);
        targetm.asm_out.internal_label (asm_out_file, COLD_END_LABEL, 0);
      }
Richard Henderson Nov. 26, 2010, 4:34 p.m. UTC | #2
On 11/24/2010 08:09 AM, Jan Hubicka wrote:
> Hi,
> as Jakub, Tromey and Jason patiently explained to me, the cold_text_section
> code in dwarf2out is just optimizations.  Functions in separate sections are
> handled elsewhere. So I take back my claims about dwarf2out being broken in
> partitioning and attaching the updated patch.  The only difference is in
> dwarf2out_begin_function that waits for first function in text section (that
> consequently have standard cold sectoin) and initialize vars correspondingly.
> 
> Bootstrapped/regtested x86_64-linux, OK?
> 
> Honza
> 
> 	* dwarf2out.c (dwarf2out_begin_function): Initialize cold_text_section
> 	and emit cold_text_section_label.
> 	(dwarf2out_init): Don't do that there.
> 	(dwarf2out_finish): Emit end label only when cold_text_section is
> 	initialized.
> Index: dwarf2out.c
> ===================================================================
> *** dwarf2out.c	(revision 167086)
> --- dwarf2out.c	(working copy)
> *************** dwarf2out_begin_function (tree fun)
> *** 21676,21681 ****
> --- 21676,21689 ----
>   {
>     if (function_section (fun) != text_section)
>       have_multiple_function_sections = true;
> +   else if (flag_reorder_blocks_and_partition && !cold_text_section)
> +     {
> +       gcc_assert (current_function_decl == fun);
> +       cold_text_section = unlikely_text_section ();
> +       switch_to_section (cold_text_section);
> +       ASM_OUTPUT_LABEL (asm_out_file, cold_text_section_label);
> +       switch_to_section (current_function_section ());
> +     }

Is there a quick way to determine if the function will ever go into the
cold_text_section?  It would be nice if a given translation unit is
entirely non-cold that we never switch into the cold_text_section to
create that label.

If there is no way for that atm, then the patch is ok as-is.  It's
clearly an improvement over the current state of affairs.


r~
Jan Hubicka Nov. 27, 2010, 7:40 p.m. UTC | #3
> 
> Is there a quick way to determine if the function will ever go into the
> cold_text_section?  It would be nice if a given translation unit is
> entirely non-cold that we never switch into the cold_text_section to
> create that label.
> 
> If there is no way for that atm, then the patch is ok as-is.  It's
> clearly an improvement over the current state of affairs.

I didn't noticed any except for walking CFG and looking for COLD_PARTITION
(or perhaps one needs to check only first and last BB as there should
be at most one partition change).
It is trivial to add a flag passed down from bb-reorder if the spurious label
seems like a problem.

I've comitted the patch. Thanks!
Honza
> 
> 
> r~
diff mbox

Patch

Index: dwarf2out.c
===================================================================
--- dwarf2out.c	(revision 167086)
+++ dwarf2out.c	(working copy)
@@ -21674,6 +21674,15 @@  dwarf2out_var_location (rtx loc_note)
 static void
 dwarf2out_begin_function (tree fun)
 {
+  if (flag_reorder_blocks_and_partition && !cold_text_section)
+    {
+      gcc_assert (current_function_decl == fun);
+      cold_text_section = unlikely_text_section ();
+      switch_to_section (cold_text_section);
+      ASM_OUTPUT_LABEL (asm_out_file, cold_text_section_label);
+      switch_to_section (current_function_section ());
+    }
+
   if (function_section (fun) != text_section)
     have_multiple_function_sections = true;
 
@@ -21996,13 +22005,6 @@  dwarf2out_init (const char *filename ATT
 
   switch_to_section (text_section);
   ASM_OUTPUT_LABEL (asm_out_file, text_section_label);
-  if (flag_reorder_blocks_and_partition)
-    {
-      cold_text_section = unlikely_text_section ();
-      switch_to_section (cold_text_section);
-      ASM_OUTPUT_LABEL (asm_out_file, cold_text_section_label);
-    }
-
 }
 
 /* Called before cgraph_optimize starts outputtting functions, variables
@@ -23108,9 +23110,9 @@  dwarf2out_finish (const char *filename)
   /* Output a terminator label for the .text section.  */
   switch_to_section (text_section);
   targetm.asm_out.internal_label (asm_out_file, TEXT_END_LABEL, 0);
-  if (flag_reorder_blocks_and_partition)
+  if (cold_text_section)
     {
-      switch_to_section (unlikely_text_section ());
+      switch_to_section (cold_text_section);
       targetm.asm_out.internal_label (asm_out_file, COLD_END_LABEL, 0);
     }