| Message ID | 20120924214232.B0F1B55CAC@rong2.dls.corp.google.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 24, 2012 at 2:42 PM, Rong Xu <xur@google.com> wrote: > Hi, > > This is for google branches only. > It fix the lino number checksum mismatch during LIPO-use build. > > Tested with SPEC and google internal banchmarks. > > Thanks, > > -Rong > > 2012-09-24 Rong Xu <xur@google.com> > > * gcc/coverage.c (coverage_checksum_string): strip out LIPO > specific string. > (crc32_string_1): New function. > * gcc/cp/decl2.c (start_static_storage_duration_function): > generate LIPO specific string. > > Index: gcc/coverage.c > =================================================================== > --- gcc/coverage.c (revision 191679) > +++ gcc/coverage.c (working copy) > @@ -903,6 +903,27 @@ > } > > > +/* Generate a crc32 of a string with specified STR_ELN when it's not 0. STR_ELN --> STR_LEN > + Non-zero STR_LEN should only be seen in LIPO mode. */ Empty line needed. > +static unsigned > +crc32_string_1 (unsigned chksum, const char *string, unsigned str_len) > +{ > + char *dup; > + > + if (!L_IPO_COMP_MODE || str_len == 0) > + return crc32_string (chksum, string); > + > + gcc_assert (str_len > 0 && str_len < strlen(string)); > + dup = xstrdup (string); > + dup[str_len] = 0; > + chksum = crc32_string (chksum, dup); > + free (dup); > + > + return chksum; > + Remove extra lines after return. > + > +} > + > /* Generate a checksum for a string. CHKSUM is the current > checksum. */ > > @@ -911,7 +932,26 @@ > { > int i; > char *dup = NULL; > + unsigned lipo_orig_str_len = 0; > > + /* Strip out the ending "_cmo_[0-9]*" string from function > + name. Otherwise we will have lineno checksum mismatch. */ > + if (L_IPO_COMP_MODE) > + { > + int len; > + > + i = len = strlen (string); > + while (i--) > + if ((string[i] < '0' || string[i] > '9')) > + break; > + if ((i > 5) && (i != len - 1)) i >= 5? > + { > + if (!strncmp (string + i - 4, "_cmo_", 5)) _cmo_ or .cmo. ? > + lipo_orig_str_len = i - 4; > + } > + > + } > + > /* Look for everything that looks if it were produced by > get_file_function_name and zero out the second part > that may result from flag_random_seed. This is not critical > @@ -957,7 +997,7 @@ > } > } > > - chksum = crc32_string (chksum, string); > + chksum = crc32_string_1 (chksum, string, lipo_orig_str_len); > if (dup) > free (dup); > > Index: gcc/cp/decl2.c > =================================================================== > --- gcc/cp/decl2.c (revision 191679) > +++ gcc/cp/decl2.c (working copy) > @@ -2911,7 +2911,7 @@ > SSDF_IDENTIFIER_<number>. */ > sprintf (id, "%s_%u", SSDF_IDENTIFIER, count); > if (L_IPO_IS_AUXILIARY_MODULE) > - sprintf (id, "%s_%u", id, current_module_id); > + sprintf (id, "%s_cmo_%u", id, current_module_id); _cmo_ or .cmo. for consistency? David > > type = build_function_type_list (void_type_node, > integer_type_node, integer_type_node, > > -- > This patch is available for review at http://codereview.appspot.com/6566044
Comments are inlined. Attached is the new patch. Thanks, -Rong On Tue, Sep 25, 2012 at 2:25 PM, Xinliang David Li <davidxl@google.com> wrote: > On Mon, Sep 24, 2012 at 2:42 PM, Rong Xu <xur@google.com> wrote: >> Hi, >> >> This is for google branches only. >> It fix the lino number checksum mismatch during LIPO-use build. >> >> Tested with SPEC and google internal banchmarks. >> >> Thanks, >> >> -Rong >> >> 2012-09-24 Rong Xu <xur@google.com> >> >> * gcc/coverage.c (coverage_checksum_string): strip out LIPO >> specific string. >> (crc32_string_1): New function. >> * gcc/cp/decl2.c (start_static_storage_duration_function): >> generate LIPO specific string. >> >> Index: gcc/coverage.c >> =================================================================== >> --- gcc/coverage.c (revision 191679) >> +++ gcc/coverage.c (working copy) >> @@ -903,6 +903,27 @@ >> } >> >> >> +/* Generate a crc32 of a string with specified STR_ELN when it's not 0. > > STR_ELN --> STR_LEN Fixed. > >> + Non-zero STR_LEN should only be seen in LIPO mode. */ > > Empty line needed. Fixed. > >> +static unsigned >> +crc32_string_1 (unsigned chksum, const char *string, unsigned str_len) >> +{ >> + char *dup; >> + >> + if (!L_IPO_COMP_MODE || str_len == 0) >> + return crc32_string (chksum, string); >> + >> + gcc_assert (str_len > 0 && str_len < strlen(string)); >> + dup = xstrdup (string); >> + dup[str_len] = 0; >> + chksum = crc32_string (chksum, dup); >> + free (dup); >> + >> + return chksum; >> + > > Remove extra lines after return. Fixed. > >> + >> +} >> + >> /* Generate a checksum for a string. CHKSUM is the current >> checksum. */ >> >> @@ -911,7 +932,26 @@ >> { >> int i; >> char *dup = NULL; >> + unsigned lipo_orig_str_len = 0; >> >> + /* Strip out the ending "_cmo_[0-9]*" string from function >> + name. Otherwise we will have lineno checksum mismatch. */ >> + if (L_IPO_COMP_MODE) >> + { >> + int len; >> + >> + i = len = strlen (string); >> + while (i--) >> + if ((string[i] < '0' || string[i] > '9')) >> + break; >> + if ((i > 5) && (i != len - 1)) > > i >= 5? This should not matter because we are expecting a non-empty sub-string before "_cmo_". If there not sub-string before "_cmo_", the original code will do nothing (which I think it's correct in the case of user defined name.) > >> + { >> + if (!strncmp (string + i - 4, "_cmo_", 5)) > > _cmo_ or .cmo. ? > >> + lipo_orig_str_len = i - 4; >> + } >> + >> + } >> + >> /* Look for everything that looks if it were produced by >> get_file_function_name and zero out the second part >> that may result from flag_random_seed. This is not critical >> @@ -957,7 +997,7 @@ >> } >> } >> >> - chksum = crc32_string (chksum, string); >> + chksum = crc32_string_1 (chksum, string, lipo_orig_str_len); >> if (dup) >> free (dup); >> >> Index: gcc/cp/decl2.c >> =================================================================== >> --- gcc/cp/decl2.c (revision 191679) >> +++ gcc/cp/decl2.c (working copy) >> @@ -2911,7 +2911,7 @@ >> SSDF_IDENTIFIER_<number>. */ >> sprintf (id, "%s_%u", SSDF_IDENTIFIER, count); >> if (L_IPO_IS_AUXILIARY_MODULE) >> - sprintf (id, "%s_%u", id, current_module_id); >> + sprintf (id, "%s_cmo_%u", id, current_module_id); > > _cmo_ or .cmo. for consistency? Changed all "_cmo_" to ".cmo.". > > David > >> >> type = build_function_type_list (void_type_node, >> integer_type_node, integer_type_node, >> >> -- >> This patch is available for review at http://codereview.appspot.com/6566044
ok (for google-47 and google/main) thanks, David On Fri, Sep 28, 2012 at 10:22 AM, Rong Xu <xur@google.com> wrote: > Comments are inlined. > Attached is the new patch. > > Thanks, > > -Rong > > On Tue, Sep 25, 2012 at 2:25 PM, Xinliang David Li <davidxl@google.com> wrote: >> On Mon, Sep 24, 2012 at 2:42 PM, Rong Xu <xur@google.com> wrote: >>> Hi, >>> >>> This is for google branches only. >>> It fix the lino number checksum mismatch during LIPO-use build. >>> >>> Tested with SPEC and google internal banchmarks. >>> >>> Thanks, >>> >>> -Rong >>> >>> 2012-09-24 Rong Xu <xur@google.com> >>> >>> * gcc/coverage.c (coverage_checksum_string): strip out LIPO >>> specific string. >>> (crc32_string_1): New function. >>> * gcc/cp/decl2.c (start_static_storage_duration_function): >>> generate LIPO specific string. >>> >>> Index: gcc/coverage.c >>> =================================================================== >>> --- gcc/coverage.c (revision 191679) >>> +++ gcc/coverage.c (working copy) >>> @@ -903,6 +903,27 @@ >>> } >>> >>> >>> +/* Generate a crc32 of a string with specified STR_ELN when it's not 0. >> >> STR_ELN --> STR_LEN > > Fixed. > >> >>> + Non-zero STR_LEN should only be seen in LIPO mode. */ >> >> Empty line needed. > > Fixed. > >> >>> +static unsigned >>> +crc32_string_1 (unsigned chksum, const char *string, unsigned str_len) >>> +{ >>> + char *dup; >>> + >>> + if (!L_IPO_COMP_MODE || str_len == 0) >>> + return crc32_string (chksum, string); >>> + >>> + gcc_assert (str_len > 0 && str_len < strlen(string)); >>> + dup = xstrdup (string); >>> + dup[str_len] = 0; >>> + chksum = crc32_string (chksum, dup); >>> + free (dup); >>> + >>> + return chksum; >>> + >> >> Remove extra lines after return. > > Fixed. > >> >>> + >>> +} >>> + >>> /* Generate a checksum for a string. CHKSUM is the current >>> checksum. */ >>> >>> @@ -911,7 +932,26 @@ >>> { >>> int i; >>> char *dup = NULL; >>> + unsigned lipo_orig_str_len = 0; >>> >>> + /* Strip out the ending "_cmo_[0-9]*" string from function >>> + name. Otherwise we will have lineno checksum mismatch. */ >>> + if (L_IPO_COMP_MODE) >>> + { >>> + int len; >>> + >>> + i = len = strlen (string); >>> + while (i--) >>> + if ((string[i] < '0' || string[i] > '9')) >>> + break; >>> + if ((i > 5) && (i != len - 1)) >> >> i >= 5? > > This should not matter because we are expecting a non-empty sub-string > before "_cmo_". If there not sub-string before "_cmo_", the original > code will do nothing (which I think it's correct in the case of user > defined name.) > >> >>> + { >>> + if (!strncmp (string + i - 4, "_cmo_", 5)) >> >> _cmo_ or .cmo. ? >> >>> + lipo_orig_str_len = i - 4; >>> + } >>> + >>> + } >>> + >>> /* Look for everything that looks if it were produced by >>> get_file_function_name and zero out the second part >>> that may result from flag_random_seed. This is not critical >>> @@ -957,7 +997,7 @@ >>> } >>> } >>> >>> - chksum = crc32_string (chksum, string); >>> + chksum = crc32_string_1 (chksum, string, lipo_orig_str_len); >>> if (dup) >>> free (dup); >>> >>> Index: gcc/cp/decl2.c >>> =================================================================== >>> --- gcc/cp/decl2.c (revision 191679) >>> +++ gcc/cp/decl2.c (working copy) >>> @@ -2911,7 +2911,7 @@ >>> SSDF_IDENTIFIER_<number>. */ >>> sprintf (id, "%s_%u", SSDF_IDENTIFIER, count); >>> if (L_IPO_IS_AUXILIARY_MODULE) >>> - sprintf (id, "%s_%u", id, current_module_id); >>> + sprintf (id, "%s_cmo_%u", id, current_module_id); >> >> _cmo_ or .cmo. for consistency? > > Changed all "_cmo_" to ".cmo.". > >> >> David >> >>> >>> type = build_function_type_list (void_type_node, >>> integer_type_node, integer_type_node, >>> >>> -- >>> This patch is available for review at http://codereview.appspot.com/6566044
Index: gcc/coverage.c =================================================================== --- gcc/coverage.c (revision 191679) +++ gcc/coverage.c (working copy) @@ -903,6 +903,27 @@ } +/* Generate a crc32 of a string with specified STR_ELN when it's not 0. + Non-zero STR_LEN should only be seen in LIPO mode. */ +static unsigned +crc32_string_1 (unsigned chksum, const char *string, unsigned str_len) +{ + char *dup; + + if (!L_IPO_COMP_MODE || str_len == 0) + return crc32_string (chksum, string); + + gcc_assert (str_len > 0 && str_len < strlen(string)); + dup = xstrdup (string); + dup[str_len] = 0; + chksum = crc32_string (chksum, dup); + free (dup); + + return chksum; + + +} + /* Generate a checksum for a string. CHKSUM is the current checksum. */ @@ -911,7 +932,26 @@ { int i; char *dup = NULL; + unsigned lipo_orig_str_len = 0; + /* Strip out the ending "_cmo_[0-9]*" string from function + name. Otherwise we will have lineno checksum mismatch. */ + if (L_IPO_COMP_MODE) + { + int len; + + i = len = strlen (string); + while (i--) + if ((string[i] < '0' || string[i] > '9')) + break; + if ((i > 5) && (i != len - 1)) + { + if (!strncmp (string + i - 4, "_cmo_", 5)) + lipo_orig_str_len = i - 4; + } + + } + /* Look for everything that looks if it were produced by get_file_function_name and zero out the second part that may result from flag_random_seed. This is not critical @@ -957,7 +997,7 @@ } } - chksum = crc32_string (chksum, string); + chksum = crc32_string_1 (chksum, string, lipo_orig_str_len); if (dup) free (dup); Index: gcc/cp/decl2.c =================================================================== --- gcc/cp/decl2.c (revision 191679) +++ gcc/cp/decl2.c (working copy) @@ -2911,7 +2911,7 @@ SSDF_IDENTIFIER_<number>. */ sprintf (id, "%s_%u", SSDF_IDENTIFIER, count); if (L_IPO_IS_AUXILIARY_MODULE) - sprintf (id, "%s_%u", id, current_module_id); + sprintf (id, "%s_cmo_%u", id, current_module_id); type = build_function_type_list (void_type_node, integer_type_node, integer_type_node,
Hi, This is for google branches only. It fix the lino number checksum mismatch during LIPO-use build. Tested with SPEC and google internal banchmarks. Thanks, -Rong 2012-09-24 Rong Xu <xur@google.com> * gcc/coverage.c (coverage_checksum_string): strip out LIPO specific string. (crc32_string_1): New function. * gcc/cp/decl2.c (start_static_storage_duration_function): generate LIPO specific string. -- This patch is available for review at http://codereview.appspot.com/6566044