ok (for google-47 and google/main)
thanks,
David
On Fri, Sep 28, 2012 at 10:22 AM, Rong Xu <x...@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 <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
