|
|
Created:
12 years, 2 months ago by saugustine Modified:
12 years, 2 months ago Reviewers:
Cary CC:
gcc-patches_gcc.gnu.org Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/branches/google/gcc-4_7/ Visibility:
Public. |
Patch Set 1 #
MessagesTotal messages: 5
The enclosed patch for Google 4.7 is an optimization for debug strings under -gsplit-dwarf. Currently under -gsplit-dwarf, all strings with DW_FORM_strp end up in the .debug_str.dwo section, which requires any string not destined for the .dwo to use DW_FORM_string, disallowing any duplication removal. With this patch, gcc creates a normal .debug_str section even under -gsplit-dwarf, and puts any DW_FORM_strp string destined for the .o file into that section. Tested with full bootstrap and the gdb test suite. OK for Google 4.7? When stage 1 opens again, I expect I will port it there as well. Sterling Index: gcc/dwarf2out.c =================================================================== --- gcc/dwarf2out.c (revision 195673) +++ gcc/dwarf2out.c (working copy) @@ -166,6 +166,7 @@ static GTY(()) section *debug_pubnames_section; static GTY(()) section *debug_pubtypes_section; static GTY(()) section *debug_str_section; +static GTY(()) section *debug_str_dwo_section; static GTY(()) section *debug_str_offsets_section; static GTY(()) section *debug_ranges_section; static GTY(()) section *debug_frame_section; @@ -217,6 +218,28 @@ static GTY ((param_is (struct indirect_string_node))) htab_t debug_str_hash; +/* With split_debug_info, both the comp_dir and dwo_name go in the + main object file, rather than the dwo, similar to the force_direct + parameter elsewhere but with additional complications: + + 1) The string is needed in both the main object file and the dwo. + That is, the comp_dir and dwo_name will appear in both places. + + 2) Strings can use three forms: DW_FORM_string, DW_FORM_strp or + DW_FORM_GNU_str_index. + + 3) GCC chooses the form to use late, depending on the size and + reference count. + + Rather than forcing the all debug string handling functions and + callers to deal with these complications, simply use a separate, + special-cased string table for any attribute that should go in the + main object file. This limits the complexity to just the places + that need it. */ + +static GTY ((param_is (struct indirect_string_node))) + htab_t skeleton_debug_str_hash; + static GTY(()) int dw2_string_counter; /* True if the compilation unit places functions in more than one section. */ @@ -3593,6 +3616,8 @@ static void schedule_generic_params_dies_gen (tree t); static void gen_scheduled_generic_parms_dies (void); +static const char *comp_dir_string (void); + /* enum for tracking thread-local variables whose address is really an offset relative to the TLS pointer, which will need link-time relocation, but will not need relocation by the DWARF consumer. */ @@ -3710,11 +3735,11 @@ (!dwarf_split_debug_info \ ? (DEBUG_NORM_STR_OFFSETS_SECTION) : (DEBUG_DWO_STR_OFFSETS_SECTION)) #endif -#define DEBUG_DWO_STR_SECTION ".debug_str.dwo" -#define DEBUG_NORM_STR_SECTION ".debug_str" +#ifndef DEBUG_STR_DWO_SECTION +#define DEBUG_STR_DWO_SECTION ".debug_str.dwo" +#endif #ifndef DEBUG_STR_SECTION -#define DEBUG_STR_SECTION \ - (!dwarf_split_debug_info ? (DEBUG_NORM_STR_SECTION) : (DEBUG_DWO_STR_SECTION)) +#define DEBUG_STR_SECTION ".debug_str" #endif #ifndef DEBUG_RANGES_SECTION #define DEBUG_RANGES_SECTION ".debug_ranges" @@ -3726,17 +3751,18 @@ #endif /* Section flags for .debug_macinfo/.debug_macro section. */ -#define DEBUG_MACRO_SECTION_FLAGS \ +#define DEBUG_MACRO_SECTION_FLAGS \ (dwarf_split_debug_info ? SECTION_DEBUG | SECTION_EXCLUDE : SECTION_DEBUG) /* Section flags for .debug_str section. */ -#define DEBUG_STR_SECTION_FLAGS \ - (dwarf_split_debug_info \ - ? SECTION_DEBUG | SECTION_EXCLUDE \ - : (HAVE_GAS_SHF_MERGE && flag_merge_debug_strings \ - ? SECTION_DEBUG | SECTION_MERGE | SECTION_STRINGS | 1 \ - : SECTION_DEBUG)) +#define DEBUG_STR_SECTION_FLAGS \ + (HAVE_GAS_SHF_MERGE && flag_merge_debug_strings \ + ? SECTION_DEBUG | SECTION_MERGE | SECTION_STRINGS | 1 \ + : SECTION_DEBUG) +/* Section flags for .debug_str.dwo section. */ +#define DEBUG_STR_DWO_SECTION_FLAGS (SECTION_DEBUG | SECTION_EXCLUDE) + /* Labels we insert at beginning sections we can reference instead of the section names themselves. */ @@ -4658,19 +4684,15 @@ (const char *)x2) == 0; } -/* Add STR to the indirect string hash table. */ +/* Add STR to the given string hash table. */ static struct indirect_string_node * -find_AT_string (const char *str) +find_AT_string_in_table (const char *str, htab_t table) { struct indirect_string_node *node; void **slot; - if (! debug_str_hash) - debug_str_hash = htab_create_ggc (10, debug_str_do_hash, - debug_str_eq, NULL); - - slot = htab_find_slot_with_hash (debug_str_hash, str, + slot = htab_find_slot_with_hash (table, str, htab_hash_string (str), INSERT); if (*slot == NULL) { @@ -4685,6 +4707,18 @@ return node; } +/* Add STR to the indirect string hash table. */ + +static struct indirect_string_node * +find_AT_string (const char *str) +{ + if (! debug_str_hash) + debug_str_hash = htab_create_ggc (10, debug_str_do_hash, + debug_str_eq, NULL); + + return find_AT_string_in_table (str, debug_str_hash); +} + /* Add a string attribute value to a DIE. */ static inline void @@ -9355,6 +9389,31 @@ } } +/* Add a string attribute value to a skeleton DIE. */ + +static inline void +add_skeleton_AT_string (dw_die_ref die, enum dwarf_attribute attr_kind, + const char *str) +{ + dw_attr_node attr; + struct indirect_string_node *node; + + if (! skeleton_debug_str_hash) + skeleton_debug_str_hash = htab_create_ggc (10, debug_str_do_hash, + debug_str_eq, NULL); + + node = find_AT_string_in_table (str, skeleton_debug_str_hash); + find_string_form (node); + if (node->form == DW_FORM_GNU_str_index) + node->form = DW_FORM_strp; + + attr.dw_attr = attr_kind; + attr.dw_attr_val.val_class = dw_val_class_str; + attr.dw_attr_val.val_entry = NULL; + attr.dw_attr_val.v.val_str = node; + add_dwarf_attr (die, &attr); +} + /* Helper function to generate top-level dies for skeleton debug_info and debug_types. */ @@ -9362,18 +9421,11 @@ add_top_level_skeleton_die_attrs (dw_die_ref die) { const char *dwo_file_name = concat (aux_base_name, ".dwo", NULL); - dw_attr_ref attr; + const char *comp_dir = comp_dir_string (); - add_comp_dir_attribute (die); - add_AT_string (die, DW_AT_GNU_dwo_name, dwo_file_name); - /* The specification suggests that these attributes be inline to avoid - having a .debug_str section. We know that they exist in the die because - we just added them. */ - attr = get_AT (die, DW_AT_GNU_dwo_name); - attr->dw_attr_val.v.val_str->form = DW_FORM_string; - attr = get_AT (die, DW_AT_comp_dir); - attr->dw_attr_val.v.val_str->form = DW_FORM_string; - + add_skeleton_AT_string (die, DW_AT_GNU_dwo_name, dwo_file_name); + if (comp_dir != NULL) + add_skeleton_AT_string (die, DW_AT_comp_dir, comp_dir); add_AT_pubnames (die); add_AT_lineptr (die, DW_AT_GNU_addr_base, debug_addr_section_label); } @@ -16496,16 +16548,16 @@ add_AT_die_ref (die, DW_AT_GNAT_descriptive_type, dtype_die); } -/* Generate a DW_AT_comp_dir attribute for DIE. */ +/* Retrieve the comp_dir string suitable for use with DW_AT_comp_dir. */ -static void -add_comp_dir_attribute (dw_die_ref die) +static const char * +comp_dir_string (void) { const char *wd = get_src_pwd (); char *wd1; if (wd == NULL) - return; + return NULL; if (DWARF2_DIR_SHOULD_END_WITH_SEPARATOR) { @@ -16518,8 +16570,17 @@ wd1 [wdlen + 1] = 0; wd = wd1; } + return remap_debug_filename (wd); +} - add_AT_string (die, DW_AT_comp_dir, remap_debug_filename (wd)); +/* Generate a DW_AT_comp_dir attribute for DIE. */ + +static void +add_comp_dir_attribute (dw_die_ref die) +{ + const char * wd = comp_dir_string (); + if (wd != NULL) + add_AT_string (die, DW_AT_comp_dir, wd); } /* Return the default for DW_AT_lower_bound, or -1 if there is not any @@ -22228,6 +22289,8 @@ DEBUG_SKELETON_INFO_SECTION_LABEL, 0); debug_loc_section = get_section (DEBUG_DWO_LOC_SECTION, SECTION_DEBUG | SECTION_EXCLUDE, NULL); + debug_str_dwo_section = get_section (DEBUG_STR_DWO_SECTION, + DEBUG_STR_DWO_SECTION_FLAGS, NULL); } debug_aranges_section = get_section (DEBUG_ARANGES_SECTION, SECTION_DEBUG, NULL); @@ -22356,7 +22419,6 @@ /* Assert that the strings are output in the same order as their indexes were assigned. */ gcc_assert (*cur_idx == node->index); - ASM_OUTPUT_LABEL (asm_out_file, node->label); assemble_string (node->str, strlen (node->str) + 1); *cur_idx += 1; } @@ -22371,6 +22433,7 @@ { struct indirect_string_node *node = (struct indirect_string_node *) *h; + node->form = find_string_form (node); if (node->form == DW_FORM_strp && node->refcount > 0) { ASM_OUTPUT_LABEL (asm_out_file, node->label); @@ -22385,21 +22448,21 @@ static void output_indirect_strings (void) { + switch_to_section (debug_str_section); if (!dwarf_split_debug_info) - { - switch_to_section (debug_str_section); - htab_traverse (debug_str_hash, output_indirect_string, NULL); - } + htab_traverse (debug_str_hash, output_indirect_string, NULL); else { unsigned int offset = 0; unsigned int cur_idx = 0; + htab_traverse (skeleton_debug_str_hash, output_indirect_string, NULL); + switch_to_section (debug_str_offsets_section); htab_traverse_noresize (debug_str_hash, output_index_string_offset, &offset); - switch_to_section (debug_str_section); + switch_to_section (debug_str_dwo_section); htab_traverse_noresize (debug_str_hash, output_index_string, &cur_idx); @@ -22810,6 +22873,8 @@ if (debug_str_hash) htab_empty (debug_str_hash); + if (skeleton_debug_str_hash) + htab_empty (skeleton_debug_str_hash); prune_unused_types_prune (comp_unit_die ()); for (node = limbo_die_list; node; node = node->next) prune_unused_types_prune (node->die); @@ -24081,6 +24146,7 @@ optimize_location_lists (comp_unit_die ()); save_macinfo_strings (); + if (dwarf_split_debug_info) { unsigned int index = 0; @@ -24230,7 +24296,7 @@ } /* If we emitted any indirect strings, output the string table too. */ - if (debug_str_hash) + if (debug_str_hash || skeleton_debug_str_hash) output_indirect_strings (); } -- This patch is available for review at http://codereview.appspot.com/7241067
Sign in to reply to this message.
> @@ -22385,21 +22448,21 @@ > static void > output_indirect_strings (void) > { > + switch_to_section (debug_str_section); > if (!dwarf_split_debug_info) > - { > - switch_to_section (debug_str_section); > - htab_traverse (debug_str_hash, output_indirect_string, NULL); > - } > + htab_traverse (debug_str_hash, output_indirect_string, NULL); > else > { > unsigned int offset = 0; > unsigned int cur_idx = 0; > > + htab_traverse (skeleton_debug_str_hash, output_indirect_string, NULL); > + > switch_to_section (debug_str_offsets_section); > htab_traverse_noresize (debug_str_hash, > output_index_string_offset, > &offset); > - switch_to_section (debug_str_section); > + switch_to_section (debug_str_dwo_section); > htab_traverse_noresize (debug_str_hash, > output_index_string, > &cur_idx); Doesn't this routine now need to check to see if debug_str_hash and skeleton_debug_str_hash are non-NULL individually? It gets called if either is non-NULL, but what will happen if only one is NULL? -cary
Sign in to reply to this message.
On Mon, Feb 4, 2013 at 11:56 AM, Cary Coutant <ccoutant@google.com> wrote: >> @@ -22385,21 +22448,21 @@ >> static void >> output_indirect_strings (void) >> { >> + switch_to_section (debug_str_section); >> if (!dwarf_split_debug_info) >> - { >> - switch_to_section (debug_str_section); >> - htab_traverse (debug_str_hash, output_indirect_string, NULL); >> - } >> + htab_traverse (debug_str_hash, output_indirect_string, NULL); >> else >> { >> unsigned int offset = 0; >> unsigned int cur_idx = 0; >> >> + htab_traverse (skeleton_debug_str_hash, output_indirect_string, NULL); >> + >> switch_to_section (debug_str_offsets_section); >> htab_traverse_noresize (debug_str_hash, >> output_index_string_offset, >> &offset); >> - switch_to_section (debug_str_section); >> + switch_to_section (debug_str_dwo_section); >> htab_traverse_noresize (debug_str_hash, >> output_index_string, >> &cur_idx); > > Doesn't this routine now need to check to see if debug_str_hash and > skeleton_debug_str_hash are non-NULL individually? It gets called if > either is non-NULL, but what will happen if only one is NULL? Ahh, good catch. Fixed as attached. Sterling
Sign in to reply to this message.
> Ahh, good catch. Fixed as attached. Looks good, thanks. OK for the google/gcc-4_7 branch. (And, yes, please do port this to trunk when Stage 1 reopens.) -cary
Sign in to reply to this message.
On Mon, Feb 4, 2013 at 1:45 PM, Cary Coutant <ccoutant@google.com> wrote: >> Ahh, good catch. Fixed as attached. > > Looks good, thanks. OK for the google/gcc-4_7 branch. > > (And, yes, please do port this to trunk when Stage 1 reopens.) Thanks. Committed.
Sign in to reply to this message.
|