|
|
Patch Set 1 #Patch Set 2 : [google] Modification of gcov pmu format to reduce gcda size bloat #
Total comments: 12
Patch Set 3 : [google] Modification of gcov pmu format to reduce gcda size bloat #
Total comments: 6
Patch Set 4 : [google] Modification of gcov pmu format to reduce gcda size bloat #
MessagesTotal messages: 21
This patch modifies pmu-profile to allow gooda_feedback access to important gcov to gcda conversion functions and also modifies the gcov pmu format store indices in a string table of filenames as opposed to storing the filename inside of every pmu entry. This reduces the amount of size bloat in gcda, especially since the gcda files do a per asm line analysis and this leads to many entries sharing the same filename data anyways. gooda_feedback considers this and outputs the string following the load latency and branch misprediction information. The changes made belong to one of the following categories: 1. Removing static declaration/defition of certain function that are needed by gooda_feedback (pmu-profile.c) 2. Modifying the gcov format to use indices instead of string for the filename (gcov-io.h) 3. Modifying gcda writing functions to write the index instead of the filename (pmu-profile.c) 4. Removing references to filename (gcov-io.c, pmu-profile.c) This was tested by using crosstool-validate with --testers=crosstool. No other testcases seemed necessary since this patch only modifies data that is relevant to pfmon, which is considered dead now. The patch should be applied to google/gcc-4_7 The CL for gooda_feedback can be found at Google ref c/31972005 2012-07-24 Chris Manghane <cmang@google.com> * libgcc/pmu-profile.c (static int parse_pfmon_load_latency): filename field no longer exists (parse_load_latency_line): filename field no longer exists (parse_branch_mispredict_line): filename field no longer exists (__gcov_stop_pmu_profiler): filename field no longer exists (gcov_write_ll_line): now writes string table index instead of filename (gcov_write_branch_mispredict_line): now writes string table index instead of filename (gcov_write_load_latency_infos): filename field no longer exists (gcov_write_branch_mispredict_infos): filename field no longer exists (gcov_tag_pmu_tool_header_length): filename field no longer exists (gcov_write_tool_header): filename field no longer exists * gcc/gcov.c (release_structures): filename field no longer exists (filter_pmu_data_lines): filename field no longer exists * gcc/gcov-io.c (gcov_read_pmu_load_latency_info): removed filename field and added string table index (gcov_read_pmu_branch_mispredict_info): removed filename field and added string table index (print_load_latency_line): filename field no longer exists (print_branch_mispredict_line): filename field no longer exists * gcc/gcov-io.h: added new tag for string table printing * gcc/gcov-dump.c (tag_pmu_load_latency_info): filename field no longer exists (tag_pmu_branch_mispredict_info): filename field no longer exists Index: libgcc/pmu-profile.c =================================================================== --- libgcc/pmu-profile.c (revision 189823) +++ libgcc/pmu-profile.c (working copy) @@ -232,11 +232,11 @@ static int parse_pfmon_load_latency (char *filenam static int parse_pfmon_branch_mispredicts (char *filename, void *pmu_data); static gcov_unsigned_t gcov_tag_pmu_tool_header_length (gcov_pmu_tool_header_t *header); -static void gcov_write_tool_header (gcov_pmu_tool_header_t *header); -static void gcov_write_load_latency_infos (void *info); -static void gcov_write_branch_mispredict_infos (void *info); -static void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); -static void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t +void gcov_write_tool_header (gcov_pmu_tool_header_t *header); +void gcov_write_load_latency_infos (void *info); +void gcov_write_branch_mispredict_infos (void *info); +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info); static int start_addr2line_symbolizer (pid_t pid); static void end_addr2line_symbolizer (void); @@ -749,14 +749,12 @@ parse_load_latency_line (char *line, gcov_pmu_ll_i if (!sep) { /* Assume entire string is srcfile. */ - ll_info->filename = (char *)sym_info; ll_info->line = 0; } else { /* Terminate the filename string at the separator. */ *sep = 0; - ll_info->filename = (char *)sym_info; /* Convert rest of the sym info to a line number. */ ll_info->line = atol (sep+1); } @@ -765,7 +763,6 @@ parse_load_latency_line (char *line, gcov_pmu_ll_i else { /* No symbolizer available. */ - ll_info->filename = NULL; ll_info->line = 0; ll_info->discriminator = 0; } @@ -815,14 +812,12 @@ parse_branch_mispredict_line (char *line, gcov_pmu if (!sep) { /* Assume entire string is srcfile. */ - brm_info->filename = sym_info; brm_info->line = 0; } else { /* Terminate the filename string at the separator. */ *sep = 0; - brm_info->filename = sym_info; /* Convert rest of the sym info to a line number. */ brm_info->line = atol (sep+1); } @@ -831,7 +826,6 @@ parse_branch_mispredict_line (char *line, gcov_pmu else { /* No symbolizer available. */ - brm_info->filename = NULL; brm_info->line = 0; brm_info->discriminator = 0; } @@ -1361,11 +1355,10 @@ __gcov_stop_pmu_profiler (void) /* Write the load latency information LL_INFO into the gcda file. */ -static void +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_LOAD_LATENCY_LENGTH (ll_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, 1); gcov_write_unsigned (ll_info->counts); gcov_write_unsigned (ll_info->self); gcov_write_unsigned (ll_info->cum); @@ -1379,31 +1372,29 @@ gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_i gcov_write_counter (ll_info->code_addr); gcov_write_unsigned (ll_info->line); gcov_write_unsigned (ll_info->discriminator); - gcov_write_string (ll_info->filename); + gcov_write_unsigned (ll_info->filetag); } /* Write the branch mispredict information BRM_INFO into the gcda file. */ -static void +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH ( - brm_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, 1); gcov_write_unsigned (brm_info->counts); gcov_write_unsigned (brm_info->self); gcov_write_unsigned (brm_info->cum); gcov_write_counter (brm_info->code_addr); gcov_write_unsigned (brm_info->line); gcov_write_unsigned (brm_info->discriminator); - gcov_write_string (brm_info->filename); + gcov_write_unsigned (brm_info->filetag); } /* Write load latency information INFO into the gcda file. The gcda file has already been opened and is available for writing. */ -static void +void gcov_write_load_latency_infos (void *info) { unsigned i; @@ -1429,7 +1420,7 @@ gcov_write_load_latency_infos (void *info) /* Write branch mispredict information INFO into the gcda file. The gcda file has already been opened and is available for writing. */ -static void +void gcov_write_branch_mispredict_infos (void *info) { unsigned i; @@ -1470,7 +1461,7 @@ gcov_tag_pmu_tool_header_length (gcov_pmu_tool_hea /* Write tool header into the gcda file. It assumes that the gcda file has already been opened and is available for writing. */ -static void +void gcov_write_tool_header (gcov_pmu_tool_header_t *header) { gcov_unsigned_t len = gcov_tag_pmu_tool_header_length (header); Index: gcc/gcov.c =================================================================== --- gcc/gcov.c (revision 189823) +++ gcc/gcov.c (working copy) @@ -1008,8 +1008,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < ll_infos->ll_count; ++i) { - if (ll_infos->ll_array[i]->filename) - XDELETE (ll_infos->ll_array[i]->filename); XDELETE (ll_infos->ll_array[i]); } /* delete the array itself */ @@ -1026,8 +1024,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < brm_infos->brm_count; ++i) { - if (brm_infos->brm_array[i]->filename) - XDELETE (brm_infos->brm_array[i]->filename); XDELETE (brm_infos->brm_array[i]); } /* delete the array itself */ @@ -2277,58 +2273,6 @@ filter_pmu_data_lines (source_t *src) ll_infos->ll_array = 0; brm_infos->brm_array = 0; - /* Go over all the load latency entries and save the ones - corresponding to this source file. */ - for (i = 0; i < pmu_global_info.ll_infos.ll_count; ++i) - { - gcov_pmu_ll_info_t *ll_info = pmu_global_info.ll_infos.ll_array[i]; - if (0 == strcmp (src->name, ll_info->filename)) - { - if (!ll_infos->ll_array) - { - ll_infos->ll_count = 0; - ll_infos->alloc_ll_count = 64; - ll_infos->ll_array = XCNEWVEC (gcov_pmu_ll_info_t *, - ll_infos->alloc_ll_count); - } - /* Found a matching entry, save it. */ - ll_infos->ll_count++; - if (ll_infos->ll_count >= ll_infos->alloc_ll_count) - { - /* need to realloc */ - ll_infos->ll_array = (gcov_pmu_ll_info_t **) - xrealloc (ll_infos->ll_array, 2 * ll_infos->alloc_ll_count); - } - ll_infos->ll_array[ll_infos->ll_count - 1] = ll_info; - } - } - - /* Go over all the branch mispredict entries and save the ones - corresponding to this source file. */ - for (i = 0; i < pmu_global_info.brm_infos.brm_count; ++i) - { - gcov_pmu_brm_info_t *brm_info = pmu_global_info.brm_infos.brm_array[i]; - if (0 == strcmp (src->name, brm_info->filename)) - { - if (!brm_infos->brm_array) - { - brm_infos->brm_count = 0; - brm_infos->alloc_brm_count = 64; - brm_infos->brm_array = XCNEWVEC (gcov_pmu_brm_info_t *, - brm_infos->alloc_brm_count); - } - /* Found a matching entry, save it. */ - brm_infos->brm_count++; - if (brm_infos->brm_count >= brm_infos->alloc_brm_count) - { - /* need to realloc */ - brm_infos->brm_array = (gcov_pmu_brm_info_t **) - xrealloc (brm_infos->brm_array, 2 * brm_infos->alloc_brm_count); - } - brm_infos->brm_array[brm_infos->brm_count - 1] = brm_info; - } - } - /* Sort the load latency data according to the line numbers because we later iterate over sources in line number order. Normally we expect the PMU tool to provide sorted data, but a few entries can Index: gcc/gcov-io.c =================================================================== --- gcc/gcov-io.c (revision 189823) +++ gcc/gcov-io.c (working copy) @@ -242,7 +242,6 @@ GCOV_LINKAGE void gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_t *ll_info, gcov_unsigned_t len ATTRIBUTE_UNUSED) { - const char *filename; ll_info->counts = gcov_read_unsigned (); ll_info->self = gcov_read_unsigned (); ll_info->cum = gcov_read_unsigned (); @@ -256,11 +255,7 @@ gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_ ll_info->code_addr = gcov_read_counter (); ll_info->line = gcov_read_unsigned (); ll_info->discriminator = gcov_read_unsigned (); - filename = gcov_read_string (); - if (filename) - ll_info->filename = gcov_canonical_filename (xstrdup (filename)); - else - ll_info->filename = 0; + ll_info->filetag = gcov_read_unsigned (); } /* Read LEN words and construct branch mispredict info BRM_INFO. */ @@ -269,18 +264,13 @@ GCOV_LINKAGE void gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, gcov_unsigned_t len ATTRIBUTE_UNUSED) { - const char *filename; brm_info->counts = gcov_read_unsigned (); brm_info->self = gcov_read_unsigned (); brm_info->cum = gcov_read_unsigned (); brm_info->code_addr = gcov_read_counter (); brm_info->line = gcov_read_unsigned (); brm_info->discriminator = gcov_read_unsigned (); - filename = gcov_read_string (); - if (filename) - brm_info->filename = gcov_canonical_filename (xstrdup (filename)); - else - brm_info->filename = 0; + brm_info->filetag = gcov_read_unsigned (); } /* Read LEN words from an open gcov file and construct data into pmu @@ -779,7 +769,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ if (!ll_info) return; fprintf (fp, " %u %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% " - "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", + "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", ll_info->counts, convert_unsigned_to_pct (ll_info->self), convert_unsigned_to_pct (ll_info->cum), @@ -791,7 +781,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ convert_unsigned_to_pct (ll_info->gt_1024), convert_unsigned_to_pct (ll_info->wself), ll_info->code_addr, - ll_info->filename, + ll_info->filetag, ll_info->line, ll_info->discriminator); if (newline == add_newline) @@ -807,12 +797,12 @@ print_branch_mispredict_line (FILE *fp, const gcov { if (!brm_info) return; - fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", + fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", brm_info->counts, convert_unsigned_to_pct (brm_info->self), convert_unsigned_to_pct (brm_info->cum), brm_info->code_addr, - brm_info->filename, + brm_info->filetag, brm_info->line, brm_info->discriminator); if (newline == add_newline) Index: gcc/gcov-io.h =================================================================== --- gcc/gcov-io.h (revision 189823) +++ gcc/gcov-io.h (working copy) @@ -449,6 +449,7 @@ typedef HOST_WIDEST_INT gcov_type; (gcov_string_length (filename) + 5 + 2) #define GCOV_TAG_PMU_TOOL_HEADER ((gcov_unsigned_t)0xa9000000) #define GCOV_TAG_MODULE_INFO ((gcov_unsigned_t)0xab000000) +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY ((gcov_unsigned_t)0xad000000) /* Counters that are collected. */ #define GCOV_COUNTER_ARCS 0 /* Arc transitions. */ @@ -635,7 +636,7 @@ typedef struct gcov_pmu_load_latency_info gcov_type code_addr; /* the actual miss address (pc+1 for Intel) */ gcov_unsigned_t line; /* line number corresponding to this miss */ gcov_unsigned_t discriminator; /* discriminator information for this miss */ - char *filename; /* filename corresponding to this miss */ + gcov_unsigned_t filetag; /* location in string table of filename corresponding to event */ } gcov_pmu_ll_info_t; /* This structure is used during runtime as well as in gcov. */ @@ -665,7 +666,7 @@ typedef struct gcov_pmu_branch_mispredict_info gcov_type code_addr; /* the actual mispredict address */ gcov_unsigned_t line; /* line number corresponding to this event */ gcov_unsigned_t discriminator; /* discriminator for this event */ - char *filename; /* filename corresponding to this event */ + gcov_unsigned_t filetag; /* location in string table of filename corresponding to event */ } gcov_pmu_brm_info_t; /* This structure is used during runtime as well as in gcov. */ Index: gcc/gcov-dump.c =================================================================== --- gcc/gcov-dump.c (revision 189823) +++ gcc/gcov-dump.c (working copy) @@ -573,7 +573,6 @@ tag_pmu_load_latency_info (const char *filename AT gcov_pmu_ll_info_t ll_info; gcov_read_pmu_load_latency_info (&ll_info, length); print_load_latency_line (stdout, &ll_info, no_newline); - free (ll_info.filename); } /* Read gcov tag GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO from the gcda @@ -586,7 +585,6 @@ tag_pmu_branch_mispredict_info (const char *filena gcov_pmu_brm_info_t brm_info; gcov_read_pmu_branch_mispredict_info (&brm_info, length); print_branch_mispredict_line (stdout, &brm_info, no_newline); - free (brm_info.filename); } -- This patch is available for review at http://codereview.appspot.com/6427063
Sign in to reply to this message.
On Tue, Jul 24, 2012 at 3:03 PM, Chris Manghane <cmang@google.com> wrote: > This patch modifies pmu-profile to allow gooda_feedback access to > Needs some more context about perf.data -> gcda conversion. > important gcov to gcda conversion functions and also Not really gcov to gcda conversion functions, but gcov io functions. modifies the gcov pmu format store indices "format to store indices into a..." > in a string table of filenames as opposed to storing the filename inside > of every pmu entry. This reduces the amount of size bloat in gcda, > especially since the gcda files do a per asm line analysis and this leads > to many entries sharing the same filename data anyways. gooda_feedback > considers this and outputs the string following the load latency and branch > misprediction information. > > The changes made belong to one of the following categories: > 1. Removing static declaration/defition of certain function that are > needed by gooda_feedback (pmu-profile.c) > 2. Modifying the gcov format to use indices instead of string for the > filename (gcov-io.h) > 3. Modifying gcda writing functions to write the index instead of the > filename (pmu-profile.c) > 4. Removing references to filename (gcov-io.c, pmu-profile.c) > > This was tested by using crosstool-validate with --testers=crosstool. No > other testcases seemed necessary since this patch only modifies data that > is relevant to pfmon, which is considered dead now. > Needs more testing (discussed with Chris offline). > > The patch should be applied to google/gcc-4_7 > > The CL for gooda_feedback can be found at Google ref c/31972005 > > > 2012-07-24 Chris Manghane <cmang@google.com> > > * libgcc/pmu-profile.c (static int parse_pfmon_load_latency): > filename field no longer exists > Also mention the removal of "static" from the declarations. > (parse_load_latency_line): filename field no longer exists > (parse_branch_mispredict_line): filename field no longer exists > For repeating descriptions use or "Ditto." or "Likewise." (__gcov_stop_pmu_profiler): filename field no longer exists > (gcov_write_ll_line): now writes string table index instead of > filename > (gcov_write_branch_mispredict_line): now writes string table index > instead of filename > (gcov_write_load_latency_infos): filename field no longer exists > (gcov_write_branch_mispredict_infos): filename field no longer > exists > (gcov_tag_pmu_tool_header_length): filename field no longer exists > (gcov_write_tool_header): filename field no longer exists > * gcc/gcov.c (release_structures): filename field no longer exists > (filter_pmu_data_lines): filename field no longer exists > * gcc/gcov-io.c (gcov_read_pmu_load_latency_info): removed > filename field and added string table index > (gcov_read_pmu_branch_mispredict_info): removed filename field and > added string table index > (print_load_latency_line): filename field no longer exists > (print_branch_mispredict_line): filename field no longer exists > * gcc/gcov-io.h: added new tag for string table printing > * gcc/gcov-dump.c (tag_pmu_load_latency_info): filename field no > longer exists > (tag_pmu_branch_mispredict_info): filename field no longer exists > > Index: libgcc/pmu-profile.c > =================================================================== > --- libgcc/pmu-profile.c (revision 189823) > +++ libgcc/pmu-profile.c (working copy) > @@ -232,11 +232,11 @@ static int parse_pfmon_load_latency (char *filenam > static int parse_pfmon_branch_mispredicts (char *filename, void > *pmu_data); > static gcov_unsigned_t gcov_tag_pmu_tool_header_length > (gcov_pmu_tool_header_t > *header); > -static void gcov_write_tool_header (gcov_pmu_tool_header_t *header); > -static void gcov_write_load_latency_infos (void *info); > -static void gcov_write_branch_mispredict_infos (void *info); > -static void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); > -static void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t > +void gcov_write_tool_header (gcov_pmu_tool_header_t *header); > +void gcov_write_load_latency_infos (void *info); > +void gcov_write_branch_mispredict_infos (void *info); > +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); > +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t > *brm_info); > They should be moved to gcov-io.h with linkage GCOV_LINKAGE which will pick extern when building libgcov (see examples in gcov-io.h). > static int start_addr2line_symbolizer (pid_t pid); > static void end_addr2line_symbolizer (void); > @@ -749,14 +749,12 @@ parse_load_latency_line (char *line, gcov_pmu_ll_i > I think it would be cleaner just to remove all the pfmon specific handling as part of this checkin, since that is obsolete and wouldn't work with your changes anyway. > if (!sep) > { > /* Assume entire string is srcfile. */ > - ll_info->filename = (char *)sym_info; > ll_info->line = 0; > } > else > { > /* Terminate the filename string at the separator. */ > *sep = 0; > - ll_info->filename = (char *)sym_info; > /* Convert rest of the sym info to a line number. */ > ll_info->line = atol (sep+1); > } > @@ -765,7 +763,6 @@ parse_load_latency_line (char *line, gcov_pmu_ll_i > else > { > /* No symbolizer available. */ > - ll_info->filename = NULL; > ll_info->line = 0; > ll_info->discriminator = 0; > } > @@ -815,14 +812,12 @@ parse_branch_mispredict_line (char *line, gcov_pmu > if (!sep) > { > /* Assume entire string is srcfile. */ > - brm_info->filename = sym_info; > brm_info->line = 0; > } > else > { > /* Terminate the filename string at the separator. */ > *sep = 0; > - brm_info->filename = sym_info; > /* Convert rest of the sym info to a line number. */ > brm_info->line = atol (sep+1); > } > @@ -831,7 +826,6 @@ parse_branch_mispredict_line (char *line, gcov_pmu > else > { > /* No symbolizer available. */ > - brm_info->filename = NULL; > brm_info->line = 0; > brm_info->discriminator = 0; > } > @@ -1361,11 +1355,10 @@ __gcov_stop_pmu_profiler (void) > > /* Write the load latency information LL_INFO into the gcda file. */ > > -static void > +void > gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info) > { > - gcov_unsigned_t len = GCOV_TAG_PMU_LOAD_LATENCY_LENGTH > (ll_info->filename); > - gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, len); > + gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, 1); > The length of 1 isn't correct - it should be the length of the data associated with this tag. See GCOV_TAG_PMU_LOAD_LATENCY_LENGTH for how it is computing this (and you can just modify that to remove the filename parameter and add 1 for your new tag). gcov_write_unsigned (ll_info->counts); > gcov_write_unsigned (ll_info->self); > gcov_write_unsigned (ll_info->cum); > @@ -1379,31 +1372,29 @@ gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_i > gcov_write_counter (ll_info->code_addr); > gcov_write_unsigned (ll_info->line); > gcov_write_unsigned (ll_info->discriminator); > - gcov_write_string (ll_info->filename); > + gcov_write_unsigned (ll_info->filetag); > } > > > /* Write the branch mispredict information BRM_INFO into the gcda file. > */ > > -static void > +void > gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info) > { > - gcov_unsigned_t len = GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH ( > - brm_info->filename); > - gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, len); > + gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, 1); > Same thing here with the length computation (see above comment). > gcov_write_unsigned (brm_info->counts); > gcov_write_unsigned (brm_info->self); > gcov_write_unsigned (brm_info->cum); > gcov_write_counter (brm_info->code_addr); > gcov_write_unsigned (brm_info->line); > gcov_write_unsigned (brm_info->discriminator); > - gcov_write_string (brm_info->filename); > + gcov_write_unsigned (brm_info->filetag); > } > > /* Write load latency information INFO into the gcda file. The gcda > file has already been opened and is available for writing. */ > > -static void > +void > gcov_write_load_latency_infos (void *info) > { > unsigned i; > @@ -1429,7 +1420,7 @@ gcov_write_load_latency_infos (void *info) > /* Write branch mispredict information INFO into the gcda file. The > gcda file has already been opened and is available for writing. */ > > -static void > +void > gcov_write_branch_mispredict_infos (void *info) > { > unsigned i; > @@ -1470,7 +1461,7 @@ gcov_tag_pmu_tool_header_length (gcov_pmu_tool_hea > /* Write tool header into the gcda file. It assumes that the gcda file > has already been opened and is available for writing. */ > > -static void > +void > gcov_write_tool_header (gcov_pmu_tool_header_t *header) > { > gcov_unsigned_t len = gcov_tag_pmu_tool_header_length (header); > Index: gcc/gcov.c > =================================================================== > --- gcc/gcov.c (revision 189823) > +++ gcc/gcov.c (working copy) > @@ -1008,8 +1008,6 @@ release_structures (void) > /* delete each element */ > for (i = 0; i < ll_infos->ll_count; ++i) > { > - if (ll_infos->ll_array[i]->filename) > - XDELETE (ll_infos->ll_array[i]->filename); > XDELETE (ll_infos->ll_array[i]); > } > /* delete the array itself */ > @@ -1026,8 +1024,6 @@ release_structures (void) > /* delete each element */ > for (i = 0; i < brm_infos->brm_count; ++i) > { > - if (brm_infos->brm_array[i]->filename) > - XDELETE (brm_infos->brm_array[i]->filename); > XDELETE (brm_infos->brm_array[i]); > } > /* delete the array itself */ > @@ -2277,58 +2273,6 @@ filter_pmu_data_lines (source_t *src) > ll_infos->ll_array = 0; > brm_infos->brm_array = 0; > > - /* Go over all the load latency entries and save the ones > - corresponding to this source file. */ > Don't want to remove this handling, as it is doing the job of deciding which pmu data matches the current file being compiled. You just need to change the handling of how to access the filename. > - for (i = 0; i < pmu_global_info.ll_infos.ll_count; ++i) > - { > - gcov_pmu_ll_info_t *ll_info = pmu_global_info.ll_infos.ll_array[i]; > - if (0 == strcmp (src->name, ll_info->filename)) > - { > - if (!ll_infos->ll_array) > - { > - ll_infos->ll_count = 0; > - ll_infos->alloc_ll_count = 64; > - ll_infos->ll_array = XCNEWVEC (gcov_pmu_ll_info_t *, > - ll_infos->alloc_ll_count); > - } > - /* Found a matching entry, save it. */ > - ll_infos->ll_count++; > - if (ll_infos->ll_count >= ll_infos->alloc_ll_count) > - { > - /* need to realloc */ > - ll_infos->ll_array = (gcov_pmu_ll_info_t **) > - xrealloc (ll_infos->ll_array, 2 * > ll_infos->alloc_ll_count); > - } > - ll_infos->ll_array[ll_infos->ll_count - 1] = ll_info; > - } > - } > - > - /* Go over all the branch mispredict entries and save the ones > - corresponding to this source file. */ > - for (i = 0; i < pmu_global_info.brm_infos.brm_count; ++i) > - { > - gcov_pmu_brm_info_t *brm_info = > pmu_global_info.brm_infos.brm_array[i]; > - if (0 == strcmp (src->name, brm_info->filename)) > - { > - if (!brm_infos->brm_array) > - { > - brm_infos->brm_count = 0; > - brm_infos->alloc_brm_count = 64; > - brm_infos->brm_array = XCNEWVEC (gcov_pmu_brm_info_t *, > - > brm_infos->alloc_brm_count); > - } > - /* Found a matching entry, save it. */ > - brm_infos->brm_count++; > - if (brm_infos->brm_count >= brm_infos->alloc_brm_count) > - { > - /* need to realloc */ > - brm_infos->brm_array = (gcov_pmu_brm_info_t **) > - xrealloc (brm_infos->brm_array, 2 * > brm_infos->alloc_brm_count); > - } > - brm_infos->brm_array[brm_infos->brm_count - 1] = brm_info; > - } > - } > - > /* Sort the load latency data according to the line numbers because > we later iterate over sources in line number order. Normally we > expect the PMU tool to provide sorted data, but a few entries can > Index: gcc/gcov-io.c > =================================================================== > --- gcc/gcov-io.c (revision 189823) > +++ gcc/gcov-io.c (working copy) > @@ -242,7 +242,6 @@ GCOV_LINKAGE void > gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_t *ll_info, > gcov_unsigned_t len ATTRIBUTE_UNUSED) > { > - const char *filename; > ll_info->counts = gcov_read_unsigned (); > ll_info->self = gcov_read_unsigned (); > ll_info->cum = gcov_read_unsigned (); > @@ -256,11 +255,7 @@ gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_ > ll_info->code_addr = gcov_read_counter (); > ll_info->line = gcov_read_unsigned (); > ll_info->discriminator = gcov_read_unsigned (); > - filename = gcov_read_string (); > - if (filename) > - ll_info->filename = gcov_canonical_filename (xstrdup (filename)); > - else > - ll_info->filename = 0; > + ll_info->filetag = gcov_read_unsigned (); > } > > /* Read LEN words and construct branch mispredict info BRM_INFO. */ > @@ -269,18 +264,13 @@ GCOV_LINKAGE void > gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, > gcov_unsigned_t len > ATTRIBUTE_UNUSED) > { > - const char *filename; > brm_info->counts = gcov_read_unsigned (); > brm_info->self = gcov_read_unsigned (); > brm_info->cum = gcov_read_unsigned (); > brm_info->code_addr = gcov_read_counter (); > brm_info->line = gcov_read_unsigned (); > brm_info->discriminator = gcov_read_unsigned (); > - filename = gcov_read_string (); > - if (filename) > - brm_info->filename = gcov_canonical_filename (xstrdup (filename)); > - else > - brm_info->filename = 0; > + brm_info->filetag = gcov_read_unsigned (); > } > > /* Read LEN words from an open gcov file and construct data into pmu > @@ -779,7 +769,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ > if (!ll_info) > return; > fprintf (fp, " %u %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% " > - "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", > + "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", > Instead of changing this to print the tag, it would be useful to print both the tag and the filename (as accessed via your string table). Ditto below for the branch mispredict info. > ll_info->counts, > convert_unsigned_to_pct (ll_info->self), > convert_unsigned_to_pct (ll_info->cum), > @@ -791,7 +781,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ > convert_unsigned_to_pct (ll_info->gt_1024), > convert_unsigned_to_pct (ll_info->wself), > ll_info->code_addr, > - ll_info->filename, > + ll_info->filetag, > ll_info->line, > ll_info->discriminator); > if (newline == add_newline) > @@ -807,12 +797,12 @@ print_branch_mispredict_line (FILE *fp, const gcov > { > if (!brm_info) > return; > - fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", > + fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", > brm_info->counts, > convert_unsigned_to_pct (brm_info->self), > convert_unsigned_to_pct (brm_info->cum), > brm_info->code_addr, > - brm_info->filename, > + brm_info->filetag, > brm_info->line, > brm_info->discriminator); > if (newline == add_newline) > Need to add i/o handling of new string table. Index: gcc/gcov-io.h > =================================================================== > --- gcc/gcov-io.h (revision 189823) > +++ gcc/gcov-io.h (working copy) > @@ -449,6 +449,7 @@ typedef HOST_WIDEST_INT gcov_type; > (gcov_string_length (filename) + 5 + 2) > #define GCOV_TAG_PMU_TOOL_HEADER ((gcov_unsigned_t)0xa9000000) > #define GCOV_TAG_MODULE_INFO ((gcov_unsigned_t)0xab000000) > +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY ((gcov_unsigned_t)0xad000000) > > /* Counters that are collected. */ > #define GCOV_COUNTER_ARCS 0 /* Arc transitions. */ > @@ -635,7 +636,7 @@ typedef struct gcov_pmu_load_latency_info > gcov_type code_addr; /* the actual miss address (pc+1 for Intel) > */ > gcov_unsigned_t line; /* line number corresponding to this miss */ > gcov_unsigned_t discriminator; /* discriminator information for this > miss */ > - char *filename; /* filename corresponding to this miss */ > + gcov_unsigned_t filetag; /* location in string table of filename > corresponding to event */ > } gcov_pmu_ll_info_t; > > /* This structure is used during runtime as well as in gcov. */ > @@ -665,7 +666,7 @@ typedef struct gcov_pmu_branch_mispredict_info > gcov_type code_addr; /* the actual mispredict address */ > gcov_unsigned_t line; /* line number corresponding to this event > */ > gcov_unsigned_t discriminator; /* discriminator for this event */ > - char *filename; /* filename corresponding to this event */ > + gcov_unsigned_t filetag; /* location in string table of filename > corresponding to event */ > } gcov_pmu_brm_info_t; > > /* This structure is used during runtime as well as in gcov. */ > Index: gcc/gcov-dump.c > =================================================================== > --- gcc/gcov-dump.c (revision 189823) > +++ gcc/gcov-dump.c (working copy) > @@ -573,7 +573,6 @@ tag_pmu_load_latency_info (const char *filename AT gcov_pmu_ll_info_t ll_info; > gcov_read_pmu_load_latency_info (&ll_info, length); > print_load_latency_line (stdout, &ll_info, no_newline); > - free (ll_info.filename); > } > > /* Read gcov tag GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO from the gcda > @@ -586,7 +585,6 @@ tag_pmu_branch_mispredict_info (const char *filena > gcov_pmu_brm_info_t brm_info; > gcov_read_pmu_branch_mispredict_info (&brm_info, length); > print_branch_mispredict_line (stdout, &brm_info, no_newline); > - free (brm_info.filename); > } > Need to add printing of new string table. Teresa > > > > -- > This patch is available for review at > http://codereview.appspot.com/6427063 > -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Sign in to reply to this message.
Resending in plain text mode so that it goes through to mailing list... On Wed, Jul 25, 2012 at 11:32 AM, Teresa Johnson <tejohnson@google.com> wrote: > > > > > On Tue, Jul 24, 2012 at 3:03 PM, Chris Manghane <cmang@google.com> wrote: >> >> This patch modifies pmu-profile to allow gooda_feedback access to >> >> > Needs some more context about perf.data -> gcda conversion. > >> >> important gcov to gcda conversion functions and also > > > Not really gcov to gcda conversion functions, but gcov io functions. > >> modifies the gcov pmu format store indices > > "format to store indices into a..." > >> >> in a string table of filenames as opposed to storing the filename inside of every pmu entry. This reduces the amount of size bloat in gcda, especially since the gcda files do a per asm line analysis and this leads to many entries sharing the same filename data anyways. gooda_feedback considers this and outputs the string following the load latency and branch misprediction information. >> >> The changes made belong to one of the following categories: >> 1. Removing static declaration/defition of certain function that are needed by gooda_feedback (pmu-profile.c) >> 2. Modifying the gcov format to use indices instead of string for the filename (gcov-io.h) >> 3. Modifying gcda writing functions to write the index instead of the filename (pmu-profile.c) >> 4. Removing references to filename (gcov-io.c, pmu-profile.c) >> >> This was tested by using crosstool-validate with --testers=crosstool. No other testcases seemed necessary since this patch only modifies data that is relevant to pfmon, which is considered dead now. > > > Needs more testing (discussed with Chris offline). > >> >> >> The patch should be applied to google/gcc-4_7 >> >> The CL for gooda_feedback can be found at Google ref c/31972005 >> >> >> 2012-07-24 Chris Manghane <cmang@google.com> >> >> * libgcc/pmu-profile.c (static int parse_pfmon_load_latency): filename field no longer exists > > > Also mention the removal of "static" from the declarations. > >> >> (parse_load_latency_line): filename field no longer exists >> (parse_branch_mispredict_line): filename field no longer exists > > > For repeating descriptions use or "Ditto." or "Likewise." > >> (__gcov_stop_pmu_profiler): filename field no longer exists >> (gcov_write_ll_line): now writes string table index instead of filename >> (gcov_write_branch_mispredict_line): now writes string table index instead of filename >> (gcov_write_load_latency_infos): filename field no longer exists >> (gcov_write_branch_mispredict_infos): filename field no longer exists >> (gcov_tag_pmu_tool_header_length): filename field no longer exists >> (gcov_write_tool_header): filename field no longer exists >> * gcc/gcov.c (release_structures): filename field no longer exists >> (filter_pmu_data_lines): filename field no longer exists >> * gcc/gcov-io.c (gcov_read_pmu_load_latency_info): removed filename field and added string table index >> (gcov_read_pmu_branch_mispredict_info): removed filename field and added string table index >> (print_load_latency_line): filename field no longer exists >> (print_branch_mispredict_line): filename field no longer exists >> * gcc/gcov-io.h: added new tag for string table printing >> * gcc/gcov-dump.c (tag_pmu_load_latency_info): filename field no longer exists >> (tag_pmu_branch_mispredict_info): filename field no longer exists >> >> Index: libgcc/pmu-profile.c >> =================================================================== >> --- libgcc/pmu-profile.c (revision 189823) >> +++ libgcc/pmu-profile.c (working copy) >> @@ -232,11 +232,11 @@ static int parse_pfmon_load_latency (char *filenam >> static int parse_pfmon_branch_mispredicts (char *filename, void *pmu_data); >> static gcov_unsigned_t gcov_tag_pmu_tool_header_length (gcov_pmu_tool_header_t >> *header); >> -static void gcov_write_tool_header (gcov_pmu_tool_header_t *header); >> -static void gcov_write_load_latency_infos (void *info); >> -static void gcov_write_branch_mispredict_infos (void *info); >> -static void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); >> -static void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t >> +void gcov_write_tool_header (gcov_pmu_tool_header_t *header); >> +void gcov_write_load_latency_infos (void *info); >> +void gcov_write_branch_mispredict_infos (void *info); >> +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); >> +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t >> *brm_info); > > > They should be moved to gcov-io.h with linkage GCOV_LINKAGE which will pick extern when building libgcov (see examples in gcov-io.h). > >> >> static int start_addr2line_symbolizer (pid_t pid); >> static void end_addr2line_symbolizer (void); >> @@ -749,14 +749,12 @@ parse_load_latency_line (char *line, gcov_pmu_ll_i > > > I think it would be cleaner just to remove all the pfmon specific handling as part of this checkin, since that is obsolete and wouldn't work with your changes anyway. > >> >> if (!sep) >> { >> /* Assume entire string is srcfile. */ >> - ll_info->filename = (char *)sym_info; >> ll_info->line = 0; >> } >> else >> { >> /* Terminate the filename string at the separator. */ >> *sep = 0; >> - ll_info->filename = (char *)sym_info; >> /* Convert rest of the sym info to a line number. */ >> ll_info->line = atol (sep+1); >> } >> @@ -765,7 +763,6 @@ parse_load_latency_line (char *line, gcov_pmu_ll_i >> else >> { >> /* No symbolizer available. */ >> - ll_info->filename = NULL; >> ll_info->line = 0; >> ll_info->discriminator = 0; >> } >> @@ -815,14 +812,12 @@ parse_branch_mispredict_line (char *line, gcov_pmu >> if (!sep) >> { >> /* Assume entire string is srcfile. */ >> - brm_info->filename = sym_info; >> brm_info->line = 0; >> } >> else >> { >> /* Terminate the filename string at the separator. */ >> *sep = 0; >> - brm_info->filename = sym_info; >> /* Convert rest of the sym info to a line number. */ >> brm_info->line = atol (sep+1); >> } >> @@ -831,7 +826,6 @@ parse_branch_mispredict_line (char *line, gcov_pmu >> else >> { >> /* No symbolizer available. */ >> - brm_info->filename = NULL; >> brm_info->line = 0; >> brm_info->discriminator = 0; >> } >> @@ -1361,11 +1355,10 @@ __gcov_stop_pmu_profiler (void) >> >> /* Write the load latency information LL_INFO into the gcda file. */ >> >> -static void >> +void >> gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info) >> { >> - gcov_unsigned_t len = GCOV_TAG_PMU_LOAD_LATENCY_LENGTH (ll_info->filename); >> - gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, len); >> + gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, 1); > > > The length of 1 isn't correct - it should be the length of the data associated with this tag. See GCOV_TAG_PMU_LOAD_LATENCY_LENGTH for how it is computing this (and you can just modify that to remove the filename parameter and add 1 for your new tag). > >> gcov_write_unsigned (ll_info->counts); >> gcov_write_unsigned (ll_info->self); >> gcov_write_unsigned (ll_info->cum); >> @@ -1379,31 +1372,29 @@ gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_i >> gcov_write_counter (ll_info->code_addr); >> gcov_write_unsigned (ll_info->line); >> gcov_write_unsigned (ll_info->discriminator); >> - gcov_write_string (ll_info->filename); >> + gcov_write_unsigned (ll_info->filetag); >> } >> >> >> /* Write the branch mispredict information BRM_INFO into the gcda file. */ >> >> -static void >> +void >> gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info) >> { >> - gcov_unsigned_t len = GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH ( >> - brm_info->filename); >> - gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, len); >> + gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, 1); > > > Same thing here with the length computation (see above comment). > >> >> gcov_write_unsigned (brm_info->counts); >> gcov_write_unsigned (brm_info->self); >> gcov_write_unsigned (brm_info->cum); >> gcov_write_counter (brm_info->code_addr); >> gcov_write_unsigned (brm_info->line); >> gcov_write_unsigned (brm_info->discriminator); >> - gcov_write_string (brm_info->filename); >> + gcov_write_unsigned (brm_info->filetag); >> } >> >> /* Write load latency information INFO into the gcda file. The gcda >> file has already been opened and is available for writing. */ >> >> -static void >> +void >> gcov_write_load_latency_infos (void *info) >> { >> unsigned i; >> @@ -1429,7 +1420,7 @@ gcov_write_load_latency_infos (void *info) >> /* Write branch mispredict information INFO into the gcda file. The >> gcda file has already been opened and is available for writing. */ >> >> -static void >> +void >> gcov_write_branch_mispredict_infos (void *info) >> { >> unsigned i; >> @@ -1470,7 +1461,7 @@ gcov_tag_pmu_tool_header_length (gcov_pmu_tool_hea >> /* Write tool header into the gcda file. It assumes that the gcda file >> has already been opened and is available for writing. */ >> >> -static void >> +void >> gcov_write_tool_header (gcov_pmu_tool_header_t *header) >> { >> gcov_unsigned_t len = gcov_tag_pmu_tool_header_length (header); >> Index: gcc/gcov.c >> =================================================================== >> --- gcc/gcov.c (revision 189823) >> +++ gcc/gcov.c (working copy) >> @@ -1008,8 +1008,6 @@ release_structures (void) >> /* delete each element */ >> for (i = 0; i < ll_infos->ll_count; ++i) >> { >> - if (ll_infos->ll_array[i]->filename) >> - XDELETE (ll_infos->ll_array[i]->filename); >> XDELETE (ll_infos->ll_array[i]); >> } >> /* delete the array itself */ >> @@ -1026,8 +1024,6 @@ release_structures (void) >> /* delete each element */ >> for (i = 0; i < brm_infos->brm_count; ++i) >> { >> - if (brm_infos->brm_array[i]->filename) >> - XDELETE (brm_infos->brm_array[i]->filename); >> XDELETE (brm_infos->brm_array[i]); >> } >> /* delete the array itself */ >> @@ -2277,58 +2273,6 @@ filter_pmu_data_lines (source_t *src) >> ll_infos->ll_array = 0; >> brm_infos->brm_array = 0; >> >> - /* Go over all the load latency entries and save the ones >> - corresponding to this source file. */ > > > Don't want to remove this handling, as it is doing the job of deciding which pmu data matches the current file being compiled. You just need to change the handling of how to access the filename. > >> >> - for (i = 0; i < pmu_global_info.ll_infos.ll_count; ++i) >> - { >> - gcov_pmu_ll_info_t *ll_info = pmu_global_info.ll_infos.ll_array[i]; >> - if (0 == strcmp (src->name, ll_info->filename)) >> - { >> - if (!ll_infos->ll_array) >> - { >> - ll_infos->ll_count = 0; >> - ll_infos->alloc_ll_count = 64; >> - ll_infos->ll_array = XCNEWVEC (gcov_pmu_ll_info_t *, >> - ll_infos->alloc_ll_count); >> - } >> - /* Found a matching entry, save it. */ >> - ll_infos->ll_count++; >> - if (ll_infos->ll_count >= ll_infos->alloc_ll_count) >> - { >> - /* need to realloc */ >> - ll_infos->ll_array = (gcov_pmu_ll_info_t **) >> - xrealloc (ll_infos->ll_array, 2 * ll_infos->alloc_ll_count); >> - } >> - ll_infos->ll_array[ll_infos->ll_count - 1] = ll_info; >> - } >> - } >> - >> - /* Go over all the branch mispredict entries and save the ones >> - corresponding to this source file. */ >> - for (i = 0; i < pmu_global_info.brm_infos.brm_count; ++i) >> - { >> - gcov_pmu_brm_info_t *brm_info = pmu_global_info.brm_infos.brm_array[i]; >> - if (0 == strcmp (src->name, brm_info->filename)) >> - { >> - if (!brm_infos->brm_array) >> - { >> - brm_infos->brm_count = 0; >> - brm_infos->alloc_brm_count = 64; >> - brm_infos->brm_array = XCNEWVEC (gcov_pmu_brm_info_t *, >> - brm_infos->alloc_brm_count); >> - } >> - /* Found a matching entry, save it. */ >> - brm_infos->brm_count++; >> - if (brm_infos->brm_count >= brm_infos->alloc_brm_count) >> - { >> - /* need to realloc */ >> - brm_infos->brm_array = (gcov_pmu_brm_info_t **) >> - xrealloc (brm_infos->brm_array, 2 * brm_infos->alloc_brm_count); >> - } >> - brm_infos->brm_array[brm_infos->brm_count - 1] = brm_info; >> - } >> - } >> - >> /* Sort the load latency data according to the line numbers because >> we later iterate over sources in line number order. Normally we >> expect the PMU tool to provide sorted data, but a few entries can >> Index: gcc/gcov-io.c >> =================================================================== >> --- gcc/gcov-io.c (revision 189823) >> +++ gcc/gcov-io.c (working copy) >> @@ -242,7 +242,6 @@ GCOV_LINKAGE void >> gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_t *ll_info, >> gcov_unsigned_t len ATTRIBUTE_UNUSED) >> { >> - const char *filename; >> ll_info->counts = gcov_read_unsigned (); >> ll_info->self = gcov_read_unsigned (); >> ll_info->cum = gcov_read_unsigned (); >> @@ -256,11 +255,7 @@ gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_ >> ll_info->code_addr = gcov_read_counter (); >> ll_info->line = gcov_read_unsigned (); >> ll_info->discriminator = gcov_read_unsigned (); >> - filename = gcov_read_string (); >> - if (filename) >> - ll_info->filename = gcov_canonical_filename (xstrdup (filename)); >> - else >> - ll_info->filename = 0; >> + ll_info->filetag = gcov_read_unsigned (); >> } >> >> /* Read LEN words and construct branch mispredict info BRM_INFO. */ >> @@ -269,18 +264,13 @@ GCOV_LINKAGE void >> gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, >> gcov_unsigned_t len ATTRIBUTE_UNUSED) >> { >> - const char *filename; >> brm_info->counts = gcov_read_unsigned (); >> brm_info->self = gcov_read_unsigned (); >> brm_info->cum = gcov_read_unsigned (); >> brm_info->code_addr = gcov_read_counter (); >> brm_info->line = gcov_read_unsigned (); >> brm_info->discriminator = gcov_read_unsigned (); >> - filename = gcov_read_string (); >> - if (filename) >> - brm_info->filename = gcov_canonical_filename (xstrdup (filename)); >> - else >> - brm_info->filename = 0; >> + brm_info->filetag = gcov_read_unsigned (); >> } >> >> /* Read LEN words from an open gcov file and construct data into pmu >> @@ -779,7 +769,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ >> if (!ll_info) >> return; >> fprintf (fp, " %u %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% " >> - "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", >> + "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", > > > Instead of changing this to print the tag, it would be useful to print both the tag and the filename (as accessed via your string table). Ditto below for the branch mispredict info. > >> >> ll_info->counts, >> convert_unsigned_to_pct (ll_info->self), >> convert_unsigned_to_pct (ll_info->cum), >> @@ -791,7 +781,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ >> convert_unsigned_to_pct (ll_info->gt_1024), >> convert_unsigned_to_pct (ll_info->wself), >> ll_info->code_addr, >> - ll_info->filename, >> + ll_info->filetag, >> ll_info->line, >> ll_info->discriminator); >> if (newline == add_newline) >> @@ -807,12 +797,12 @@ print_branch_mispredict_line (FILE *fp, const gcov >> { >> if (!brm_info) >> return; >> - fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", >> + fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", >> brm_info->counts, >> convert_unsigned_to_pct (brm_info->self), >> convert_unsigned_to_pct (brm_info->cum), >> brm_info->code_addr, >> - brm_info->filename, >> + brm_info->filetag, >> brm_info->line, >> brm_info->discriminator); >> if (newline == add_newline) > > > Need to add i/o handling of new string table. > >> Index: gcc/gcov-io.h >> =================================================================== >> --- gcc/gcov-io.h (revision 189823) >> +++ gcc/gcov-io.h (working copy) >> @@ -449,6 +449,7 @@ typedef HOST_WIDEST_INT gcov_type; >> (gcov_string_length (filename) + 5 + 2) >> #define GCOV_TAG_PMU_TOOL_HEADER ((gcov_unsigned_t)0xa9000000) >> #define GCOV_TAG_MODULE_INFO ((gcov_unsigned_t)0xab000000) >> +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY ((gcov_unsigned_t)0xad000000) >> >> /* Counters that are collected. */ >> #define GCOV_COUNTER_ARCS 0 /* Arc transitions. */ >> @@ -635,7 +636,7 @@ typedef struct gcov_pmu_load_latency_info >> gcov_type code_addr; /* the actual miss address (pc+1 for Intel) */ >> gcov_unsigned_t line; /* line number corresponding to this miss */ >> gcov_unsigned_t discriminator; /* discriminator information for this miss */ >> - char *filename; /* filename corresponding to this miss */ >> + gcov_unsigned_t filetag; /* location in string table of filename corresponding to event */ >> } gcov_pmu_ll_info_t; >> >> /* This structure is used during runtime as well as in gcov. */ >> @@ -665,7 +666,7 @@ typedef struct gcov_pmu_branch_mispredict_info >> gcov_type code_addr; /* the actual mispredict address */ >> gcov_unsigned_t line; /* line number corresponding to this event */ >> gcov_unsigned_t discriminator; /* discriminator for this event */ >> - char *filename; /* filename corresponding to this event */ >> + gcov_unsigned_t filetag; /* location in string table of filename corresponding to event */ >> } gcov_pmu_brm_info_t; >> >> /* This structure is used during runtime as well as in gcov. */ >> Index: gcc/gcov-dump.c >> =================================================================== >> --- gcc/gcov-dump.c (revision 189823) >> +++ gcc/gcov-dump.c (working copy) >> @@ -573,7 +573,6 @@ tag_pmu_load_latency_info (const char *filename AT >> >> gcov_pmu_ll_info_t ll_info; >> gcov_read_pmu_load_latency_info (&ll_info, length); >> print_load_latency_line (stdout, &ll_info, no_newline); >> - free (ll_info.filename); >> } >> >> /* Read gcov tag GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO from the gcda >> @@ -586,7 +585,6 @@ tag_pmu_branch_mispredict_info (const char *filena >> gcov_pmu_brm_info_t brm_info; >> gcov_read_pmu_branch_mispredict_info (&brm_info, length); >> print_branch_mispredict_line (stdout, &brm_info, no_newline); >> - free (brm_info.filename); >> } > > > Need to add printing of new string table. > > Teresa >> >> >> >> >> -- >> This patch is available for review at http://codereview.appspot.com/6427063 > > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Sign in to reply to this message.
This patch has been updated to reflect changes in patch r190247, which removed pfmon support. The patch should be applied to google/main Tested with crosstools. 2012-08-14 Chris Manghane <cmang@google.com> * libgcc/pmu-profile.c (gcov_write_load_latency_infos): Removed unused function. (gcov_write_branch_mispredict_infos): Ditto. (destroy_load_latency_infos): Removed static keyword. (init_pmu_branch_mispredict): Ditto. (gcov_write_ll_line): Ditto, plus replaced filename field with filetag. (gcov_write_branch_mispredict_line): Ditto. (gcov_write_string_table_entry): New function. (gcov_write_tool_header): Removed static keyword. * gcc/gcov.c (release_structures): Removed filename field from PMU structures. (filter_pmu_data_lines): Added PMU string table support. (process_pmu_profile): Ditto. * gcc/gcov-io.c (gcov_read_pmu_load_latency_info): Replaced filename field with filetag. (gcov_read_pmu_branch_mispredict_info): Ditto. (gcov_read_pmu_string_table_entry): New Function. (print_load_latency_line): Replaced filename field with filetag. (print_branch_mispredict_line): Ditto. (print_string_table_entry): New function. * gcc/gcov-io.h (GCOV_TAG_PMU_LOAD_LATENCY_LENGTH): Replaced filename field with filetag (GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH): Ditto. (GCOV_TAG_PMU_STRING_TABLE_ENTRY): Added new tag. (GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH): Ditto. (gcov_pmu_load_latency_info): Replaced filename field with filetag. (gcov_pmu_branch_mispredict_info): Ditto. (gcov_pmu_string_table_entry): New struct. (gcov_pmu_string_table): New struct. * gcc/gcov-dump.c (tag_pmu_load_latency_info): Removed PMU filename field. (tag_pmu_branch_mispredict_info): Ditto. (tag_pmu_string_table_entry): New function. Index: libgcc/pmu-profile.c =================================================================== --- libgcc/pmu-profile.c (revision 190362) +++ libgcc/pmu-profile.c (working copy) @@ -1,3 +1,4 @@ + /* Performance monitoring unit (PMU) profiler. If available, use an external tool to collect hardware performance counter data and write it in the .gcda files. @@ -74,13 +75,13 @@ static void destroy_load_latency_infos (void *info static void destroy_branch_mispredict_infos (void *info); static gcov_unsigned_t gcov_tag_pmu_tool_header_length (gcov_pmu_tool_header_t *header); -static void gcov_write_tool_header (gcov_pmu_tool_header_t *header); -static void gcov_write_load_latency_infos (void *info); -static void gcov_write_branch_mispredict_infos (void *info); -static void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); -static void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t +void gcov_write_tool_header (gcov_pmu_tool_header_t *header); +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info); +void gcov_write_string_table_entry (const gcov_pmu_st_entry_t *st_entry); + /* Convert a fractional PCT to an unsigned integer after muliplying by 100. */ @@ -156,11 +157,11 @@ init_pmu_branch_mispredict (void) /* Write the load latency information LL_INFO into the gcda file. */ -static void +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_LOAD_LATENCY_LENGTH (ll_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, + GCOV_TAG_PMU_LOAD_LATENCY_LENGTH()); gcov_write_unsigned (ll_info->counts); gcov_write_unsigned (ll_info->self); gcov_write_unsigned (ll_info->cum); @@ -174,27 +175,38 @@ gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_i gcov_write_counter (ll_info->code_addr); gcov_write_unsigned (ll_info->line); gcov_write_unsigned (ll_info->discriminator); - gcov_write_string (ll_info->filename); + gcov_write_unsigned (ll_info->filetag); } /* Write the branch mispredict information BRM_INFO into the gcda file. */ -static void +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH ( - brm_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, + GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH()); gcov_write_unsigned (brm_info->counts); gcov_write_unsigned (brm_info->self); gcov_write_unsigned (brm_info->cum); gcov_write_counter (brm_info->code_addr); gcov_write_unsigned (brm_info->line); gcov_write_unsigned (brm_info->discriminator); - gcov_write_string (brm_info->filename); + gcov_write_unsigned (brm_info->filetag); } +/* Write the string table entry ST_ENTRY into the gcda file. */ + +void +gcov_write_string_table_entry (const gcov_pmu_st_entry_t *st_entry) +{ + gcov_write_tag_length (GCOV_TAG_PMU_STRING_TABLE_ENTRY, + GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH( + st_entry->filename)); + gcov_write_unsigned(st_entry->index); + gcov_write_string(st_entry->filename); +} + /* Compute TOOL_HEADER length for writing into the gcov file. */ static gcov_unsigned_t @@ -216,7 +228,7 @@ gcov_tag_pmu_tool_header_length (gcov_pmu_tool_hea /* Write tool header into the gcda file. It assumes that the gcda file has already been opened and is available for writing. */ -static void +void gcov_write_tool_header (gcov_pmu_tool_header_t *header) { gcov_unsigned_t len = gcov_tag_pmu_tool_header_length (header); Index: gcc/gcov.c =================================================================== --- gcc/gcov.c (revision 190359) +++ gcc/gcov.c (working copy) @@ -222,6 +222,7 @@ typedef struct pmu_data { ll_infos_t ll_infos; brm_infos_t brm_infos; + string_table_t string_table; } pmu_data_t; /* Describes a single line of source. Contains a chain of basic blocks @@ -975,6 +976,7 @@ release_structures (void) function_t *fn; ll_infos_t *ll_infos = &pmu_global_info.ll_infos; brm_infos_t *brm_infos = &pmu_global_info.brm_infos; + string_table_t *string_table = &pmu_global_info.string_table; for (ix = n_sources; ix--;) { @@ -1008,8 +1010,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < ll_infos->ll_count; ++i) { - if (ll_infos->ll_array[i]->filename) - XDELETE (ll_infos->ll_array[i]->filename); XDELETE (ll_infos->ll_array[i]); } /* delete the array itself */ @@ -1026,8 +1026,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < brm_infos->brm_count; ++i) { - if (brm_infos->brm_array[i]->filename) - XDELETE (brm_infos->brm_array[i]->filename); XDELETE (brm_infos->brm_array[i]); } /* delete the array itself */ @@ -1035,6 +1033,22 @@ release_structures (void) brm_infos->brm_array = NULL; brm_infos->brm_count = 0; } + + /* Cleanup PMU string table entries. */ + if (string_table->st_count) + { + unsigned i; + + /* delete each element */ + for (i = 0; i < string_table->st_count; ++i) + { + XDELETE (string_table->st_array[i]); + } + /* delete the array itself */ + XDELETE (string_table->st_array); + string_table->st_array = NULL; + string_table->st_count = 0; + } } /* Generate the names of the graph and data files. If OBJECT_DIRECTORY @@ -2263,17 +2277,21 @@ filter_pmu_data_lines (source_t *src) int changed; ll_infos_t *ll_infos; /* load latency information for this source */ brm_infos_t *brm_infos; /* branch mispredict information for this source */ + string_table_t *string_table; /* string table information for this source */ if (pmu_global_info.ll_infos.ll_count == 0 && - pmu_global_info.brm_infos.brm_count == 0) + pmu_global_info.brm_infos.brm_count == 0 && + pmu_global_info.string_table.st_count == 0) /* If there are no global entries, there is nothing to filter. */ return; src->pmu_data = XCNEW (pmu_data_t); ll_infos = &src->pmu_data->ll_infos; brm_infos = &src->pmu_data->brm_infos; + string_table = &src->pmu_data->string_table; ll_infos->pmu_tool_header = pmu_global_info.ll_infos.pmu_tool_header; brm_infos->pmu_tool_header = pmu_global_info.brm_infos.pmu_tool_header; + string_table->pmu_tool_header = pmu_global_info.string_table.pmu_tool_header; ll_infos->ll_array = 0; brm_infos->brm_array = 0; @@ -2282,7 +2300,8 @@ filter_pmu_data_lines (source_t *src) for (i = 0; i < pmu_global_info.ll_infos.ll_count; ++i) { gcov_pmu_ll_info_t *ll_info = pmu_global_info.ll_infos.ll_array[i]; - if (0 == strcmp (src->name, ll_info->filename)) + if (0 == strcmp (src->name, + string_table->st_array[ll_info->filetag - 1]->filename)) { if (!ll_infos->ll_array) { @@ -2295,9 +2314,9 @@ filter_pmu_data_lines (source_t *src) ll_infos->ll_count++; if (ll_infos->ll_count >= ll_infos->alloc_ll_count) { - /* need to realloc */ + /* need to realloc */ ll_infos->ll_array = (gcov_pmu_ll_info_t **) - xrealloc (ll_infos->ll_array, 2 * ll_infos->alloc_ll_count); + xrealloc (ll_infos->ll_array, 2 * ll_infos->alloc_ll_count); } ll_infos->ll_array[ll_infos->ll_count - 1] = ll_info; } @@ -2308,7 +2327,8 @@ filter_pmu_data_lines (source_t *src) for (i = 0; i < pmu_global_info.brm_infos.brm_count; ++i) { gcov_pmu_brm_info_t *brm_info = pmu_global_info.brm_infos.brm_array[i]; - if (0 == strcmp (src->name, brm_info->filename)) + if (0 == strcmp (src->name, + string_table->st_array[brm_info->filetag - 1]->filename)) { if (!brm_infos->brm_array) { @@ -2323,12 +2343,14 @@ filter_pmu_data_lines (source_t *src) { /* need to realloc */ brm_infos->brm_array = (gcov_pmu_brm_info_t **) - xrealloc (brm_infos->brm_array, 2 * brm_infos->alloc_brm_count); + xrealloc (brm_infos->brm_array, + 2 * brm_infos->alloc_brm_count); } brm_infos->brm_array[brm_infos->brm_count - 1] = brm_info; } } + /* Sort the load latency data according to the line numbers because we later iterate over sources in line number order. Normally we expect the PMU tool to provide sorted data, but a few entries can @@ -2914,6 +2936,7 @@ static void process_pmu_profile (void) int error = 0; ll_infos_t *ll_infos = &pmu_global_info.ll_infos; brm_infos_t *brm_infos = &pmu_global_info.brm_infos; + string_table_t *string_table = &pmu_global_info.string_table; /* Construct path for pmuprofile.gcda filename. */ create_file_names (pmu_profile_filename); @@ -2953,6 +2976,11 @@ static void process_pmu_profile (void) brm_infos->brm_array = XCNEWVEC (gcov_pmu_brm_info_t *, brm_infos->alloc_brm_count); + string_table->st_count = 0; + string_table->alloc_st_count = 64; + string_table->st_array = XCNEWVEC (gcov_pmu_st_entry_t *, + string_table->alloc_st_count); + while ((tag = gcov_read_unsigned ())) { unsigned length = gcov_read_unsigned (); @@ -2991,7 +3019,22 @@ static void process_pmu_profile (void) ll_infos->pmu_tool_header = tool_header; brm_infos->pmu_tool_header = tool_header; } + else if (tag == GCOV_TAG_PMU_STRING_TABLE_ENTRY) + { + gcov_pmu_st_entry_t *st_entry = XCNEW (gcov_pmu_st_entry_t); + gcov_read_pmu_string_table_entry(st_entry, length); + string_table->st_count++; + if (string_table->st_count >= string_table->alloc_st_count) + { + string_table->alloc_st_count *= 2; + string_table->st_array = (gcov_pmu_st_entry_t **) + xrealloc (string_table->st_array, + string_table->alloc_st_count); + } + string_table->st_array[string_table->st_count - 1] = st_entry; + } + gcov_sync (base, length); if ((error = gcov_is_error ())) { Index: gcc/gcov-io.c =================================================================== --- gcc/gcov-io.c (revision 190359) +++ gcc/gcov-io.c (working copy) @@ -242,7 +242,6 @@ GCOV_LINKAGE void gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_t *ll_info, gcov_unsigned_t len ATTRIBUTE_UNUSED) { - const char *filename; ll_info->counts = gcov_read_unsigned (); ll_info->self = gcov_read_unsigned (); ll_info->cum = gcov_read_unsigned (); @@ -256,11 +255,7 @@ gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_ ll_info->code_addr = gcov_read_counter (); ll_info->line = gcov_read_unsigned (); ll_info->discriminator = gcov_read_unsigned (); - filename = gcov_read_string (); - if (filename) - ll_info->filename = gcov_canonical_filename (xstrdup (filename)); - else - ll_info->filename = 0; + ll_info->filetag = gcov_read_unsigned (); } /* Read LEN words and construct branch mispredict info BRM_INFO. */ @@ -269,20 +264,29 @@ GCOV_LINKAGE void gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, gcov_unsigned_t len ATTRIBUTE_UNUSED) { - const char *filename; brm_info->counts = gcov_read_unsigned (); brm_info->self = gcov_read_unsigned (); brm_info->cum = gcov_read_unsigned (); brm_info->code_addr = gcov_read_counter (); brm_info->line = gcov_read_unsigned (); brm_info->discriminator = gcov_read_unsigned (); - filename = gcov_read_string (); - if (filename) - brm_info->filename = gcov_canonical_filename (xstrdup (filename)); - else - brm_info->filename = 0; + brm_info->filetag = gcov_read_unsigned (); } +/* Read LEN words from an open gcov file and construct data into a + string table entry */ + +GCOV_LINKAGE void +gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t* st_entry, + gcov_unsigned_t len ATTRIBUTE_UNUSED) +{ + const char *str; + + st_entry->index = gcov_read_unsigned (); + str = gcov_read_string (); + st_entry->filename = str ? gcov_canonical_filename (xstrdup(str)) : 0; +} + /* Read LEN words from an open gcov file and construct data into pmu tool header TOOL_HEADER. */ @@ -779,7 +783,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ if (!ll_info) return; fprintf (fp, " %u %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% " - "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", + "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", ll_info->counts, convert_unsigned_to_pct (ll_info->self), convert_unsigned_to_pct (ll_info->cum), @@ -791,7 +795,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ convert_unsigned_to_pct (ll_info->gt_1024), convert_unsigned_to_pct (ll_info->wself), ll_info->code_addr, - ll_info->filename, + ll_info->filetag, ll_info->line, ll_info->discriminator); if (newline == add_newline) @@ -807,18 +811,31 @@ print_branch_mispredict_line (FILE *fp, const gcov { if (!brm_info) return; - fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", + fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", brm_info->counts, convert_unsigned_to_pct (brm_info->self), convert_unsigned_to_pct (brm_info->cum), brm_info->code_addr, - brm_info->filename, + brm_info->filetag, brm_info->line, brm_info->discriminator); if (newline == add_newline) fprintf (fp, "\n"); } +/* Print STRING_TABLE_ENTRY into the file pointed by FP. NEWLINE specifies + whether or not to print a trailing newline. */ + +GCOV_LINKAGE void +print_pmu_string_table_entry (FILE *fp, const gcov_pmu_st_entry_t* st_entry, + const enum print_newline newline) { + if (!st_entry) + return; + fprintf (fp, " %d %s", st_entry->index, st_entry->filename); + if (newline == add_newline) + fprintf (fp, "\n"); +} + /* Print TOOL_HEADER into the file pointed by FP. NEWLINE specifies whether or not to print a trailing newline. */ Index: gcc/gcov-io.h =================================================================== --- gcc/gcov-io.h (revision 190359) +++ gcc/gcov-io.h (working copy) @@ -442,13 +442,14 @@ typedef HOST_WIDEST_INT gcov_type; #define GCOV_TAG_SUMMARY_LENGTH \ (1 + GCOV_COUNTERS_SUMMABLE * (3 + 3 * 2)) #define GCOV_TAG_PMU_LOAD_LATENCY_INFO ((gcov_unsigned_t)0xa5000000) -#define GCOV_TAG_PMU_LOAD_LATENCY_LENGTH(filename) \ - (gcov_string_length (filename) + 12 + 2) +#define GCOV_TAG_PMU_LOAD_LATENCY_LENGTH() (15) #define GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO ((gcov_unsigned_t)0xa7000000) -#define GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH(filename) \ - (gcov_string_length (filename) + 5 + 2) +#define GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH() (8) #define GCOV_TAG_PMU_TOOL_HEADER ((gcov_unsigned_t)0xa9000000) #define GCOV_TAG_MODULE_INFO ((gcov_unsigned_t)0xab000000) +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY ((gcov_unsigned_t)0xad000000) +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH(filename) \ + (gcov_string_length (filename) + 1) /* Counters that are collected. */ #define GCOV_COUNTER_ARCS 0 /* Arc transitions. */ @@ -635,7 +636,7 @@ typedef struct gcov_pmu_load_latency_info gcov_type code_addr; /* the actual miss address (pc+1 for Intel) */ gcov_unsigned_t line; /* line number corresponding to this miss */ gcov_unsigned_t discriminator; /* discriminator information for this miss */ - char *filename; /* filename corresponding to this miss */ + gcov_unsigned_t filetag; /* location in string table of filename */ } gcov_pmu_ll_info_t; /* This structure is used during runtime as well as in gcov. */ @@ -665,7 +666,7 @@ typedef struct gcov_pmu_branch_mispredict_info gcov_type code_addr; /* the actual mispredict address */ gcov_unsigned_t line; /* line number corresponding to this event */ gcov_unsigned_t discriminator; /* discriminator for this event */ - char *filename; /* filename corresponding to this event */ + gcov_unsigned_t filetag; /* location in string table of filename */ } gcov_pmu_brm_info_t; /* This structure is used during runtime as well as in gcov. */ @@ -682,6 +683,25 @@ typedef struct branch_mispredict_infos gcov_pmu_tool_header_t *pmu_tool_header; } brm_infos_t; +typedef struct gcov_pmu_string_table_entry +{ + gcov_unsigned_t index; /* The corresponding string table index */ + char* filename; /* The filename that belongs at this index */ +} gcov_pmu_st_entry_t; + +typedef struct string_table +{ + /* An array describing the total number of string table entries. */ + gcov_pmu_st_entry_t **st_array; + /* The total number of entries in the above array. */ + unsigned st_count; + /* The total number of entries currently allocated in the array. + Used for bookkeeping. */ + unsigned alloc_st_count; + /* PMU tool header */ + gcov_pmu_tool_header_t *pmu_tool_header; +} string_table_t; + /* Structures embedded in coveraged program. The structures generated by write_profile must match these. */ @@ -871,6 +891,10 @@ GCOV_LINKAGE void gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, gcov_unsigned_t len) ATTRIBUTE_HIDDEN; GCOV_LINKAGE void +gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t* entry, + gcov_unsigned_t len) ATTRIBUTE_HIDDEN; + +GCOV_LINKAGE void gcov_read_pmu_tool_header (gcov_pmu_tool_header_t *tool_header, gcov_unsigned_t len) ATTRIBUTE_HIDDEN; #ifndef __GCOV_KERNEL__ @@ -887,6 +911,9 @@ GCOV_LINKAGE void print_load_latency_line (FILE *f GCOV_LINKAGE void print_branch_mispredict_line (FILE *fp, const gcov_pmu_brm_info_t *brm_info, const enum print_newline); +GCOV_LINKAGE void +print_pmu_string_table_entry (FILE* fp, const gcov_pmu_st_entry_t* entry, + const enum print_newline); GCOV_LINKAGE void print_pmu_tool_header (FILE *fp, gcov_pmu_tool_header_t *tool_header, const enum print_newline); Index: gcc/gcov-dump.c =================================================================== --- gcc/gcov-dump.c (revision 190359) +++ gcc/gcov-dump.c (working copy) @@ -43,6 +43,7 @@ static void tag_summary (const char *, unsigned, u static void tag_module_info (const char *, unsigned, unsigned); static void tag_pmu_load_latency_info (const char *, unsigned, unsigned); static void tag_pmu_branch_mispredict_info (const char *, unsigned, unsigned); +static void tag_pmu_string_table_entry (const char*, unsigned, unsigned); static void tag_pmu_tool_header (const char *, unsigned, unsigned); extern int main (int, char **); @@ -84,6 +85,8 @@ static const tag_format_t tag_table[] = {GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, "PMU_BRANCH_MISPREDICT_INFO", tag_pmu_branch_mispredict_info}, {GCOV_TAG_PMU_TOOL_HEADER, "PMU_TOOL_HEADER", tag_pmu_tool_header}, + {GCOV_TAG_PMU_STRING_TABLE_ENTRY, "PMU_STRING_TABLE_ENTRY", + tag_pmu_string_table_entry}, {0, NULL, NULL} }; @@ -573,7 +576,6 @@ tag_pmu_load_latency_info (const char *filename AT gcov_pmu_ll_info_t ll_info; gcov_read_pmu_load_latency_info (&ll_info, length); print_load_latency_line (stdout, &ll_info, no_newline); - free (ll_info.filename); } /* Read gcov tag GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO from the gcda @@ -586,9 +588,17 @@ tag_pmu_branch_mispredict_info (const char *filena gcov_pmu_brm_info_t brm_info; gcov_read_pmu_branch_mispredict_info (&brm_info, length); print_branch_mispredict_line (stdout, &brm_info, no_newline); - free (brm_info.filename); } +static void +tag_pmu_string_table_entry (const char *filename ATTRIBUTE_UNUSED, + unsigned tag ATTRIBUTE_UNUSED, unsigned length) +{ + gcov_pmu_st_entry_t st_entry; + gcov_read_pmu_string_table_entry(&st_entry, length); + print_pmu_string_table_entry(stdout, &st_entry, no_newline); + free(st_entry.filename); +} /* Read gcov tag GCOV_TAG_PMU_TOOL_HEADER from the gcda file and print the contents in a human readable form. */ -- This patch is available for review at http://codereview.appspot.com/6427063
Sign in to reply to this message.
Hi David, Could you take a look at the patch I mailed to gcc-patches when you get a chance? Thanks, Chris
Sign in to reply to this message.
Hi Rong, Could you take a look at the patch I mailed to gcc-patches when you get a chance? It reduces the gcda size bloat issue by replacing gcov_pmu data with a filetag field that holds the position of the correct filename inside of the newly added string table. Thanks, Chris
Sign in to reply to this message.
Where is the string table management code? The gcov.c file is not properly uploaded either. David http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c File gcc/gcov-io.c (right): http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode280 gcc/gcov-io.c:280: gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t* st_entry, Fix format: ..entry_t *st_entry, http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode281 gcc/gcov-io.c:281: gcov_unsigned_t len ATTRIBUTE_UNUSED) Why having an unused parameter? Can it be used in assertion check? http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode830 gcc/gcov-io.c:830: print_pmu_string_table_entry (FILE *fp, const gcov_pmu_st_entry_t* st_entry, Fix format. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode831 gcc/gcov-io.c:831: const enum print_newline newline) { '{' goes to the new line. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h File gcc/gcov-io.h (right): http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode699 gcc/gcov-io.h:699: Used for bookkeeping. */ typo. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode916 gcc/gcov-io.h:916: const enum print_newline); Fix indentation.
Sign in to reply to this message.
Ok, I fixed most of the problem you found. I'm not sure what happened with gcov.c, but I'll try uploading it again. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c File gcc/gcov-io.c (right): http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode280 gcc/gcov-io.c:280: gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t* st_entry, On 2012/08/24 20:42:03, davidxl wrote: > Fix format: > > ..entry_t *st_entry, Done. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode281 gcc/gcov-io.c:281: gcov_unsigned_t len ATTRIBUTE_UNUSED) On 2012/08/24 20:42:03, davidxl wrote: > Why having an unused parameter? Can it be used in assertion check? I'm following the format of the pmu_branch_mispredict/load_latency_info function defined above. Should I remove the ATTRIBUTE_UNUSED symbol from them as well? http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode830 gcc/gcov-io.c:830: print_pmu_string_table_entry (FILE *fp, const gcov_pmu_st_entry_t* st_entry, On 2012/08/24 20:42:03, davidxl wrote: > Fix format. Done. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode831 gcc/gcov-io.c:831: const enum print_newline newline) { On 2012/08/24 20:42:03, davidxl wrote: > '{' goes to the new line. Done. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h File gcc/gcov-io.h (right): http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode699 gcc/gcov-io.h:699: Used for bookkeeping. */ On 2012/08/24 20:42:03, davidxl wrote: > typo. Sorry, I don't notice the typo here. This line is copied from load_latency/branch_mispredict_infos. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode916 gcc/gcov-io.h:916: const enum print_newline); On 2012/08/24 20:42:03, davidxl wrote: > Fix indentation. Done.
Sign in to reply to this message.
Also, what did you mean by there being missing string table management code? On Fri, Aug 24, 2012 at 2:23 PM, <cmang@google.com> wrote: > > Ok, I fixed most of the problem you found. I'm not sure what happened > with gcov.c, but I'll try uploading it again. > > > > http://codereview.appspot.com/**6427063/diff/5001/gcc/gcov-io.**c<http://code... > File gcc/gcov-io.c (right): > > http://codereview.appspot.com/**6427063/diff/5001/gcc/gcov-io.** > c#newcode280<http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode280> > gcc/gcov-io.c:280: gcov_read_pmu_string_table_**entry > (gcov_pmu_st_entry_t* st_entry, > On 2012/08/24 20:42:03, davidxl wrote: > >> Fix format: >> > > ..entry_t *st_entry, >> > > Done. > > > http://codereview.appspot.com/**6427063/diff/5001/gcc/gcov-io.** > c#newcode281<http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode281> > gcc/gcov-io.c:281: gcov_unsigned_t len ATTRIBUTE_UNUSED) > On 2012/08/24 20:42:03, davidxl wrote: > >> Why having an unused parameter? Can it be used in assertion check? >> > > I'm following the format of the pmu_branch_mispredict/load_**latency_info > function defined above. Should I remove the ATTRIBUTE_UNUSED symbol from > them as well? > > > http://codereview.appspot.com/**6427063/diff/5001/gcc/gcov-io.** > c#newcode830<http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode830> > gcc/gcov-io.c:830: print_pmu_string_table_entry (FILE *fp, const > gcov_pmu_st_entry_t* st_entry, > On 2012/08/24 20:42:03, davidxl wrote: > >> Fix format. >> > > Done. > > > http://codereview.appspot.com/**6427063/diff/5001/gcc/gcov-io.** > c#newcode831<http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode831> > gcc/gcov-io.c:831: const enum print_newline newline) { > On 2012/08/24 20:42:03, davidxl wrote: > >> '{' goes to the new line. >> > > Done. > > > http://codereview.appspot.com/**6427063/diff/5001/gcc/gcov-io.**h<http://code... > File gcc/gcov-io.h (right): > > http://codereview.appspot.com/**6427063/diff/5001/gcc/gcov-io.** > h#newcode699<http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode699> > gcc/gcov-io.h:699: Used for bookkeeping. */ > On 2012/08/24 20:42:03, davidxl wrote: > >> typo. >> > > Sorry, I don't notice the typo here. This line is copied from > load_latency/branch_**mispredict_infos. > > > http://codereview.appspot.com/**6427063/diff/5001/gcc/gcov-io.** > h#newcode916<http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode916> > gcc/gcov-io.h:916: const enum print_newline); > On 2012/08/24 20:42:03, davidxl wrote: > >> Fix indentation. >> > > Done. > > http://codereview.appspot.com/**6427063/<http://codereview.appspot.com/6427063/> >
Sign in to reply to this message.
I don't see any code where a string table is declared nor created ... David On Fri, Aug 24, 2012 at 2:26 PM, Chris Manghane <cmang@google.com> wrote: > Also, what did you mean by there being missing string table management code? > > > On Fri, Aug 24, 2012 at 2:23 PM, <cmang@google.com> wrote: >> >> >> Ok, I fixed most of the problem you found. I'm not sure what happened >> with gcov.c, but I'll try uploading it again. >> >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c >> File gcc/gcov-io.c (right): >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode280 >> gcc/gcov-io.c:280: gcov_read_pmu_string_table_entry >> (gcov_pmu_st_entry_t* st_entry, >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> Fix format: >> >> >>> ..entry_t *st_entry, >> >> >> Done. >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode281 >> gcc/gcov-io.c:281: gcov_unsigned_t len ATTRIBUTE_UNUSED) >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> Why having an unused parameter? Can it be used in assertion check? >> >> >> I'm following the format of the pmu_branch_mispredict/load_latency_info >> function defined above. Should I remove the ATTRIBUTE_UNUSED symbol from >> them as well? >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode830 >> gcc/gcov-io.c:830: print_pmu_string_table_entry (FILE *fp, const >> gcov_pmu_st_entry_t* st_entry, >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> Fix format. >> >> >> Done. >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode831 >> gcc/gcov-io.c:831: const enum print_newline newline) { >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> '{' goes to the new line. >> >> >> Done. >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h >> File gcc/gcov-io.h (right): >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode699 >> gcc/gcov-io.h:699: Used for bookkeeping. */ >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> typo. >> >> >> Sorry, I don't notice the typo here. This line is copied from >> load_latency/branch_mispredict_infos. >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode916 >> gcc/gcov-io.h:916: const enum print_newline); >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> Fix indentation. >> >> >> Done. >> >> http://codereview.appspot.com/6427063/ > >
Sign in to reply to this message.
Oh ok, that's because of the uploading problem (although I seem to be able to view gcov.c now). In gcov.c and gcov-dump.c, I've added code that stores data with the tag TAG_PMU_STRING_TABLE_ENTRY into a string table that ll/brm_info reference when they need the appropriate filename. Another use of this is inside https://critique.corp.google.com/#review/31972005, where I use the string table writing methods added to gcov-io.*. Chris On Fri, Aug 24, 2012 at 2:32 PM, Xinliang David Li <davidxl@google.com>wrote: > > I don't see any code where a string table is declared nor created ... > > David > > > On Fri, Aug 24, 2012 at 2:26 PM, Chris Manghane <cmang@google.com> wrote: > > Also, what did you mean by there being missing string table management > code? > > > > > > On Fri, Aug 24, 2012 at 2:23 PM, <cmang@google.com> wrote: > >> > >> > >> Ok, I fixed most of the problem you found. I'm not sure what happened > >> with gcov.c, but I'll try uploading it again. > >> > >> > >> > >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c > >> File gcc/gcov-io.c (right): > >> > >> > http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode280 > >> gcc/gcov-io.c:280: gcov_read_pmu_string_table_entry > >> (gcov_pmu_st_entry_t* st_entry, > >> On 2012/08/24 20:42:03, davidxl wrote: > >>> > >>> Fix format: > >> > >> > >>> ..entry_t *st_entry, > >> > >> > >> Done. > >> > >> > >> > http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode281 > >> gcc/gcov-io.c:281: gcov_unsigned_t len ATTRIBUTE_UNUSED) > >> On 2012/08/24 20:42:03, davidxl wrote: > >>> > >>> Why having an unused parameter? Can it be used in assertion check? > >> > >> > >> I'm following the format of the pmu_branch_mispredict/load_latency_info > >> function defined above. Should I remove the ATTRIBUTE_UNUSED symbol > from > >> them as well? > >> > >> > >> > http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode830 > >> gcc/gcov-io.c:830: print_pmu_string_table_entry (FILE *fp, const > >> gcov_pmu_st_entry_t* st_entry, > >> On 2012/08/24 20:42:03, davidxl wrote: > >>> > >>> Fix format. > >> > >> > >> Done. > >> > >> > >> > http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode831 > >> gcc/gcov-io.c:831: const enum print_newline newline) { > >> On 2012/08/24 20:42:03, davidxl wrote: > >>> > >>> '{' goes to the new line. > >> > >> > >> Done. > >> > >> > >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h > >> File gcc/gcov-io.h (right): > >> > >> > http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode699 > >> gcc/gcov-io.h:699: Used for bookkeeping. */ > >> On 2012/08/24 20:42:03, davidxl wrote: > >>> > >>> typo. > >> > >> > >> Sorry, I don't notice the typo here. This line is copied from > >> load_latency/branch_mispredict_infos. > >> > >> > >> > http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode916 > >> gcc/gcov-io.h:916: const enum print_newline); > >> On 2012/08/24 20:42:03, davidxl wrote: > >>> > >>> Fix indentation. > >> > >> > >> Done. > >> > >> http://codereview.appspot.com/6427063/ > > > > >
Sign in to reply to this message.
Fixed formatting issues. The patch should be applied to google/main Tested with crosstools. 2012-08-24 Chris Manghane <cmang@google.com> * libgcc/pmu-profile.c (gcov_write_load_latency_infos): Removed unused function. (gcov_write_branch_mispredict_infos): Ditto. (destroy_load_latency_infos): Removed static keyword. (init_pmu_branch_mispredict): Ditto. (gcov_write_ll_line): Ditto, plus replaced filename field with filetag. (gcov_write_branch_mispredict_line): Ditto. (gcov_write_string_table_entry): New function. (gcov_write_tool_header): Removed static keyword. * gcc/gcov.c (release_structures): Removed filename field from PMU structures. (filter_pmu_data_lines): Added PMU string table support. (process_pmu_profile): Ditto. * gcc/gcov-io.c (gcov_read_pmu_load_latency_info): Replaced filename field with filetag. (gcov_read_pmu_branch_mispredict_info): Ditto. (gcov_read_pmu_string_table_entry): New Function. (print_load_latency_line): Replaced filename field with filetag. (print_branch_mispredict_line): Ditto. (print_string_table_entry): New function. * gcc/gcov-io.h (GCOV_TAG_PMU_LOAD_LATENCY_LENGTH): Replaced filename field with filetag (GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH): Ditto. (GCOV_TAG_PMU_STRING_TABLE_ENTRY): Added new tag. (GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH): Ditto. (gcov_pmu_load_latency_info): Replaced filename field with filetag. (gcov_pmu_branch_mispredict_info): Ditto. (gcov_pmu_string_table_entry): New struct. (gcov_pmu_string_table): New struct. * gcc/gcov-dump.c (tag_pmu_load_latency_info): Removed PMU filename field. (tag_pmu_branch_mispredict_info): Ditto. (tag_pmu_string_table_entry): New function. Index: libgcc/pmu-profile.c =================================================================== --- libgcc/pmu-profile.c (revision 190362) +++ libgcc/pmu-profile.c (working copy) @@ -1,3 +1,4 @@ + /* Performance monitoring unit (PMU) profiler. If available, use an external tool to collect hardware performance counter data and write it in the .gcda files. @@ -74,13 +75,13 @@ static void destroy_load_latency_infos (void *info static void destroy_branch_mispredict_infos (void *info); static gcov_unsigned_t gcov_tag_pmu_tool_header_length (gcov_pmu_tool_header_t *header); -static void gcov_write_tool_header (gcov_pmu_tool_header_t *header); -static void gcov_write_load_latency_infos (void *info); -static void gcov_write_branch_mispredict_infos (void *info); -static void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); -static void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t +void gcov_write_tool_header (gcov_pmu_tool_header_t *header); +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info); +void gcov_write_string_table_entry (const gcov_pmu_st_entry_t *st_entry); + /* Convert a fractional PCT to an unsigned integer after muliplying by 100. */ @@ -156,11 +157,11 @@ init_pmu_branch_mispredict (void) /* Write the load latency information LL_INFO into the gcda file. */ -static void +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_LOAD_LATENCY_LENGTH (ll_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, + GCOV_TAG_PMU_LOAD_LATENCY_LENGTH()); gcov_write_unsigned (ll_info->counts); gcov_write_unsigned (ll_info->self); gcov_write_unsigned (ll_info->cum); @@ -174,27 +175,38 @@ gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_i gcov_write_counter (ll_info->code_addr); gcov_write_unsigned (ll_info->line); gcov_write_unsigned (ll_info->discriminator); - gcov_write_string (ll_info->filename); + gcov_write_unsigned (ll_info->filetag); } /* Write the branch mispredict information BRM_INFO into the gcda file. */ -static void +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH ( - brm_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, + GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH()); gcov_write_unsigned (brm_info->counts); gcov_write_unsigned (brm_info->self); gcov_write_unsigned (brm_info->cum); gcov_write_counter (brm_info->code_addr); gcov_write_unsigned (brm_info->line); gcov_write_unsigned (brm_info->discriminator); - gcov_write_string (brm_info->filename); + gcov_write_unsigned (brm_info->filetag); } +/* Write the string table entry ST_ENTRY into the gcda file. */ + +void +gcov_write_string_table_entry (const gcov_pmu_st_entry_t *st_entry) +{ + gcov_write_tag_length (GCOV_TAG_PMU_STRING_TABLE_ENTRY, + GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH( + st_entry->filename)); + gcov_write_unsigned(st_entry->index); + gcov_write_string(st_entry->filename); +} + /* Compute TOOL_HEADER length for writing into the gcov file. */ static gcov_unsigned_t @@ -216,7 +228,7 @@ gcov_tag_pmu_tool_header_length (gcov_pmu_tool_hea /* Write tool header into the gcda file. It assumes that the gcda file has already been opened and is available for writing. */ -static void +void gcov_write_tool_header (gcov_pmu_tool_header_t *header) { gcov_unsigned_t len = gcov_tag_pmu_tool_header_length (header); Index: gcc/gcov.c =================================================================== --- gcc/gcov.c (revision 190359) +++ gcc/gcov.c (working copy) @@ -222,6 +222,7 @@ typedef struct pmu_data { ll_infos_t ll_infos; brm_infos_t brm_infos; + string_table_t string_table; } pmu_data_t; /* Describes a single line of source. Contains a chain of basic blocks @@ -975,6 +976,7 @@ release_structures (void) function_t *fn; ll_infos_t *ll_infos = &pmu_global_info.ll_infos; brm_infos_t *brm_infos = &pmu_global_info.brm_infos; + string_table_t *string_table = &pmu_global_info.string_table; for (ix = n_sources; ix--;) { @@ -1008,8 +1010,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < ll_infos->ll_count; ++i) { - if (ll_infos->ll_array[i]->filename) - XDELETE (ll_infos->ll_array[i]->filename); XDELETE (ll_infos->ll_array[i]); } /* delete the array itself */ @@ -1026,8 +1026,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < brm_infos->brm_count; ++i) { - if (brm_infos->brm_array[i]->filename) - XDELETE (brm_infos->brm_array[i]->filename); XDELETE (brm_infos->brm_array[i]); } /* delete the array itself */ @@ -1035,6 +1033,22 @@ release_structures (void) brm_infos->brm_array = NULL; brm_infos->brm_count = 0; } + + /* Cleanup PMU string table entries. */ + if (string_table->st_count) + { + unsigned i; + + /* delete each element */ + for (i = 0; i < string_table->st_count; ++i) + { + XDELETE (string_table->st_array[i]); + } + /* delete the array itself */ + XDELETE (string_table->st_array); + string_table->st_array = NULL; + string_table->st_count = 0; + } } /* Generate the names of the graph and data files. If OBJECT_DIRECTORY @@ -2263,17 +2277,21 @@ filter_pmu_data_lines (source_t *src) int changed; ll_infos_t *ll_infos; /* load latency information for this source */ brm_infos_t *brm_infos; /* branch mispredict information for this source */ + string_table_t *string_table; /* string table information for this source */ if (pmu_global_info.ll_infos.ll_count == 0 && - pmu_global_info.brm_infos.brm_count == 0) + pmu_global_info.brm_infos.brm_count == 0 && + pmu_global_info.string_table.st_count == 0) /* If there are no global entries, there is nothing to filter. */ return; src->pmu_data = XCNEW (pmu_data_t); ll_infos = &src->pmu_data->ll_infos; brm_infos = &src->pmu_data->brm_infos; + string_table = &src->pmu_data->string_table; ll_infos->pmu_tool_header = pmu_global_info.ll_infos.pmu_tool_header; brm_infos->pmu_tool_header = pmu_global_info.brm_infos.pmu_tool_header; + string_table->pmu_tool_header = pmu_global_info.string_table.pmu_tool_header; ll_infos->ll_array = 0; brm_infos->brm_array = 0; @@ -2282,7 +2300,8 @@ filter_pmu_data_lines (source_t *src) for (i = 0; i < pmu_global_info.ll_infos.ll_count; ++i) { gcov_pmu_ll_info_t *ll_info = pmu_global_info.ll_infos.ll_array[i]; - if (0 == strcmp (src->name, ll_info->filename)) + if (0 == strcmp (src->name, + string_table->st_array[ll_info->filetag - 1]->filename)) { if (!ll_infos->ll_array) { @@ -2295,9 +2314,9 @@ filter_pmu_data_lines (source_t *src) ll_infos->ll_count++; if (ll_infos->ll_count >= ll_infos->alloc_ll_count) { - /* need to realloc */ + /* need to realloc */ ll_infos->ll_array = (gcov_pmu_ll_info_t **) - xrealloc (ll_infos->ll_array, 2 * ll_infos->alloc_ll_count); + xrealloc (ll_infos->ll_array, 2 * ll_infos->alloc_ll_count); } ll_infos->ll_array[ll_infos->ll_count - 1] = ll_info; } @@ -2308,7 +2327,8 @@ filter_pmu_data_lines (source_t *src) for (i = 0; i < pmu_global_info.brm_infos.brm_count; ++i) { gcov_pmu_brm_info_t *brm_info = pmu_global_info.brm_infos.brm_array[i]; - if (0 == strcmp (src->name, brm_info->filename)) + if (0 == strcmp (src->name, + string_table->st_array[brm_info->filetag - 1]->filename)) { if (!brm_infos->brm_array) { @@ -2323,12 +2343,14 @@ filter_pmu_data_lines (source_t *src) { /* need to realloc */ brm_infos->brm_array = (gcov_pmu_brm_info_t **) - xrealloc (brm_infos->brm_array, 2 * brm_infos->alloc_brm_count); + xrealloc (brm_infos->brm_array, + 2 * brm_infos->alloc_brm_count); } brm_infos->brm_array[brm_infos->brm_count - 1] = brm_info; } } + /* Sort the load latency data according to the line numbers because we later iterate over sources in line number order. Normally we expect the PMU tool to provide sorted data, but a few entries can @@ -2914,6 +2936,7 @@ static void process_pmu_profile (void) int error = 0; ll_infos_t *ll_infos = &pmu_global_info.ll_infos; brm_infos_t *brm_infos = &pmu_global_info.brm_infos; + string_table_t *string_table = &pmu_global_info.string_table; /* Construct path for pmuprofile.gcda filename. */ create_file_names (pmu_profile_filename); @@ -2953,6 +2976,11 @@ static void process_pmu_profile (void) brm_infos->brm_array = XCNEWVEC (gcov_pmu_brm_info_t *, brm_infos->alloc_brm_count); + string_table->st_count = 0; + string_table->alloc_st_count = 64; + string_table->st_array = XCNEWVEC (gcov_pmu_st_entry_t *, + string_table->alloc_st_count); + while ((tag = gcov_read_unsigned ())) { unsigned length = gcov_read_unsigned (); @@ -2991,7 +3019,22 @@ static void process_pmu_profile (void) ll_infos->pmu_tool_header = tool_header; brm_infos->pmu_tool_header = tool_header; } + else if (tag == GCOV_TAG_PMU_STRING_TABLE_ENTRY) + { + gcov_pmu_st_entry_t *st_entry = XCNEW (gcov_pmu_st_entry_t); + gcov_read_pmu_string_table_entry(st_entry, length); + string_table->st_count++; + if (string_table->st_count >= string_table->alloc_st_count) + { + string_table->alloc_st_count *= 2; + string_table->st_array = (gcov_pmu_st_entry_t **) + xrealloc (string_table->st_array, + string_table->alloc_st_count); + } + string_table->st_array[string_table->st_count - 1] = st_entry; + } + gcov_sync (base, length); if ((error = gcov_is_error ())) { Index: gcc/gcov-io.c =================================================================== --- gcc/gcov-io.c (revision 190359) +++ gcc/gcov-io.c (working copy) @@ -242,7 +242,6 @@ GCOV_LINKAGE void gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_t *ll_info, gcov_unsigned_t len ATTRIBUTE_UNUSED) { - const char *filename; ll_info->counts = gcov_read_unsigned (); ll_info->self = gcov_read_unsigned (); ll_info->cum = gcov_read_unsigned (); @@ -256,11 +255,7 @@ gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_ ll_info->code_addr = gcov_read_counter (); ll_info->line = gcov_read_unsigned (); ll_info->discriminator = gcov_read_unsigned (); - filename = gcov_read_string (); - if (filename) - ll_info->filename = gcov_canonical_filename (xstrdup (filename)); - else - ll_info->filename = 0; + ll_info->filetag = gcov_read_unsigned (); } /* Read LEN words and construct branch mispredict info BRM_INFO. */ @@ -269,20 +264,29 @@ GCOV_LINKAGE void gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, gcov_unsigned_t len ATTRIBUTE_UNUSED) { - const char *filename; brm_info->counts = gcov_read_unsigned (); brm_info->self = gcov_read_unsigned (); brm_info->cum = gcov_read_unsigned (); brm_info->code_addr = gcov_read_counter (); brm_info->line = gcov_read_unsigned (); brm_info->discriminator = gcov_read_unsigned (); - filename = gcov_read_string (); - if (filename) - brm_info->filename = gcov_canonical_filename (xstrdup (filename)); - else - brm_info->filename = 0; + brm_info->filetag = gcov_read_unsigned (); } +/* Read LEN words from an open gcov file and construct data into a + string table entry */ + +GCOV_LINKAGE void +gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t *st_entry, + gcov_unsigned_t len ATTRIBUTE_UNUSED) +{ + const char *str; + + st_entry->index = gcov_read_unsigned (); + str = gcov_read_string (); + st_entry->filename = str ? gcov_canonical_filename (xstrdup(str)) : 0; +} + /* Read LEN words from an open gcov file and construct data into pmu tool header TOOL_HEADER. */ @@ -779,7 +783,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ if (!ll_info) return; fprintf (fp, " %u %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% " - "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", + "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", ll_info->counts, convert_unsigned_to_pct (ll_info->self), convert_unsigned_to_pct (ll_info->cum), @@ -791,7 +795,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ convert_unsigned_to_pct (ll_info->gt_1024), convert_unsigned_to_pct (ll_info->wself), ll_info->code_addr, - ll_info->filename, + ll_info->filetag, ll_info->line, ll_info->discriminator); if (newline == add_newline) @@ -807,18 +811,32 @@ print_branch_mispredict_line (FILE *fp, const gcov { if (!brm_info) return; - fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", + fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", brm_info->counts, convert_unsigned_to_pct (brm_info->self), convert_unsigned_to_pct (brm_info->cum), brm_info->code_addr, - brm_info->filename, + brm_info->filetag, brm_info->line, brm_info->discriminator); if (newline == add_newline) fprintf (fp, "\n"); } +/* Print STRING_TABLE_ENTRY into the file pointed by FP. NEWLINE specifies + whether or not to print a trailing newline. */ + +GCOV_LINKAGE void +print_pmu_string_table_entry (FILE *fp, const gcov_pmu_st_entry_t *st_entry, + const enum print_newline newline) +{ + if (!st_entry) + return; + fprintf (fp, " %d %s", st_entry->index, st_entry->filename); + if (newline == add_newline) + fprintf (fp, "\n"); +} + /* Print TOOL_HEADER into the file pointed by FP. NEWLINE specifies whether or not to print a trailing newline. */ Index: gcc/gcov-io.h =================================================================== --- gcc/gcov-io.h (revision 190359) +++ gcc/gcov-io.h (working copy) @@ -442,13 +442,14 @@ typedef HOST_WIDEST_INT gcov_type; #define GCOV_TAG_SUMMARY_LENGTH \ (1 + GCOV_COUNTERS_SUMMABLE * (3 + 3 * 2)) #define GCOV_TAG_PMU_LOAD_LATENCY_INFO ((gcov_unsigned_t)0xa5000000) -#define GCOV_TAG_PMU_LOAD_LATENCY_LENGTH(filename) \ - (gcov_string_length (filename) + 12 + 2) +#define GCOV_TAG_PMU_LOAD_LATENCY_LENGTH() (15) #define GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO ((gcov_unsigned_t)0xa7000000) -#define GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH(filename) \ - (gcov_string_length (filename) + 5 + 2) +#define GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH() (8) #define GCOV_TAG_PMU_TOOL_HEADER ((gcov_unsigned_t)0xa9000000) #define GCOV_TAG_MODULE_INFO ((gcov_unsigned_t)0xab000000) +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY ((gcov_unsigned_t)0xad000000) +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH(filename) \ + (gcov_string_length (filename) + 1) /* Counters that are collected. */ #define GCOV_COUNTER_ARCS 0 /* Arc transitions. */ @@ -635,7 +636,7 @@ typedef struct gcov_pmu_load_latency_info gcov_type code_addr; /* the actual miss address (pc+1 for Intel) */ gcov_unsigned_t line; /* line number corresponding to this miss */ gcov_unsigned_t discriminator; /* discriminator information for this miss */ - char *filename; /* filename corresponding to this miss */ + gcov_unsigned_t filetag; /* location in string table of filename */ } gcov_pmu_ll_info_t; /* This structure is used during runtime as well as in gcov. */ @@ -665,7 +666,7 @@ typedef struct gcov_pmu_branch_mispredict_info gcov_type code_addr; /* the actual mispredict address */ gcov_unsigned_t line; /* line number corresponding to this event */ gcov_unsigned_t discriminator; /* discriminator for this event */ - char *filename; /* filename corresponding to this event */ + gcov_unsigned_t filetag; /* location in string table of filename */ } gcov_pmu_brm_info_t; /* This structure is used during runtime as well as in gcov. */ @@ -682,6 +683,25 @@ typedef struct branch_mispredict_infos gcov_pmu_tool_header_t *pmu_tool_header; } brm_infos_t; +typedef struct gcov_pmu_string_table_entry +{ + gcov_unsigned_t index; /* The corresponding string table index */ + char* filename; /* The filename that belongs at this index */ +} gcov_pmu_st_entry_t; + +typedef struct string_table +{ + /* An array describing the total number of string table entries. */ + gcov_pmu_st_entry_t **st_array; + /* The total number of entries in the above array. */ + unsigned st_count; + /* The total number of entries currently allocated in the array. + Used for bookkeeping. */ + unsigned alloc_st_count; + /* PMU tool header */ + gcov_pmu_tool_header_t *pmu_tool_header; +} string_table_t; + /* Structures embedded in coveraged program. The structures generated by write_profile must match these. */ @@ -871,6 +891,10 @@ GCOV_LINKAGE void gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, gcov_unsigned_t len) ATTRIBUTE_HIDDEN; GCOV_LINKAGE void +gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t *entry, + gcov_unsigned_t len) ATTRIBUTE_HIDDEN; + +GCOV_LINKAGE void gcov_read_pmu_tool_header (gcov_pmu_tool_header_t *tool_header, gcov_unsigned_t len) ATTRIBUTE_HIDDEN; #ifndef __GCOV_KERNEL__ @@ -887,6 +911,9 @@ GCOV_LINKAGE void print_load_latency_line (FILE *f GCOV_LINKAGE void print_branch_mispredict_line (FILE *fp, const gcov_pmu_brm_info_t *brm_info, const enum print_newline); +GCOV_LINKAGE void +print_pmu_string_table_entry (FILE* fp, const gcov_pmu_st_entry_t *entry, + const enum print_newline); GCOV_LINKAGE void print_pmu_tool_header (FILE *fp, gcov_pmu_tool_header_t *tool_header, const enum print_newline); Index: gcc/gcov-dump.c =================================================================== --- gcc/gcov-dump.c (revision 190359) +++ gcc/gcov-dump.c (working copy) @@ -43,6 +43,7 @@ static void tag_summary (const char *, unsigned, u static void tag_module_info (const char *, unsigned, unsigned); static void tag_pmu_load_latency_info (const char *, unsigned, unsigned); static void tag_pmu_branch_mispredict_info (const char *, unsigned, unsigned); +static void tag_pmu_string_table_entry (const char*, unsigned, unsigned); static void tag_pmu_tool_header (const char *, unsigned, unsigned); extern int main (int, char **); @@ -84,6 +85,8 @@ static const tag_format_t tag_table[] = {GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, "PMU_BRANCH_MISPREDICT_INFO", tag_pmu_branch_mispredict_info}, {GCOV_TAG_PMU_TOOL_HEADER, "PMU_TOOL_HEADER", tag_pmu_tool_header}, + {GCOV_TAG_PMU_STRING_TABLE_ENTRY, "PMU_STRING_TABLE_ENTRY", + tag_pmu_string_table_entry}, {0, NULL, NULL} }; @@ -573,7 +576,6 @@ tag_pmu_load_latency_info (const char *filename AT gcov_pmu_ll_info_t ll_info; gcov_read_pmu_load_latency_info (&ll_info, length); print_load_latency_line (stdout, &ll_info, no_newline); - free (ll_info.filename); } /* Read gcov tag GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO from the gcda @@ -586,9 +588,17 @@ tag_pmu_branch_mispredict_info (const char *filena gcov_pmu_brm_info_t brm_info; gcov_read_pmu_branch_mispredict_info (&brm_info, length); print_branch_mispredict_line (stdout, &brm_info, no_newline); - free (brm_info.filename); } +static void +tag_pmu_string_table_entry (const char *filename ATTRIBUTE_UNUSED, + unsigned tag ATTRIBUTE_UNUSED, unsigned length) +{ + gcov_pmu_st_entry_t st_entry; + gcov_read_pmu_string_table_entry(&st_entry, length); + print_pmu_string_table_entry(stdout, &st_entry, no_newline); + free(st_entry.filename); +} /* Read gcov tag GCOV_TAG_PMU_TOOL_HEADER from the gcda file and print the contents in a human readable form. */ -- This patch is available for review at http://codereview.appspot.com/6427063
Sign in to reply to this message.
On 2012/08/24 21:51:24, cmang wrote: > Fixed formatting issues. The gcov.c is still not uploaded properly. > =================================================================== > --- gcc/gcov.c (revision 190359) > +++ gcc/gcov.c (working copy) > @@ -222,6 +222,7 @@ typedef struct pmu_data > { > ll_infos_t ll_infos; > brm_infos_t brm_infos; > + string_table_t string_table; > } pmu_data_t; > > /* Describes a single line of source. Contains a chain of basic blocks > @@ -975,6 +976,7 @@ release_structures (void) > function_t *fn; > ll_infos_t *ll_infos = &pmu_global_info.ll_infos; > brm_infos_t *brm_infos = &pmu_global_info.brm_infos; > + string_table_t *string_table = &pmu_global_info.string_table; > > for (ix = n_sources; ix--;) > { > @@ -1008,8 +1010,6 @@ release_structures (void) > /* delete each element */ > for (i = 0; i < ll_infos->ll_count; ++i) > { > - if (ll_infos->ll_array[i]->filename) > - XDELETE (ll_infos->ll_array[i]->filename); > XDELETE (ll_infos->ll_array[i]); > } > /* delete the array itself */ > @@ -1026,8 +1026,6 @@ release_structures (void) > /* delete each element */ > for (i = 0; i < brm_infos->brm_count; ++i) > { > - if (brm_infos->brm_array[i]->filename) > - XDELETE (brm_infos->brm_array[i]->filename); > XDELETE (brm_infos->brm_array[i]); > } > /* delete the array itself */ > @@ -1035,6 +1033,22 @@ release_structures (void) > brm_infos->brm_array = NULL; > brm_infos->brm_count = 0; > } > + > + /* Cleanup PMU string table entries. */ > + if (string_table->st_count) > + { > + unsigned i; > + > + /* delete each element */ > + for (i = 0; i < string_table->st_count; ++i) > + { > + XDELETE (string_table->st_array[i]); > + } Remove {}.
Sign in to reply to this message.
http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h File gcc/gcov-io.h (right): http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding string table index */ Is this field necessary? http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 gcc/gcov-io.h:689: char* filename; /* The filename that belongs at this index */ Can this field name be generalized? http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c File libgcc/pmu-profile.c (right): http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 libgcc/pmu-profile.c:83: Who is the caller to this interface? It should be declared in gcov-io.h
Sign in to reply to this message.
http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h File gcc/gcov-io.h (right): http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding string table index */ On 2012/08/24 22:30:30, davidxl wrote: > Is this field necessary? For the purposes of gcov_dump output we wanted to output not only the string table entry but also its index in the string table just in case there were any problems with ordering that might map filetags to the wrong filename. Because of this and the fact that gcov.c and gcov-dump.c read one entry at a time, it seemed better to have the index be self-contained within the struct to avoid extra logic. Also, since the string table entry has to be written with its corresponding index for gcov-dump, the gcov_write_string_table_entry function could have a similar syntax to the ll/brm writing functions. http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 gcc/gcov-io.h:689: char* filename; /* The filename that belongs at this index */ On 2012/08/24 22:30:30, davidxl wrote: > Can this field name be generalized? Generalized in what way? I used this name because it was used before in the old gcda format. http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c File libgcc/pmu-profile.c (right): http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 libgcc/pmu-profile.c:83: On 2012/08/24 22:30:30, davidxl wrote: > Who is the caller to this interface? > > It should be declared in gcov-io.h The only current caller of this is https://critique.corp.google.com/#review/31972005. This is also the same for the other two ll/brm writing functions. Should I declare those in gcov-io.h as well?
Sign in to reply to this message.
On Fri, Aug 24, 2012 at 3:56 PM, <cmang@google.com> wrote: > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h > File gcc/gcov-io.h (right): > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 > gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding string > table index */ > On 2012/08/24 22:30:30, davidxl wrote: >> >> Is this field necessary? > > > For the purposes of gcov_dump output we wanted to output not only the > string table entry but also its index in the string table just in case > there were any problems with ordering that might map filetags to the > wrong filename. Because of this and the fact that gcov.c and gcov-dump.c > read one entry at a time, it seemed better to have the index be > self-contained within the struct to avoid extra logic. That is fine -- but you probably need to add assertion check when the string table is read in. > > Also, since the string table entry has to be written with its > corresponding index for gcov-dump, the gcov_write_string_table_entry > function could have a similar syntax to the ll/brm writing functions. > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 > gcc/gcov-io.h:689: char* filename; /* The filename that belongs > at this index */ > On 2012/08/24 22:30:30, davidxl wrote: >> >> Can this field name be generalized? > > > Generalized in what way? I used this name because it was used before in > the old gcda format. Since it is string table, it can be something like str_ or strval_ etc. > > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c > File libgcc/pmu-profile.c (right): > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 > libgcc/pmu-profile.c:83: > On 2012/08/24 22:30:30, davidxl wrote: >> >> Who is the caller to this interface? > > >> It should be declared in gcov-io.h > > > The only current caller of this is > https://critique.corp.google.com/#review/31972005. This is also the same > for the other two ll/brm writing functions. Should I declare those in > gcov-io.h as well? Any utilities functions that may be be used by clients other than gcov tool should be in the common header. David > > http://codereview.appspot.com/6427063/
Sign in to reply to this message.
On 2012/08/24 23:20:21, davidxl wrote: > On Fri, Aug 24, 2012 at 3:56 PM, <mailto:cmang@google.com> wrote: > > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h > > File gcc/gcov-io.h (right): > > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 > > gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding string > > table index */ > > On 2012/08/24 22:30:30, davidxl wrote: > >> > >> Is this field necessary? > > > > > > For the purposes of gcov_dump output we wanted to output not only the > > string table entry but also its index in the string table just in case > > there were any problems with ordering that might map filetags to the > > wrong filename. Because of this and the fact that gcov.c and gcov-dump.c > > read one entry at a time, it seemed better to have the index be > > self-contained within the struct to avoid extra logic. > > > That is fine -- but you probably need to add assertion check when the > string table is read in. > > > > > Also, since the string table entry has to be written with its > > corresponding index for gcov-dump, the gcov_write_string_table_entry > > function could have a similar syntax to the ll/brm writing functions. > > > > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 > > gcc/gcov-io.h:689: char* filename; /* The filename that belongs > > at this index */ > > On 2012/08/24 22:30:30, davidxl wrote: > >> > >> Can this field name be generalized? > > > > > > Generalized in what way? I used this name because it was used before in > > the old gcda format. > > Since it is string table, it can be something like str_ or strval_ etc. > Done. filename will be changed to str in an upcoming patch. > > > > > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c > > File libgcc/pmu-profile.c (right): > > > > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 > > libgcc/pmu-profile.c:83: > > On 2012/08/24 22:30:30, davidxl wrote: > >> > >> Who is the caller to this interface? > > > > > >> It should be declared in gcov-io.h > > > > > > The only current caller of this is > > https://critique.corp.google.com/#review/31972005. This is also the same > > for the other two ll/brm writing functions. Should I declare those in > > gcov-io.h as well? > > Any utilities functions that may be be used by clients other than gcov > tool should be in the common header. > However, I don't really know how to answer this. It seems that if I move the mentioned utility functions to gcov-io.h, I should also move their helper functions, like convert_unsigned_to_pct, to gcov-io.h as well and completely remove pmu-profile.c. Does this seem like a reasonable solution? > David > > > > > http://codereview.appspot.com/6427063/
Sign in to reply to this message.
On Mon, Aug 27, 2012 at 10:55 AM, <cmang@google.com> wrote: > On 2012/08/24 23:20:21, davidxl wrote: >> >> On Fri, Aug 24, 2012 at 3:56 PM, <mailto:cmang@google.com> wrote: >> > >> > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h >> > File gcc/gcov-io.h (right): >> > >> > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 >> >> > gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding > > string >> >> > table index */ >> > On 2012/08/24 22:30:30, davidxl wrote: >> >> >> >> Is this field necessary? >> > >> > >> > For the purposes of gcov_dump output we wanted to output not only > > the >> >> > string table entry but also its index in the string table just in > > case >> >> > there were any problems with ordering that might map filetags to the >> > wrong filename. Because of this and the fact that gcov.c and > > gcov-dump.c >> >> > read one entry at a time, it seemed better to have the index be >> > self-contained within the struct to avoid extra logic. > > > >> That is fine -- but you probably need to add assertion check when the >> string table is read in. > > >> > >> > Also, since the string table entry has to be written with its >> > corresponding index for gcov-dump, the gcov_write_string_table_entry >> > function could have a similar syntax to the ll/brm writing > > functions. >> >> > >> > >> > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 >> >> > gcc/gcov-io.h:689: char* filename; /* The filename that > > belongs >> >> > at this index */ >> > On 2012/08/24 22:30:30, davidxl wrote: >> >> >> >> Can this field name be generalized? >> > >> > >> > Generalized in what way? I used this name because it was used before > > in >> >> > the old gcda format. > > >> Since it is string table, it can be something like str_ or strval_ > > etc. > > > Done. filename will be changed to str in an upcoming patch. > > >> > >> > >> > > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c >> >> > File libgcc/pmu-profile.c (right): >> > >> > > > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 >> >> > libgcc/pmu-profile.c:83: >> > On 2012/08/24 22:30:30, davidxl wrote: >> >> >> >> Who is the caller to this interface? >> > >> > >> >> It should be declared in gcov-io.h >> > >> > >> > The only current caller of this is >> > https://critique.corp.google.com/#review/31972005. This is also the > > same >> >> > for the other two ll/brm writing functions. Should I declare those > > in >> >> > gcov-io.h as well? > > >> Any utilities functions that may be be used by clients other than gcov >> tool should be in the common header. > > > > However, I don't really know how to answer this. It seems that if I move > the mentioned utility functions to gcov-io.h, I should also move their > helper functions, like convert_unsigned_to_pct, to gcov-io.h as well and > completely remove pmu-profile.c. Does this seem like a reasonable > solution? I think the idea is to put the declarations into gcov-io.h since they are no longer static. But you could leave the implementation in pmu-profile.c Teresa > >> David > > >> > >> > http://codereview.appspot.com/6427063/ > > > > > http://codereview.appspot.com/6427063/ -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Sign in to reply to this message.
Oh, I see. I misunderstood. I'll make those changes and send a patch. On Mon, Aug 27, 2012 at 10:59 AM, Teresa Johnson <tejohnson@google.com>wrote: > > On Mon, Aug 27, 2012 at 10:55 AM, <cmang@google.com> wrote: > > On 2012/08/24 23:20:21, davidxl wrote: > >> > >> On Fri, Aug 24, 2012 at 3:56 PM, <mailto:cmang@google.com> wrote: > >> > > >> > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h > >> > File gcc/gcov-io.h (right): > >> > > >> > > > > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 > >> > >> > gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding > > > > string > >> > >> > table index */ > >> > On 2012/08/24 22:30:30, davidxl wrote: > >> >> > >> >> Is this field necessary? > >> > > >> > > >> > For the purposes of gcov_dump output we wanted to output not only > > > > the > >> > >> > string table entry but also its index in the string table just in > > > > case > >> > >> > there were any problems with ordering that might map filetags to the > >> > wrong filename. Because of this and the fact that gcov.c and > > > > gcov-dump.c > >> > >> > read one entry at a time, it seemed better to have the index be > >> > self-contained within the struct to avoid extra logic. > > > > > > > >> That is fine -- but you probably need to add assertion check when the > >> string table is read in. > > > > > >> > > >> > Also, since the string table entry has to be written with its > >> > corresponding index for gcov-dump, the gcov_write_string_table_entry > >> > function could have a similar syntax to the ll/brm writing > > > > functions. > >> > >> > > >> > > >> > > > > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 > >> > >> > gcc/gcov-io.h:689: char* filename; /* The filename that > > > > belongs > >> > >> > at this index */ > >> > On 2012/08/24 22:30:30, davidxl wrote: > >> >> > >> >> Can this field name be generalized? > >> > > >> > > >> > Generalized in what way? I used this name because it was used before > > > > in > >> > >> > the old gcda format. > > > > > >> Since it is string table, it can be something like str_ or strval_ > > > > etc. > > > > > > Done. filename will be changed to str in an upcoming patch. > > > > > >> > > >> > > >> > > > > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c > >> > >> > File libgcc/pmu-profile.c (right): > >> > > >> > > > > > > > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 > >> > >> > libgcc/pmu-profile.c:83: > >> > On 2012/08/24 22:30:30, davidxl wrote: > >> >> > >> >> Who is the caller to this interface? > >> > > >> > > >> >> It should be declared in gcov-io.h > >> > > >> > > >> > The only current caller of this is > >> > https://critique.corp.google.com/#review/31972005. This is also the > > > > same > >> > >> > for the other two ll/brm writing functions. Should I declare those > > > > in > >> > >> > gcov-io.h as well? > > > > > >> Any utilities functions that may be be used by clients other than gcov > >> tool should be in the common header. > > > > > > > > However, I don't really know how to answer this. It seems that if I move > > the mentioned utility functions to gcov-io.h, I should also move their > > helper functions, like convert_unsigned_to_pct, to gcov-io.h as well and > > completely remove pmu-profile.c. Does this seem like a reasonable > > solution? > > I think the idea is to put the declarations into gcov-io.h since they > are no longer static. But you could leave the implementation in > pmu-profile.c > > Teresa > > > > >> David > > > > > >> > > >> > http://codereview.appspot.com/6427063/ > > > > > > > > > > http://codereview.appspot.com/6427063/ > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 >
Sign in to reply to this message.
Moved pmu writing utilities to global header. The patch should be applied to google/main Tested with crosstools. 2012-08-27 Chris Manghane <cmang@google.com> * libgcc/pmu-profile.c (gcov_write_load_latency_infos): Removed unused function. (gcov_write_branch_mispredict_infos): Ditto. (destroy_load_latency_infos): Removed static keyword. (init_pmu_branch_mispredict): Ditto. (gcov_write_ll_line): Ditto, plus replaced filename field with filetag. (gcov_write_branch_mispredict_line): Ditto. (gcov_write_string_table_entry): New function. (gcov_write_tool_header): Removed static keyword. * gcc/gcov.c (release_structures): Removed filename field from PMU structures. (filter_pmu_data_lines): Added PMU string table support. (process_pmu_profile): Ditto. * gcc/gcov-io.c (gcov_read_pmu_load_latency_info): Replaced filename field with filetag. (gcov_read_pmu_branch_mispredict_info): Ditto. (gcov_read_pmu_string_table_entry): New Function. (print_load_latency_line): Replaced filename field with filetag. (print_branch_mispredict_line): Ditto. (print_string_table_entry): New function. * gcc/gcov-io.h (GCOV_TAG_PMU_LOAD_LATENCY_LENGTH): Replaced filename field with filetag (GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH): Ditto. (GCOV_TAG_PMU_STRING_TABLE_ENTRY): Added new tag. (GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH): Ditto. (gcov_pmu_load_latency_info): Replaced filename field with filetag. (gcov_pmu_branch_mispredict_info): Ditto. (gcov_pmu_string_table_entry): New struct. (gcov_pmu_string_table): New struct. (gcov_write_ll_line): Moved pmu writing utilities to global header. (gcov_write_branch_mispredict_line): Ditto. (gcov_write_string_table_entry): Ditto. (gcov_write_tool_header): Ditto. * gcc/gcov-dump.c (tag_pmu_load_latency_info): Removed PMU filename field. (tag_pmu_branch_mispredict_info): Ditto. (tag_pmu_string_table_entry): New function. Index: libgcc/pmu-profile.c =================================================================== --- libgcc/pmu-profile.c (revision 190362) +++ libgcc/pmu-profile.c (working copy) @@ -1,3 +1,4 @@ + /* Performance monitoring unit (PMU) profiler. If available, use an external tool to collect hardware performance counter data and write it in the .gcda files. @@ -74,12 +75,6 @@ static void destroy_load_latency_infos (void *info static void destroy_branch_mispredict_infos (void *info); static gcov_unsigned_t gcov_tag_pmu_tool_header_length (gcov_pmu_tool_header_t *header); -static void gcov_write_tool_header (gcov_pmu_tool_header_t *header); -static void gcov_write_load_latency_infos (void *info); -static void gcov_write_branch_mispredict_infos (void *info); -static void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); -static void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t - *brm_info); /* Convert a fractional PCT to an unsigned integer after muliplying by 100. */ @@ -156,11 +151,11 @@ init_pmu_branch_mispredict (void) /* Write the load latency information LL_INFO into the gcda file. */ -static void +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_LOAD_LATENCY_LENGTH (ll_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, + GCOV_TAG_PMU_LOAD_LATENCY_LENGTH()); gcov_write_unsigned (ll_info->counts); gcov_write_unsigned (ll_info->self); gcov_write_unsigned (ll_info->cum); @@ -174,27 +169,38 @@ gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_i gcov_write_counter (ll_info->code_addr); gcov_write_unsigned (ll_info->line); gcov_write_unsigned (ll_info->discriminator); - gcov_write_string (ll_info->filename); + gcov_write_unsigned (ll_info->filetag); } /* Write the branch mispredict information BRM_INFO into the gcda file. */ -static void +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH ( - brm_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, + GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH()); gcov_write_unsigned (brm_info->counts); gcov_write_unsigned (brm_info->self); gcov_write_unsigned (brm_info->cum); gcov_write_counter (brm_info->code_addr); gcov_write_unsigned (brm_info->line); gcov_write_unsigned (brm_info->discriminator); - gcov_write_string (brm_info->filename); + gcov_write_unsigned (brm_info->filetag); } +/* Write the string table entry ST_ENTRY into the gcda file. */ + +void +gcov_write_string_table_entry (const gcov_pmu_st_entry_t *st_entry) +{ + gcov_write_tag_length (GCOV_TAG_PMU_STRING_TABLE_ENTRY, + GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH( + st_entry->str)); + gcov_write_unsigned(st_entry->index); + gcov_write_string(st_entry->str); +} + /* Compute TOOL_HEADER length for writing into the gcov file. */ static gcov_unsigned_t @@ -216,7 +222,7 @@ gcov_tag_pmu_tool_header_length (gcov_pmu_tool_hea /* Write tool header into the gcda file. It assumes that the gcda file has already been opened and is available for writing. */ -static void +void gcov_write_tool_header (gcov_pmu_tool_header_t *header) { gcov_unsigned_t len = gcov_tag_pmu_tool_header_length (header); Index: gcc/gcov.c =================================================================== --- gcc/gcov.c (revision 190359) +++ gcc/gcov.c (working copy) @@ -222,6 +222,7 @@ typedef struct pmu_data { ll_infos_t ll_infos; brm_infos_t brm_infos; + string_table_t string_table; } pmu_data_t; /* Describes a single line of source. Contains a chain of basic blocks @@ -975,6 +976,7 @@ release_structures (void) function_t *fn; ll_infos_t *ll_infos = &pmu_global_info.ll_infos; brm_infos_t *brm_infos = &pmu_global_info.brm_infos; + string_table_t *string_table = &pmu_global_info.string_table; for (ix = n_sources; ix--;) { @@ -1008,8 +1010,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < ll_infos->ll_count; ++i) { - if (ll_infos->ll_array[i]->filename) - XDELETE (ll_infos->ll_array[i]->filename); XDELETE (ll_infos->ll_array[i]); } /* delete the array itself */ @@ -1026,8 +1026,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < brm_infos->brm_count; ++i) { - if (brm_infos->brm_array[i]->filename) - XDELETE (brm_infos->brm_array[i]->filename); XDELETE (brm_infos->brm_array[i]); } /* delete the array itself */ @@ -1035,6 +1033,22 @@ release_structures (void) brm_infos->brm_array = NULL; brm_infos->brm_count = 0; } + + /* Cleanup PMU string table entries. */ + if (string_table->st_count) + { + unsigned i; + + /* delete each element */ + for (i = 0; i < string_table->st_count; ++i) + { + XDELETE (string_table->st_array[i]); + } + /* delete the array itself */ + XDELETE (string_table->st_array); + string_table->st_array = NULL; + string_table->st_count = 0; + } } /* Generate the names of the graph and data files. If OBJECT_DIRECTORY @@ -2263,17 +2277,21 @@ filter_pmu_data_lines (source_t *src) int changed; ll_infos_t *ll_infos; /* load latency information for this source */ brm_infos_t *brm_infos; /* branch mispredict information for this source */ + string_table_t *string_table; /* string table information for this source */ if (pmu_global_info.ll_infos.ll_count == 0 && - pmu_global_info.brm_infos.brm_count == 0) + pmu_global_info.brm_infos.brm_count == 0 && + pmu_global_info.string_table.st_count == 0) /* If there are no global entries, there is nothing to filter. */ return; src->pmu_data = XCNEW (pmu_data_t); ll_infos = &src->pmu_data->ll_infos; brm_infos = &src->pmu_data->brm_infos; + string_table = &src->pmu_data->string_table; ll_infos->pmu_tool_header = pmu_global_info.ll_infos.pmu_tool_header; brm_infos->pmu_tool_header = pmu_global_info.brm_infos.pmu_tool_header; + string_table->pmu_tool_header = pmu_global_info.string_table.pmu_tool_header; ll_infos->ll_array = 0; brm_infos->brm_array = 0; @@ -2282,7 +2300,8 @@ filter_pmu_data_lines (source_t *src) for (i = 0; i < pmu_global_info.ll_infos.ll_count; ++i) { gcov_pmu_ll_info_t *ll_info = pmu_global_info.ll_infos.ll_array[i]; - if (0 == strcmp (src->name, ll_info->filename)) + if (0 == strcmp (src->name, + string_table->st_array[ll_info->filetag - 1]->str)) { if (!ll_infos->ll_array) { @@ -2295,9 +2314,9 @@ filter_pmu_data_lines (source_t *src) ll_infos->ll_count++; if (ll_infos->ll_count >= ll_infos->alloc_ll_count) { - /* need to realloc */ + /* need to realloc */ ll_infos->ll_array = (gcov_pmu_ll_info_t **) - xrealloc (ll_infos->ll_array, 2 * ll_infos->alloc_ll_count); + xrealloc (ll_infos->ll_array, 2 * ll_infos->alloc_ll_count); } ll_infos->ll_array[ll_infos->ll_count - 1] = ll_info; } @@ -2308,7 +2327,8 @@ filter_pmu_data_lines (source_t *src) for (i = 0; i < pmu_global_info.brm_infos.brm_count; ++i) { gcov_pmu_brm_info_t *brm_info = pmu_global_info.brm_infos.brm_array[i]; - if (0 == strcmp (src->name, brm_info->filename)) + if (0 == strcmp (src->name, + string_table->st_array[brm_info->filetag - 1]->str)) { if (!brm_infos->brm_array) { @@ -2323,12 +2343,14 @@ filter_pmu_data_lines (source_t *src) { /* need to realloc */ brm_infos->brm_array = (gcov_pmu_brm_info_t **) - xrealloc (brm_infos->brm_array, 2 * brm_infos->alloc_brm_count); + xrealloc (brm_infos->brm_array, + 2 * brm_infos->alloc_brm_count); } brm_infos->brm_array[brm_infos->brm_count - 1] = brm_info; } } + /* Sort the load latency data according to the line numbers because we later iterate over sources in line number order. Normally we expect the PMU tool to provide sorted data, but a few entries can @@ -2914,6 +2936,7 @@ static void process_pmu_profile (void) int error = 0; ll_infos_t *ll_infos = &pmu_global_info.ll_infos; brm_infos_t *brm_infos = &pmu_global_info.brm_infos; + string_table_t *string_table = &pmu_global_info.string_table; /* Construct path for pmuprofile.gcda filename. */ create_file_names (pmu_profile_filename); @@ -2953,6 +2976,11 @@ static void process_pmu_profile (void) brm_infos->brm_array = XCNEWVEC (gcov_pmu_brm_info_t *, brm_infos->alloc_brm_count); + string_table->st_count = 0; + string_table->alloc_st_count = 64; + string_table->st_array = XCNEWVEC (gcov_pmu_st_entry_t *, + string_table->alloc_st_count); + while ((tag = gcov_read_unsigned ())) { unsigned length = gcov_read_unsigned (); @@ -2991,7 +3019,24 @@ static void process_pmu_profile (void) ll_infos->pmu_tool_header = tool_header; brm_infos->pmu_tool_header = tool_header; } + else if (tag == GCOV_TAG_PMU_STRING_TABLE_ENTRY) + { + gcov_pmu_st_entry_t *st_entry = XCNEW (gcov_pmu_st_entry_t); + gcov_read_pmu_string_table_entry (st_entry, length); + /* Verify that we read string table entries in the right order */ + gcc_assert (st_entry->index == string_table->st_count); + string_table->st_count++; + if (string_table->st_count >= string_table->alloc_st_count) + { + string_table->alloc_st_count *= 2; + string_table->st_array = (gcov_pmu_st_entry_t **) + xrealloc (string_table->st_array, + string_table->alloc_st_count); + } + string_table->st_array[string_table->st_count - 1] = st_entry; + } + gcov_sync (base, length); if ((error = gcov_is_error ())) { Index: gcc/gcov-io.c =================================================================== --- gcc/gcov-io.c (revision 190359) +++ gcc/gcov-io.c (working copy) @@ -242,7 +242,6 @@ GCOV_LINKAGE void gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_t *ll_info, gcov_unsigned_t len ATTRIBUTE_UNUSED) { - const char *filename; ll_info->counts = gcov_read_unsigned (); ll_info->self = gcov_read_unsigned (); ll_info->cum = gcov_read_unsigned (); @@ -256,11 +255,7 @@ gcov_read_pmu_load_latency_info (gcov_pmu_ll_info_ ll_info->code_addr = gcov_read_counter (); ll_info->line = gcov_read_unsigned (); ll_info->discriminator = gcov_read_unsigned (); - filename = gcov_read_string (); - if (filename) - ll_info->filename = gcov_canonical_filename (xstrdup (filename)); - else - ll_info->filename = 0; + ll_info->filetag = gcov_read_unsigned (); } /* Read LEN words and construct branch mispredict info BRM_INFO. */ @@ -269,20 +264,29 @@ GCOV_LINKAGE void gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, gcov_unsigned_t len ATTRIBUTE_UNUSED) { - const char *filename; brm_info->counts = gcov_read_unsigned (); brm_info->self = gcov_read_unsigned (); brm_info->cum = gcov_read_unsigned (); brm_info->code_addr = gcov_read_counter (); brm_info->line = gcov_read_unsigned (); brm_info->discriminator = gcov_read_unsigned (); - filename = gcov_read_string (); - if (filename) - brm_info->filename = gcov_canonical_filename (xstrdup (filename)); - else - brm_info->filename = 0; + brm_info->filetag = gcov_read_unsigned (); } +/* Read LEN words from an open gcov file and construct data into a + string table entry */ + +GCOV_LINKAGE void +gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t *st_entry, + gcov_unsigned_t len ATTRIBUTE_UNUSED) +{ + const char *str; + + st_entry->index = gcov_read_unsigned (); + str = gcov_read_string (); + st_entry->str = str ? gcov_canonical_filename (xstrdup(str)) : 0; +} + /* Read LEN words from an open gcov file and construct data into pmu tool header TOOL_HEADER. */ @@ -779,7 +783,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ if (!ll_info) return; fprintf (fp, " %u %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% %.2f%% " - "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", + "%.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", ll_info->counts, convert_unsigned_to_pct (ll_info->self), convert_unsigned_to_pct (ll_info->cum), @@ -791,7 +795,7 @@ print_load_latency_line (FILE *fp, const gcov_pmu_ convert_unsigned_to_pct (ll_info->gt_1024), convert_unsigned_to_pct (ll_info->wself), ll_info->code_addr, - ll_info->filename, + ll_info->filetag, ll_info->line, ll_info->discriminator); if (newline == add_newline) @@ -807,18 +811,32 @@ print_branch_mispredict_line (FILE *fp, const gcov { if (!brm_info) return; - fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %s %d %d", + fprintf (fp, " %u %.2f%% %.2f%% " HOST_WIDEST_INT_PRINT_HEX " %d %d %d", brm_info->counts, convert_unsigned_to_pct (brm_info->self), convert_unsigned_to_pct (brm_info->cum), brm_info->code_addr, - brm_info->filename, + brm_info->filetag, brm_info->line, brm_info->discriminator); if (newline == add_newline) fprintf (fp, "\n"); } +/* Print STRING_TABLE_ENTRY into the file pointed by FP. NEWLINE specifies + whether or not to print a trailing newline. */ + +GCOV_LINKAGE void +print_pmu_string_table_entry (FILE *fp, const gcov_pmu_st_entry_t *st_entry, + const enum print_newline newline) +{ + if (!st_entry) + return; + fprintf (fp, " %d %s", st_entry->index, st_entry->str); + if (newline == add_newline) + fprintf (fp, "\n"); +} + /* Print TOOL_HEADER into the file pointed by FP. NEWLINE specifies whether or not to print a trailing newline. */ Index: gcc/gcov-io.h =================================================================== --- gcc/gcov-io.h (revision 190359) +++ gcc/gcov-io.h (working copy) @@ -442,13 +442,14 @@ typedef HOST_WIDEST_INT gcov_type; #define GCOV_TAG_SUMMARY_LENGTH \ (1 + GCOV_COUNTERS_SUMMABLE * (3 + 3 * 2)) #define GCOV_TAG_PMU_LOAD_LATENCY_INFO ((gcov_unsigned_t)0xa5000000) -#define GCOV_TAG_PMU_LOAD_LATENCY_LENGTH(filename) \ - (gcov_string_length (filename) + 12 + 2) +#define GCOV_TAG_PMU_LOAD_LATENCY_LENGTH() (15) #define GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO ((gcov_unsigned_t)0xa7000000) -#define GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH(filename) \ - (gcov_string_length (filename) + 5 + 2) +#define GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH() (8) #define GCOV_TAG_PMU_TOOL_HEADER ((gcov_unsigned_t)0xa9000000) #define GCOV_TAG_MODULE_INFO ((gcov_unsigned_t)0xab000000) +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY ((gcov_unsigned_t)0xad000000) +#define GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH(filename) \ + (gcov_string_length (filename) + 1) /* Counters that are collected. */ #define GCOV_COUNTER_ARCS 0 /* Arc transitions. */ @@ -635,7 +636,7 @@ typedef struct gcov_pmu_load_latency_info gcov_type code_addr; /* the actual miss address (pc+1 for Intel) */ gcov_unsigned_t line; /* line number corresponding to this miss */ gcov_unsigned_t discriminator; /* discriminator information for this miss */ - char *filename; /* filename corresponding to this miss */ + gcov_unsigned_t filetag; /* location in string table of filename */ } gcov_pmu_ll_info_t; /* This structure is used during runtime as well as in gcov. */ @@ -665,7 +666,7 @@ typedef struct gcov_pmu_branch_mispredict_info gcov_type code_addr; /* the actual mispredict address */ gcov_unsigned_t line; /* line number corresponding to this event */ gcov_unsigned_t discriminator; /* discriminator for this event */ - char *filename; /* filename corresponding to this event */ + gcov_unsigned_t filetag; /* location in string table of filename */ } gcov_pmu_brm_info_t; /* This structure is used during runtime as well as in gcov. */ @@ -682,6 +683,25 @@ typedef struct branch_mispredict_infos gcov_pmu_tool_header_t *pmu_tool_header; } brm_infos_t; +typedef struct gcov_pmu_string_table_entry +{ + gcov_unsigned_t index; /* The corresponding string table index */ + char* str; /* The string that belongs at this index */ +} gcov_pmu_st_entry_t; + +typedef struct string_table +{ + /* An array describing the total number of string table entries. */ + gcov_pmu_st_entry_t **st_array; + /* The total number of entries in the above array. */ + unsigned st_count; + /* The total number of entries currently allocated in the array. + Used for bookkeeping. */ + unsigned alloc_st_count; + /* PMU tool header */ + gcov_pmu_tool_header_t *pmu_tool_header; +} string_table_t; + /* Structures embedded in coveraged program. The structures generated by write_profile must match these. */ @@ -871,8 +891,13 @@ GCOV_LINKAGE void gcov_read_pmu_branch_mispredict_info (gcov_pmu_brm_info_t *brm_info, gcov_unsigned_t len) ATTRIBUTE_HIDDEN; GCOV_LINKAGE void +gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t *entry, + gcov_unsigned_t len) ATTRIBUTE_HIDDEN; + +GCOV_LINKAGE void gcov_read_pmu_tool_header (gcov_pmu_tool_header_t *tool_header, gcov_unsigned_t len) ATTRIBUTE_HIDDEN; + #ifndef __GCOV_KERNEL__ GCOV_LINKAGE float convert_unsigned_to_pct ( const unsigned number) ATTRIBUTE_HIDDEN; @@ -887,6 +912,9 @@ GCOV_LINKAGE void print_load_latency_line (FILE *f GCOV_LINKAGE void print_branch_mispredict_line (FILE *fp, const gcov_pmu_brm_info_t *brm_info, const enum print_newline); +GCOV_LINKAGE void +print_pmu_string_table_entry (FILE* fp, const gcov_pmu_st_entry_t *entry, + const enum print_newline); GCOV_LINKAGE void print_pmu_tool_header (FILE *fp, gcov_pmu_tool_header_t *tool_header, const enum print_newline); @@ -905,7 +933,16 @@ GCOV_LINKAGE void gcov_write_tag_length (gcov_unsi GCOV_LINKAGE void gcov_write_summary (gcov_unsigned_t /*tag*/, const struct gcov_summary *) ATTRIBUTE_HIDDEN; - +GCOV_LINKAGE void +gcov_write_tool_header (gcov_pmu_tool_header_t *header) ATTRIBUTE_HIDDEN; +GCOV_LINKAGE void +gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info) ATTRIBUTE_HIDDEN; +GCOV_LINKAGE void +gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t + *brm_info) ATTRIBUTE_HIDDEN; +GCOV_LINKAGE void +gcov_write_string_table_entry (const gcov_pmu_st_entry_t + *st_entry) ATTRIBUTE_HIDDEN; GCOV_LINKAGE void gcov_write_module_infos (struct gcov_info *mod_info) ATTRIBUTE_HIDDEN; GCOV_LINKAGE const struct dyn_imp_mod ** Index: gcc/gcov-dump.c =================================================================== --- gcc/gcov-dump.c (revision 190359) +++ gcc/gcov-dump.c (working copy) @@ -43,6 +43,7 @@ static void tag_summary (const char *, unsigned, u static void tag_module_info (const char *, unsigned, unsigned); static void tag_pmu_load_latency_info (const char *, unsigned, unsigned); static void tag_pmu_branch_mispredict_info (const char *, unsigned, unsigned); +static void tag_pmu_string_table_entry (const char*, unsigned, unsigned); static void tag_pmu_tool_header (const char *, unsigned, unsigned); extern int main (int, char **); @@ -84,6 +85,8 @@ static const tag_format_t tag_table[] = {GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, "PMU_BRANCH_MISPREDICT_INFO", tag_pmu_branch_mispredict_info}, {GCOV_TAG_PMU_TOOL_HEADER, "PMU_TOOL_HEADER", tag_pmu_tool_header}, + {GCOV_TAG_PMU_STRING_TABLE_ENTRY, "PMU_STRING_TABLE_ENTRY", + tag_pmu_string_table_entry}, {0, NULL, NULL} }; @@ -573,7 +576,6 @@ tag_pmu_load_latency_info (const char *filename AT gcov_pmu_ll_info_t ll_info; gcov_read_pmu_load_latency_info (&ll_info, length); print_load_latency_line (stdout, &ll_info, no_newline); - free (ll_info.filename); } /* Read gcov tag GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO from the gcda @@ -586,9 +588,17 @@ tag_pmu_branch_mispredict_info (const char *filena gcov_pmu_brm_info_t brm_info; gcov_read_pmu_branch_mispredict_info (&brm_info, length); print_branch_mispredict_line (stdout, &brm_info, no_newline); - free (brm_info.filename); } +static void +tag_pmu_string_table_entry (const char *filename ATTRIBUTE_UNUSED, + unsigned tag ATTRIBUTE_UNUSED, unsigned length) +{ + gcov_pmu_st_entry_t st_entry; + gcov_read_pmu_string_table_entry(&st_entry, length); + print_pmu_string_table_entry(stdout, &st_entry, no_newline); + free(st_entry.str); +} /* Read gcov tag GCOV_TAG_PMU_TOOL_HEADER from the gcda file and print the contents in a human readable form. */ -- This patch is available for review at http://codereview.appspot.com/6427063
Sign in to reply to this message.
|