| Message ID | CABtf2+QZP3Y7f+x075fKDxjaX7PGiAirV87ajML2nJpfnoz5BA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 12/05/14 15:41, Caroline Tice wrote: > When hot/cold function splitting occurs, a symbol is generated for the > cold partition, and gets output in the assembly & debug info, but the > symbol currently gets a size of 0 and a type of NOTYPE, as in this > example (on x86_64-linux) from the cold_partition_label test in the > testsuite: > > $ readelf -sW cold_partition_label.x02 | grep foo > 36: 0000000000400450 0 NOTYPE LOCAL DEFAULT 12 foo.cold.0 > 58: 0000000000400490 43 FUNC GLOBAL DEFAULT 12 foo > $ > > This patch fixes this by calculating the right size for the partition, > and outputing the size and type fo the cold partition symbol. After > applying this patch and looking at the same test, I get: > > $ readelf -sW cold_partition_label.x02 | grep foo > 36: 0000000000400450 29 FUNC LOCAL DEFAULT 12 foo.cold.0 > 58: 0000000000400490 43 FUNC GLOBAL DEFAULT 12 foo > $ > > This patch has been tested by bootstrapping the compiler, running the > dejagnu testsuite with no regressions, and checked as shown above that > it fixes the original problem. Is this patch OK to commit to ToT? > > -- Caroline Tice > cmtice@google.com > > 2014-12-05 Caroline Tice <cmtice@google.com> > > * final.c (final_scan_insn): Change 'cold_function_name' to > 'cold_partition_name' and make it a global variable; also output > assembly to give it a 'FUNC' type, if appropriate. > * varasm.c (cold_partition_name): Declare and initialize global > variable. > (assemble_start_function): Re-set value for cold_partition_name. > (assemble_end_function): Output assembly to calculate size of cold > partition, and associate size with name, if appropriate. > * varash.h (cold_partition_name): Add extern declaration for global > variable. OK for the trunk. I'm ever-so-slightly worried about the PA and PTX ports are they're probably the most sensitive to types. But we'll deal with them if they complain. Adding a testcase would be helpful. I believe some of the LTO tests use readelf, so there should be some infrastructure you can factor out and reuse. jeff
Index: gcc/final.c =================================================================== --- gcc/final.c (revision 218434) +++ gcc/final.c (working copy) @@ -2223,10 +2223,16 @@ suffixing "cold" to the original function's name. */ if (in_cold_section_p) { - tree cold_function_name + cold_partition_name = clone_function_name (current_function_decl, "cold"); +#ifdef ASM_DECLARE_FUNCTION_NAME + ASM_DECLARE_FUNCTION_NAME (asm_out_file, + IDENTIFIER_POINTER (cold_partition_name), + current_function_decl); +#else ASM_OUTPUT_LABEL (asm_out_file, - IDENTIFIER_POINTER (cold_function_name)); + IDENTIFIER_POINTER (cold_partition_name)); +#endif } break; Index: gcc/varasm.c =================================================================== --- gcc/varasm.c (revision 218434) +++ gcc/varasm.c (working copy) @@ -171,6 +171,13 @@ at the cold section. */ bool in_cold_section_p; +/* The following global holds the partition name for the code in the + cold section of a function, if hot/cold function splitting is enabled + and there was actually code that went into the cold section. A + pseudo function name is needed for the cold section of code for some + debugging tools that perform symbolization. */ +tree cold_partition_name = NULL_TREE; + /* A linked list of all the unnamed sections. */ static GTY(()) section *unnamed_sections; @@ -1708,6 +1715,7 @@ ASM_GENERATE_INTERNAL_LABEL (tmp_label, "LCOLDE", const_labelno); crtl->subsections.cold_section_end_label = ggc_strdup (tmp_label); const_labelno++; + cold_partition_name = NULL_TREE; } else { @@ -1843,6 +1851,10 @@ save_text_section = in_section; switch_to_section (unlikely_text_section ()); + if (cold_partition_name != NULL_TREE) + ASM_DECLARE_FUNCTION_SIZE (asm_out_file, + IDENTIFIER_POINTER (cold_partition_name), + decl); ASM_OUTPUT_LABEL (asm_out_file, crtl->subsections.cold_section_end_label); if (first_function_block_is_cold) switch_to_section (text_section); Index: gcc/varasm.h =================================================================== --- gcc/varasm.h (revision 218434) +++ gcc/varasm.h (working copy) @@ -20,6 +20,13 @@ #ifndef GCC_VARASM_H #define GCC_VARASM_H +/* The following global holds the partition name for the code in the + cold section of a function, if hot/cold function splitting is enabled + and there was actually code that went into the cold section. A + pseudo function name is needed for the cold section of code for some + debugging tools that perform symbolization. */ +extern tree cold_partition_name; + extern tree tree_output_constant_def (tree); extern void make_decl_rtl (tree); extern rtx make_decl_rtl_for_debug (tree);
When hot/cold function splitting occurs, a symbol is generated for the cold partition, and gets output in the assembly & debug info, but the symbol currently gets a size of 0 and a type of NOTYPE, as in this example (on x86_64-linux) from the cold_partition_label test in the testsuite: $ readelf -sW cold_partition_label.x02 | grep foo 36: 0000000000400450 0 NOTYPE LOCAL DEFAULT 12 foo.cold.0 58: 0000000000400490 43 FUNC GLOBAL DEFAULT 12 foo $ This patch fixes this by calculating the right size for the partition, and outputing the size and type fo the cold partition symbol. After applying this patch and looking at the same test, I get: $ readelf -sW cold_partition_label.x02 | grep foo 36: 0000000000400450 29 FUNC LOCAL DEFAULT 12 foo.cold.0 58: 0000000000400490 43 FUNC GLOBAL DEFAULT 12 foo $ This patch has been tested by bootstrapping the compiler, running the dejagnu testsuite with no regressions, and checked as shown above that it fixes the original problem. Is this patch OK to commit to ToT? -- Caroline Tice cmtice@google.com 2014-12-05 Caroline Tice <cmtice@google.com> * final.c (final_scan_insn): Change 'cold_function_name' to 'cold_partition_name' and make it a global variable; also output assembly to give it a 'FUNC' type, if appropriate. * varasm.c (cold_partition_name): Declare and initialize global variable. (assemble_start_function): Re-set value for cold_partition_name. (assemble_end_function): Output assembly to calculate size of cold partition, and associate size with name, if appropriate. * varash.h (cold_partition_name): Add extern declaration for global variable.