Comments are inlined. Attached is the new patch. Thanks,
-Rong
On Tue, Sep 25, 2012 at 2:25 PM, Xinliang David Li <davi...@google.com> wrote:
> On Mon, Sep 24, 2012 at 2:42 PM, Rong Xu <x...@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 <x...@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
patch_set2.diff
Description: Binary data
