Message ID | 20101123235749.GA13018@kam.mff.cuni.cz |
---|---|
State | New |
Headers | show |
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); }
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~
> > 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~
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); }