Patchwork [google,4.7] fdo build for linux kernel (issue 6968046)

login
register
mail settings
Submitter Rong Xu
Date Dec. 20, 2012, 1:22 a.m.
Message ID <CAF1bQ=Qa8jo+Djsa0Bjzbq8ibiyrPPb2YqT109Kwmuzevxh6Sw@mail.gmail.com>
Download mbox | patch
Permalink /patch/207583/
State New
Headers show

Comments

Rong Xu - Dec. 20, 2012, 1:22 a.m.
On Wed, Dec 19, 2012 at 5:04 PM,  <davidxl@google.com> wrote:
> The change in gcov-io.h is from a different patch.

sorry. here is the patch for gcov-io.h:

 GCOV_LINKAGE void gcov_sync (gcov_position_t /*base*/,

>
> David
>
>
> https://codereview.appspot.com/6968046/diff/1/gcc/gcov-io.c
> File gcc/gcov-io.c (right):
>
> https://codereview.appspot.com/6968046/diff/1/gcc/gcov-io.c#newcode688
> gcc/gcov-io.c:688:
> Have you compared this with this impl:
>
> while (x)
> {
>    c++;
>    x&=(x-1)
> }
> return c;
>

I did not try this pimplier version. I can do a test on the dump speed
and report back.

> https://codereview.appspot.com/6968046/
Rong Xu - Dec. 20, 2012, 7:30 p.m.
On Wed, Dec 19, 2012 at 5:22 PM, Rong Xu <xur@google.com> wrote:
> On Wed, Dec 19, 2012 at 5:04 PM,  <davidxl@google.com> wrote:
>> The change in gcov-io.h is from a different patch.
>
> sorry. here is the patch for gcov-io.h:
>
> Index: gcov-io.h
> ===================================================================
> --- gcov-io.h   (revision 194562)
> +++ gcov-io.h   (working copy)
> @@ -781,8 +781,8 @@
>                                           unused) */
>
>    unsigned n_functions;                /* number of functions */
> -  const struct gcov_fn_info *const *functions; /* pointer to pointers
> -                                                 to function information  */
> +  const struct gcov_fn_info **functions; /* pointer to pointers
> +                                           to function information  */
>  };
>
>  /* Information about a single imported module.  */
> @@ -988,8 +988,7 @@
>  GCOV_LINKAGE void gcov_seek (gcov_position_t /*position*/) ATTRIBUTE_HIDDEN;
>  GCOV_LINKAGE void gcov_truncate (void) ATTRIBUTE_HIDDEN;
>  GCOV_LINKAGE gcov_unsigned_t gcov_string_length (const char *)
> ATTRIBUTE_HIDDEN;
> -GCOV_LINKAGE unsigned gcov_gcda_file_size (struct gcov_info *,
> -                                           struct gcov_summary *);
> +GCOV_LINKAGE unsigned gcov_gcda_file_size (struct gcov_info *);
>  #else
>  /* Available outside libgcov */
>  GCOV_LINKAGE void gcov_sync (gcov_position_t /*base*/,
>
>>
>> David
>>
>>
>> https://codereview.appspot.com/6968046/diff/1/gcc/gcov-io.c
>> File gcc/gcov-io.c (right):
>>
>> https://codereview.appspot.com/6968046/diff/1/gcc/gcov-io.c#newcode688
>> gcc/gcov-io.c:688:
>> Have you compared this with this impl:
>>
>> while (x)
>> {
>>    c++;
>>    x&=(x-1)
>> }
>> return c;
>>
>
> I did not try this pimplier version. I can do a test on the dump speed
> and report back.

This simpler version is about 2% slow in dumping the profiles (average
of 10 dumps). But this is not a big deal.
I'll use this this version.

-Rong

>
>> https://codereview.appspot.com/6968046/
Xinliang David Li - Dec. 20, 2012, 7:54 p.m.
It  depends on the value distribution .

David

On Thu, Dec 20, 2012 at 11:30 AM, Rong Xu <xur@google.com> wrote:
> On Wed, Dec 19, 2012 at 5:22 PM, Rong Xu <xur@google.com> wrote:
>> On Wed, Dec 19, 2012 at 5:04 PM,  <davidxl@google.com> wrote:
>>> The change in gcov-io.h is from a different patch.
>>
>> sorry. here is the patch for gcov-io.h:
>>
>> Index: gcov-io.h
>> ===================================================================
>> --- gcov-io.h   (revision 194562)
>> +++ gcov-io.h   (working copy)
>> @@ -781,8 +781,8 @@
>>                                           unused) */
>>
>>    unsigned n_functions;                /* number of functions */
>> -  const struct gcov_fn_info *const *functions; /* pointer to pointers
>> -                                                 to function information  */
>> +  const struct gcov_fn_info **functions; /* pointer to pointers
>> +                                           to function information  */
>>  };
>>
>>  /* Information about a single imported module.  */
>> @@ -988,8 +988,7 @@
>>  GCOV_LINKAGE void gcov_seek (gcov_position_t /*position*/) ATTRIBUTE_HIDDEN;
>>  GCOV_LINKAGE void gcov_truncate (void) ATTRIBUTE_HIDDEN;
>>  GCOV_LINKAGE gcov_unsigned_t gcov_string_length (const char *)
>> ATTRIBUTE_HIDDEN;
>> -GCOV_LINKAGE unsigned gcov_gcda_file_size (struct gcov_info *,
>> -                                           struct gcov_summary *);
>> +GCOV_LINKAGE unsigned gcov_gcda_file_size (struct gcov_info *);
>>  #else
>>  /* Available outside libgcov */
>>  GCOV_LINKAGE void gcov_sync (gcov_position_t /*base*/,
>>
>>>
>>> David
>>>
>>>
>>> https://codereview.appspot.com/6968046/diff/1/gcc/gcov-io.c
>>> File gcc/gcov-io.c (right):
>>>
>>> https://codereview.appspot.com/6968046/diff/1/gcc/gcov-io.c#newcode688
>>> gcc/gcov-io.c:688:
>>> Have you compared this with this impl:
>>>
>>> while (x)
>>> {
>>>    c++;
>>>    x&=(x-1)
>>> }
>>> return c;
>>>
>>
>> I did not try this pimplier version. I can do a test on the dump speed
>> and report back.
>
> This simpler version is about 2% slow in dumping the profiles (average
> of 10 dumps). But this is not a big deal.
> I'll use this this version.
>
> -Rong
>
>>
>>> https://codereview.appspot.com/6968046/
Rong Xu - Dec. 20, 2012, 10 p.m.
that's right. but there is no way to predict the pattern.
what I meant was as far as it does not introduce major slow-down in
dumping profile, I'd like to use the simpler version.
what do you think?

-Rong

On Thu, Dec 20, 2012 at 11:54 AM, Xinliang David Li <davidxl@google.com> wrote:
> It  depends on the value distribution .
>
> David
>
> On Thu, Dec 20, 2012 at 11:30 AM, Rong Xu <xur@google.com> wrote:
>> On Wed, Dec 19, 2012 at 5:22 PM, Rong Xu <xur@google.com> wrote:
>>> On Wed, Dec 19, 2012 at 5:04 PM,  <davidxl@google.com> wrote:
>>>> The change in gcov-io.h is from a different patch.
>>>
>>> sorry. here is the patch for gcov-io.h:
>>>
>>> Index: gcov-io.h
>>> ===================================================================
>>> --- gcov-io.h   (revision 194562)
>>> +++ gcov-io.h   (working copy)
>>> @@ -781,8 +781,8 @@
>>>                                           unused) */
>>>
>>>    unsigned n_functions;                /* number of functions */
>>> -  const struct gcov_fn_info *const *functions; /* pointer to pointers
>>> -                                                 to function information  */
>>> +  const struct gcov_fn_info **functions; /* pointer to pointers
>>> +                                           to function information  */
>>>  };
>>>
>>>  /* Information about a single imported module.  */
>>> @@ -988,8 +988,7 @@
>>>  GCOV_LINKAGE void gcov_seek (gcov_position_t /*position*/) ATTRIBUTE_HIDDEN;
>>>  GCOV_LINKAGE void gcov_truncate (void) ATTRIBUTE_HIDDEN;
>>>  GCOV_LINKAGE gcov_unsigned_t gcov_string_length (const char *)
>>> ATTRIBUTE_HIDDEN;
>>> -GCOV_LINKAGE unsigned gcov_gcda_file_size (struct gcov_info *,
>>> -                                           struct gcov_summary *);
>>> +GCOV_LINKAGE unsigned gcov_gcda_file_size (struct gcov_info *);
>>>  #else
>>>  /* Available outside libgcov */
>>>  GCOV_LINKAGE void gcov_sync (gcov_position_t /*base*/,
>>>
>>>>
>>>> David
>>>>
>>>>
>>>> https://codereview.appspot.com/6968046/diff/1/gcc/gcov-io.c
>>>> File gcc/gcov-io.c (right):
>>>>
>>>> https://codereview.appspot.com/6968046/diff/1/gcc/gcov-io.c#newcode688
>>>> gcc/gcov-io.c:688:
>>>> Have you compared this with this impl:
>>>>
>>>> while (x)
>>>> {
>>>>    c++;
>>>>    x&=(x-1)
>>>> }
>>>> return c;
>>>>
>>>
>>> I did not try this pimplier version. I can do a test on the dump speed
>>> and report back.
>>
>> This simpler version is about 2% slow in dumping the profiles (average
>> of 10 dumps). But this is not a big deal.
>> I'll use this this version.
>>
>> -Rong
>>
>>>
>>>> https://codereview.appspot.com/6968046/

Patch

Index: gcov-io.h
===================================================================
--- gcov-io.h   (revision 194562)
+++ gcov-io.h   (working copy)
@@ -781,8 +781,8 @@ 
                                          unused) */

   unsigned n_functions;                /* number of functions */
-  const struct gcov_fn_info *const *functions; /* pointer to pointers
-                                                 to function information  */
+  const struct gcov_fn_info **functions; /* pointer to pointers
+                                           to function information  */
 };

 /* Information about a single imported module.  */
@@ -988,8 +988,7 @@ 
 GCOV_LINKAGE void gcov_seek (gcov_position_t /*position*/) ATTRIBUTE_HIDDEN;
 GCOV_LINKAGE void gcov_truncate (void) ATTRIBUTE_HIDDEN;
 GCOV_LINKAGE gcov_unsigned_t gcov_string_length (const char *)
ATTRIBUTE_HIDDEN;
-GCOV_LINKAGE unsigned gcov_gcda_file_size (struct gcov_info *,
-                                           struct gcov_summary *);
+GCOV_LINKAGE unsigned gcov_gcda_file_size (struct gcov_info *);
 #else
 /* Available outside libgcov */