Issue 6968046: [google 4.7] fdo build for linux kernel

Can't Edit
Can't Publish+Mail
Start Review

Created:
11 years, 4 months ago by xur

Modified:
11 years, 4 months ago

Reviewers:
tejohnson, davidxl

CC:
gcc-patches_gcc.gnu.org

Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/branches/google/gcc-4_7/

Visibility:
Public.

Patch Set 1 #

Total comments: 1

Created: 11 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+95 lines, -31 lines)			Patch
M	gcc/gcov-io.h	View	4 chunks	+23 lines, -4 lines	0 comments	Download
M	gcc/gcov-io.c	View	5 chunks	+34 lines, -5 lines	1 comment	Download
M	libgcc/libgcov.c	View	18 chunks	+38 lines, -22 lines	0 comments	Download

The change in gcov-io.h is from a different patch. 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 ...

11 years, 4 months ago (2012-12-20 01:04:46 UTC) #3

xur

On Wed, Dec 19, 2012 at 5:04 PM, <davidxl@google.com> wrote: > The change in gcov-io.h ...

11 years, 4 months ago (2012-12-20 01:22:23 UTC) #4

xur

On Wed, Dec 19, 2012 at 5:22 PM, Rong Xu <xur@google.com> wrote: > On Wed, ...

11 years, 4 months ago (2012-12-20 19:30:38 UTC) #5

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/

davidxl

It depends on the value distribution . David On Thu, Dec 20, 2012 at 11:30 ...

11 years, 4 months ago (2012-12-20 19:54:58 UTC) #6

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/

xur

that's right. but there is no way to predict the pattern. what I meant was ...

11 years, 4 months ago (2012-12-20 22:00:53 UTC) #7

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/

tejohnson

On Wed, Dec 19, 2012 at 12:11 PM, Rong Xu <xur@google.com> wrote: > Hi, > ...

11 years, 4 months ago (2012-12-21 16:31:48 UTC) #8

On Wed, Dec 19, 2012 at 12:11 PM, Rong Xu <xur@google.com> wrote:
> Hi,
>
> This patch updates the support for FDO build in linux kernel for gcc 4.7.
> Tested with 2.6.34 kernel and google internal benchmarks.
>
> Thanks,
>
> -Rong
>
> 2012-12-19  Rong Xu  <xur@google.com>
>
>         * libgcc/libgcov.c
>         (gcov_counter_active): v4.7 kernel fdo support.
>         (crc32_unsigned): Include in GCOV_KERNEL build.
>         (gcov_alloc_filename): Remove from GCOV_KERNEL build.
>         (gcov_sort_icall_topn_counter): Ditto.
>         (gcov_dump_module_info): Ditto.
>         (gcov_compute_histogram): v4.7 kernel fdo support.
>         (gcov_merge_gcda_file): Ditto.
>         (gcov_gcda_file_size): Ditto.
>         (gcov_write_gcda_file): Ditto.
>         (__gcov_topn_value_profiler_body): Ditto.
>         (gcov_set_vfile): Ditto.
>         (gcov_kernel_dump_gcov_init): Ditto.
>         (gcov_kernel_dump_one_gcov): Ditto.
>         * gcc/gcov-io.c (gcov_read_string): Ditto.
>         (static int k_popcountll): Ditto.
>         (gcov_read_summary): Ditto.
>         (kernel_file_fclose): Ditto.
>         (kernel_file_ftell): Ditto.
>         (kernel_file_fwrite): Ditto.
>         * gcc/gcov-io.h (struct gcov_info): remove const keyword.
>
> Index: libgcc/libgcov.c
> ===================================================================
> --- libgcc/libgcov.c    (revision 194562)
> +++ libgcc/libgcov.c    (working copy)
> @@ -46,11 +46,11 @@ see the files COPYING3 and COPYING.RUNTIME respect
>  #include "tsystem.h"
>  #include "coretypes.h"
>  #include "tm.h"
> -#endif /* __KERNEL__ */
>  #include "libgcc_tm.h"
>  #include "gthr.h"
> +#endif /* __KERNEL__ */
>
> -#if 1
> +#ifndef __KERNEL__
>  #define THREAD_PREFIX __thread
>  #else
>  #define THREAD_PREFIX
> @@ -120,6 +120,7 @@ extern int gcov_dump_complete ATTRIBUTE_HIDDEN;
>  #ifdef L_gcov
>  #include "gcov-io.c"
>
> +#ifndef __GCOV_KERNEL__
>  /* Create a strong reference to these symbols so that they are
>     unconditionally pulled into the instrumented binary, even when
>     the only reference is a weak reference. This is necessary because
> @@ -134,6 +135,7 @@ extern int gcov_dump_complete ATTRIBUTE_HIDDEN;
>     these symbols will always need to be resolved.  */
>  void (*__gcov_dummy_ref1)() = &__gcov_reset;
>  void (*__gcov_dummy_ref2)() = &__gcov_dump;
> +#endif /* __GCOV_KERNEL__ */
>
>  /* Utility function for outputing errors.  */
>  static int
> @@ -151,6 +153,10 @@ gcov_error (const char *fmt, ...)
>    return ret;
>  }
>
> +/* A program checksum allows us to distinguish program data for an
> +   object file included in multiple programs.  */
> +static gcov_unsigned_t gcov_crc32;
> +
>  #ifndef __GCOV_KERNEL__
>  /* Emitted in coverage.c.  */
>  extern char * __gcov_pmu_profile_filename;
> @@ -183,10 +189,6 @@ THREAD_PREFIX gcov_unsigned_t __gcov_sample_counte
>  /* Chain of per-object gcov structures.  */
>  extern struct gcov_info *__gcov_list;
>
> -/* A program checksum allows us to distinguish program data for an
> -   object file included in multiple programs.  */
> -static gcov_unsigned_t gcov_crc32;
> -
>  /* Size of the longest file name. */
>  static size_t gcov_max_filename = 0;
>
> @@ -323,8 +325,6 @@ gcov_counter_active (const struct gcov_info *info,
>    return (info->merge[type] != 0);
>  }
>
> -#ifndef __GCOV_KERNEL__
> -
>  /* Add an unsigned value to the current crc */
>
>  static gcov_unsigned_t
> @@ -344,6 +344,8 @@ crc32_unsigned (gcov_unsigned_t crc32, gcov_unsign
>    return crc32;
>  }
>
> +#ifndef __GCOV_KERNEL__
> +
>  /* Check if VERSION of the info block PTR matches libgcov one.
>     Return 1 on success, or zero in case of versions mismatch.
>     If FILENAME is not NULL, its value used for reporting purposes
> @@ -464,6 +466,8 @@ gcov_alloc_filename (void)
>    gi_filename_up = gi_filename + prefix_length;
>  }
>
> +#endif /* __GCOV_KERNEL__ */
> +
>  /* Sort N entries in VALUE_ARRAY in descending order.
>     Each entry in VALUE_ARRAY has two values. The sorting
>     is based on the second value.  */
> @@ -509,6 +513,8 @@ gcov_sort_icall_topn_counter (const struct gcov_ct
>      }
>  }
>
> +#ifndef __GCOV_KERNEL__
> +
>  /* Write imported files (auxiliary modules) for primary module GI_PTR
>     into file GI_FILENAME.  */
>
> @@ -586,6 +592,8 @@ gcov_dump_module_info (void)
>    __gcov_finalize_dyn_callgraph ();
>  }
>
> +#endif /* __GCOV_KERNEL__ */
> +
>  /* Insert counter VALUE into HISTOGRAM.  */
>
>  static void
> @@ -656,6 +664,8 @@ gcov_compute_histogram (struct gcov_summary *sum)
>      }
>  }
>
> +#ifndef __GCOV_KERNEL__
> +
>  /* Dump the coverage counts. We merge with existing counts when
>     possible, to avoid growing the .da files ad infinitum. We use this
>     program's checksum to make sure we only accumulate whole program
> @@ -1013,12 +1023,7 @@ gcov_merge_gcda_file (struct gcov_info *gi_ptr)
>               goto read_error;
>         }
>       if (tag && tag != GCOV_TAG_MODULE_INFO)
> -       {
> -         read_mismatch:;
> -        fprintf (stderr, "profiling:%s:Merge mismatch for %s\n",
> -                 gi_filename, f_ix + 1 ? "function" : "summaries");
> -         goto read_fatal;
> -       }
> +       goto read_mismatch;
>      }
>    goto rewrite;
>
> @@ -1031,6 +1036,11 @@ read_error:;
>
>      goto rewrite;
>
> +read_mismatch:;
> +    gcov_error ("profiling:%s:Merge mismatch for %s\n",
> +            gi_filename, f_ix + 1 ? "function" : "summaries");
> +    goto read_fatal;
> +
>  read_fatal:;
>      gcov_close ();
>      return 1;
> @@ -1076,7 +1086,7 @@ rewrite:;
>                                sizeof (*cs_all) - (sizeof (gcov_bucket_type)
>                                                    * GCOV_HISTOGRAM_SIZE)))
>            {
> -            fprintf (stderr, "profiling:%s:Invocation mismatch - "
> +            gcov_error ("profiling:%s:Invocation mismatch - "
>                  "some data files may have been removed%s\n",
>              gi_filename, GCOV_LOCKED
>              ? "" : " or concurrent update without locking support");
> @@ -1093,14 +1103,14 @@ rewrite:;
>     the size is in units of gcov_type.  */
>
>  GCOV_LINKAGE unsigned
> -gcov_gcda_file_size (struct gcov_info *gi_ptr,
> -                     struct gcov_summary *sum)
> +gcov_gcda_file_size (struct gcov_info *gi_ptr)
>  {
>    unsigned size;
>    const struct gcov_fn_info *fi_ptr;
>    unsigned f_ix, t_ix, h_ix, h_cnt = 0;
>    unsigned n_counts;
>    const struct gcov_ctr_info *ci_ptr;
> +  struct gcov_summary *sum = &this_program;
>    const struct gcov_ctr_summary *csum;
>
>    /* GCOV_DATA_MAGIC, GCOV_VERSION and time_stamp.  */
> @@ -1181,13 +1191,14 @@ gcov_write_gcda_file (struct gcov_info *gi_ptr)
>        ci_ptr = gfi_ptr->ctrs;
>        for (t_ix = 0; t_ix < GCOV_COUNTERS; t_ix++)
>          {
> +          gcov_type *c_ptr;
>            if (!gi_ptr->merge[t_ix])
>              continue;
>
>            n_counts = ci_ptr->num;
>            gcov_write_tag_length (GCOV_TAG_FOR_COUNTER (t_ix),
>                                   GCOV_TAG_COUNTER_LENGTH (n_counts));
> -          gcov_type *c_ptr = ci_ptr->values;
> +          c_ptr = ci_ptr->values;
>            while (n_counts--)
>              gcov_write_counter (*c_ptr++);
>            ci_ptr++;
> @@ -1692,7 +1703,7 @@ __gcov_topn_value_profiler_body (gcov_type *counte
>       {
>         unsigned i, j;
>         gcov_type *p, minv;
> -       gcov_type* tmp_cnts
> +       gcov_type* tmp_cnts
>             = (gcov_type *)alloca (topn_val * sizeof(gcov_type));
>
>         *num_eviction = 0;
> @@ -2050,15 +2061,20 @@ gcov_set_vfile (gcov_kernel_vfile *file)
>    gcov_current_file = file;
>  }
>
> +/* Init function before dumping the gcda file in kernel.  */
> +
> +void
> +gcov_kernel_dump_gcov_init (void)
> +{
> +  gcov_exit_init ();
> +}
> +
>  /* Dump one entry in the gcov_info list (for one object) in kernel.  */
>
>  void
>  gcov_kernel_dump_one_gcov (struct gcov_info *info)
>  {
>    gcc_assert (gcov_current_file);
> -
> -  gcov_exit_init ();
> -
>    gcov_dump_one_gcov (info);
>  }
>
> Index: gcc/gcov-io.c
> ===================================================================
> --- gcc/gcov-io.c       (revision 194562)
> +++ gcc/gcov-io.c       (working copy)
> @@ -662,6 +662,30 @@ gcov_read_string (void)
>    return (const char *) gcov_read_words (length);
>  }
>
> +#ifdef __GCOV_KERNEL__
> +static const unsigned char __popcount_tab[256] =
> +{
> +    0,1,1,2,1,2,2,3,1,2,2,3,2,3,3,4,1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,
> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
> +    3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,4,5,5,6,5,6,6,7,5,6,6,7,6,7,7,8
> +};
> +
> +static int k_popcountll (long long x)
> +{
> +  int i, ret = 0;
> +
> +  for (i = 0; i < 8*sizeof(long long) ; i += 8)
> +    ret += __popcount_tab[(x >> i) & 0xff];
> +
> +  return ret;
> +}
> +#endif
> +
>  GCOV_LINKAGE void
>  gcov_read_summary (struct gcov_summary *summary)
>  {
> @@ -683,7 +707,11 @@ gcov_read_summary (struct gcov_summary *summary)
>        for (bv_ix = 0; bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE; bv_ix++)
>          {
>            histo_bitvector[bv_ix] = gcov_read_unsigned ();
> +#ifndef __GCOV_KERNEL__
>            h_cnt += __builtin_popcountll (histo_bitvector[bv_ix]);
> +#else
> +          h_cnt += k_popcountll (histo_bitvector[bv_ix]);
> +#endif

Is the issue here that __builtin_popcountll is not available? I just
committed a fix to trunk to deal with a similar issue, can you see if
this would address the problem here too?

http://gcc.gnu.org/ml/gcc-bugs/2012-12/msg01976.html

Thanks,
Teresa

>          }
>        bv_ix = 0;
>        h_ix = 0;
> @@ -1137,7 +1165,6 @@ kernel_file_fclose (gcov_kernel_vfile *fp)
>  long
>  kernel_file_ftell (gcov_kernel_vfile *fp)
>  {
> -  gcc_assert (0);  /* should not reach here */
>    return 0;
>  }
>
> @@ -1192,8 +1219,9 @@ kernel_file_fwrite (const void *ptr, size_t size,
>    if (vsize <= vpos)
>      {
>        printk (KERN_ERR
> -         "GCOV_KERNEL: something wrong: vbuf=%p vsize=%u vpos=%u\n",
> -          vbuf, vsize, vpos);
> +         "GCOV_KERNEL: something wrong in file %s: vbuf=%p vsize=%u"
> +         " vpos=%u\n",
> +          fp->info->filename, vbuf, vsize, vpos);
>        return 0;
>      }
>    len = vsize - vpos;
> @@ -1207,8 +1235,9 @@ kernel_file_fwrite (const void *ptr, size_t size,
>
>    if (len != nitems)
>      printk (KERN_ERR
> -        "GCOV_KERNEL: something wrong: size=%lu nitems=%lu ret=%d\n",
> -        size, nitems, len);
> +        "GCOV_KERNEL: something wrong in file %s: size=%lu nitems=%lu"
> +        " ret=%d, vsize=%u vpos=%u \n",
> +        fp->info->filename, size, nitems, len, vsize, vpos);
>    return len;
>  }
>
> Index: gcc/gcov-io.h
> ===================================================================
> --- gcc/gcov-io.h       (revision 194562)
> +++ gcc/gcov-io.h       (working copy)
> @@ -300,6 +300,14 @@ typedef unsigned gcov_type_unsigned __attribute__
>
>  #endif  /* BITS_PER_UNIT == 8  */
>
> +#if LONG_LONG_TYPE_SIZE > 32
> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_8
> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_8
> +#else
> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_4
> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_4
> +#endif
> +
>  #undef EXTRACT_MODULE_ID_FROM_GLOBAL_ID
>  #undef EXTRACT_FUNC_ID_FROM_GLOBAL_ID
>  #undef GEN_FUNC_GLOBAL_ID
> @@ -322,6 +330,18 @@ typedef unsigned gcov_type_unsigned __attribute__
>  typedef unsigned gcov_unsigned_t;
>  typedef unsigned gcov_position_t;
>
> +#if LONG_LONG_TYPE_SIZE > 32
> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_8
> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_8
> +#else
> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_4
> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_4
> +#endif
> +#define PROFILE_GEN_EDGE_ATOMIC (flag_profile_gen_atomic == 1 || \
> +                                 flag_profile_gen_atomic == 3)
> +#define PROFILE_GEN_VALUE_ATOMIC (flag_profile_gen_atomic == 2 || \
> +                                  flag_profile_gen_atomic == 3)
> +
>  /* gcov_type is typedef'd elsewhere for the compiler */
>  #if IN_GCOV
>  #define GCOV_LINKAGE static
> @@ -781,8 +801,8 @@ struct gcov_info
>                                           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 +1008,7 @@ static void gcov_rewrite (void);
>  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*/,
>
> --
> This patch is available for review at http://codereview.appspot.com/6968046



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

davidxl

Kernel build does not link in libgcc, which defines the function. David On Fri, Dec ...

11 years, 4 months ago (2012-12-21 17:52:59 UTC) #9

Kernel build does not link in libgcc, which defines the function.

David

On Fri, Dec 21, 2012 at 8:31 AM, Teresa Johnson <tejohnson@google.com> wrote:
> On Wed, Dec 19, 2012 at 12:11 PM, Rong Xu <xur@google.com> wrote:
>> Hi,
>>
>> This patch updates the support for FDO build in linux kernel for gcc 4.7.
>> Tested with 2.6.34 kernel and google internal benchmarks.
>>
>> Thanks,
>>
>> -Rong
>>
>> 2012-12-19  Rong Xu  <xur@google.com>
>>
>>         * libgcc/libgcov.c
>>         (gcov_counter_active): v4.7 kernel fdo support.
>>         (crc32_unsigned): Include in GCOV_KERNEL build.
>>         (gcov_alloc_filename): Remove from GCOV_KERNEL build.
>>         (gcov_sort_icall_topn_counter): Ditto.
>>         (gcov_dump_module_info): Ditto.
>>         (gcov_compute_histogram): v4.7 kernel fdo support.
>>         (gcov_merge_gcda_file): Ditto.
>>         (gcov_gcda_file_size): Ditto.
>>         (gcov_write_gcda_file): Ditto.
>>         (__gcov_topn_value_profiler_body): Ditto.
>>         (gcov_set_vfile): Ditto.
>>         (gcov_kernel_dump_gcov_init): Ditto.
>>         (gcov_kernel_dump_one_gcov): Ditto.
>>         * gcc/gcov-io.c (gcov_read_string): Ditto.
>>         (static int k_popcountll): Ditto.
>>         (gcov_read_summary): Ditto.
>>         (kernel_file_fclose): Ditto.
>>         (kernel_file_ftell): Ditto.
>>         (kernel_file_fwrite): Ditto.
>>         * gcc/gcov-io.h (struct gcov_info): remove const keyword.
>>
>> Index: libgcc/libgcov.c
>> ===================================================================
>> --- libgcc/libgcov.c    (revision 194562)
>> +++ libgcc/libgcov.c    (working copy)
>> @@ -46,11 +46,11 @@ see the files COPYING3 and COPYING.RUNTIME respect
>>  #include "tsystem.h"
>>  #include "coretypes.h"
>>  #include "tm.h"
>> -#endif /* __KERNEL__ */
>>  #include "libgcc_tm.h"
>>  #include "gthr.h"
>> +#endif /* __KERNEL__ */
>>
>> -#if 1
>> +#ifndef __KERNEL__
>>  #define THREAD_PREFIX __thread
>>  #else
>>  #define THREAD_PREFIX
>> @@ -120,6 +120,7 @@ extern int gcov_dump_complete ATTRIBUTE_HIDDEN;
>>  #ifdef L_gcov
>>  #include "gcov-io.c"
>>
>> +#ifndef __GCOV_KERNEL__
>>  /* Create a strong reference to these symbols so that they are
>>     unconditionally pulled into the instrumented binary, even when
>>     the only reference is a weak reference. This is necessary because
>> @@ -134,6 +135,7 @@ extern int gcov_dump_complete ATTRIBUTE_HIDDEN;
>>     these symbols will always need to be resolved.  */
>>  void (*__gcov_dummy_ref1)() = &__gcov_reset;
>>  void (*__gcov_dummy_ref2)() = &__gcov_dump;
>> +#endif /* __GCOV_KERNEL__ */
>>
>>  /* Utility function for outputing errors.  */
>>  static int
>> @@ -151,6 +153,10 @@ gcov_error (const char *fmt, ...)
>>    return ret;
>>  }
>>
>> +/* A program checksum allows us to distinguish program data for an
>> +   object file included in multiple programs.  */
>> +static gcov_unsigned_t gcov_crc32;
>> +
>>  #ifndef __GCOV_KERNEL__
>>  /* Emitted in coverage.c.  */
>>  extern char * __gcov_pmu_profile_filename;
>> @@ -183,10 +189,6 @@ THREAD_PREFIX gcov_unsigned_t __gcov_sample_counte
>>  /* Chain of per-object gcov structures.  */
>>  extern struct gcov_info *__gcov_list;
>>
>> -/* A program checksum allows us to distinguish program data for an
>> -   object file included in multiple programs.  */
>> -static gcov_unsigned_t gcov_crc32;
>> -
>>  /* Size of the longest file name. */
>>  static size_t gcov_max_filename = 0;
>>
>> @@ -323,8 +325,6 @@ gcov_counter_active (const struct gcov_info *info,
>>    return (info->merge[type] != 0);
>>  }
>>
>> -#ifndef __GCOV_KERNEL__
>> -
>>  /* Add an unsigned value to the current crc */
>>
>>  static gcov_unsigned_t
>> @@ -344,6 +344,8 @@ crc32_unsigned (gcov_unsigned_t crc32, gcov_unsign
>>    return crc32;
>>  }
>>
>> +#ifndef __GCOV_KERNEL__
>> +
>>  /* Check if VERSION of the info block PTR matches libgcov one.
>>     Return 1 on success, or zero in case of versions mismatch.
>>     If FILENAME is not NULL, its value used for reporting purposes
>> @@ -464,6 +466,8 @@ gcov_alloc_filename (void)
>>    gi_filename_up = gi_filename + prefix_length;
>>  }
>>
>> +#endif /* __GCOV_KERNEL__ */
>> +
>>  /* Sort N entries in VALUE_ARRAY in descending order.
>>     Each entry in VALUE_ARRAY has two values. The sorting
>>     is based on the second value.  */
>> @@ -509,6 +513,8 @@ gcov_sort_icall_topn_counter (const struct gcov_ct
>>      }
>>  }
>>
>> +#ifndef __GCOV_KERNEL__
>> +
>>  /* Write imported files (auxiliary modules) for primary module GI_PTR
>>     into file GI_FILENAME.  */
>>
>> @@ -586,6 +592,8 @@ gcov_dump_module_info (void)
>>    __gcov_finalize_dyn_callgraph ();
>>  }
>>
>> +#endif /* __GCOV_KERNEL__ */
>> +
>>  /* Insert counter VALUE into HISTOGRAM.  */
>>
>>  static void
>> @@ -656,6 +664,8 @@ gcov_compute_histogram (struct gcov_summary *sum)
>>      }
>>  }
>>
>> +#ifndef __GCOV_KERNEL__
>> +
>>  /* Dump the coverage counts. We merge with existing counts when
>>     possible, to avoid growing the .da files ad infinitum. We use this
>>     program's checksum to make sure we only accumulate whole program
>> @@ -1013,12 +1023,7 @@ gcov_merge_gcda_file (struct gcov_info *gi_ptr)
>>               goto read_error;
>>         }
>>       if (tag && tag != GCOV_TAG_MODULE_INFO)
>> -       {
>> -         read_mismatch:;
>> -        fprintf (stderr, "profiling:%s:Merge mismatch for %s\n",
>> -                 gi_filename, f_ix + 1 ? "function" : "summaries");
>> -         goto read_fatal;
>> -       }
>> +       goto read_mismatch;
>>      }
>>    goto rewrite;
>>
>> @@ -1031,6 +1036,11 @@ read_error:;
>>
>>      goto rewrite;
>>
>> +read_mismatch:;
>> +    gcov_error ("profiling:%s:Merge mismatch for %s\n",
>> +            gi_filename, f_ix + 1 ? "function" : "summaries");
>> +    goto read_fatal;
>> +
>>  read_fatal:;
>>      gcov_close ();
>>      return 1;
>> @@ -1076,7 +1086,7 @@ rewrite:;
>>                                sizeof (*cs_all) - (sizeof (gcov_bucket_type)
>>                                                    * GCOV_HISTOGRAM_SIZE)))
>>            {
>> -            fprintf (stderr, "profiling:%s:Invocation mismatch - "
>> +            gcov_error ("profiling:%s:Invocation mismatch - "
>>                  "some data files may have been removed%s\n",
>>              gi_filename, GCOV_LOCKED
>>              ? "" : " or concurrent update without locking support");
>> @@ -1093,14 +1103,14 @@ rewrite:;
>>     the size is in units of gcov_type.  */
>>
>>  GCOV_LINKAGE unsigned
>> -gcov_gcda_file_size (struct gcov_info *gi_ptr,
>> -                     struct gcov_summary *sum)
>> +gcov_gcda_file_size (struct gcov_info *gi_ptr)
>>  {
>>    unsigned size;
>>    const struct gcov_fn_info *fi_ptr;
>>    unsigned f_ix, t_ix, h_ix, h_cnt = 0;
>>    unsigned n_counts;
>>    const struct gcov_ctr_info *ci_ptr;
>> +  struct gcov_summary *sum = &this_program;
>>    const struct gcov_ctr_summary *csum;
>>
>>    /* GCOV_DATA_MAGIC, GCOV_VERSION and time_stamp.  */
>> @@ -1181,13 +1191,14 @@ gcov_write_gcda_file (struct gcov_info *gi_ptr)
>>        ci_ptr = gfi_ptr->ctrs;
>>        for (t_ix = 0; t_ix < GCOV_COUNTERS; t_ix++)
>>          {
>> +          gcov_type *c_ptr;
>>            if (!gi_ptr->merge[t_ix])
>>              continue;
>>
>>            n_counts = ci_ptr->num;
>>            gcov_write_tag_length (GCOV_TAG_FOR_COUNTER (t_ix),
>>                                   GCOV_TAG_COUNTER_LENGTH (n_counts));
>> -          gcov_type *c_ptr = ci_ptr->values;
>> +          c_ptr = ci_ptr->values;
>>            while (n_counts--)
>>              gcov_write_counter (*c_ptr++);
>>            ci_ptr++;
>> @@ -1692,7 +1703,7 @@ __gcov_topn_value_profiler_body (gcov_type *counte
>>       {
>>         unsigned i, j;
>>         gcov_type *p, minv;
>> -       gcov_type* tmp_cnts
>> +       gcov_type* tmp_cnts
>>             = (gcov_type *)alloca (topn_val * sizeof(gcov_type));
>>
>>         *num_eviction = 0;
>> @@ -2050,15 +2061,20 @@ gcov_set_vfile (gcov_kernel_vfile *file)
>>    gcov_current_file = file;
>>  }
>>
>> +/* Init function before dumping the gcda file in kernel.  */
>> +
>> +void
>> +gcov_kernel_dump_gcov_init (void)
>> +{
>> +  gcov_exit_init ();
>> +}
>> +
>>  /* Dump one entry in the gcov_info list (for one object) in kernel.  */
>>
>>  void
>>  gcov_kernel_dump_one_gcov (struct gcov_info *info)
>>  {
>>    gcc_assert (gcov_current_file);
>> -
>> -  gcov_exit_init ();
>> -
>>    gcov_dump_one_gcov (info);
>>  }
>>
>> Index: gcc/gcov-io.c
>> ===================================================================
>> --- gcc/gcov-io.c       (revision 194562)
>> +++ gcc/gcov-io.c       (working copy)
>> @@ -662,6 +662,30 @@ gcov_read_string (void)
>>    return (const char *) gcov_read_words (length);
>>  }
>>
>> +#ifdef __GCOV_KERNEL__
>> +static const unsigned char __popcount_tab[256] =
>> +{
>> +    0,1,1,2,1,2,2,3,1,2,2,3,2,3,3,4,1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,
>> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
>> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
>> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
>> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
>> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
>> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
>> +    3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,4,5,5,6,5,6,6,7,5,6,6,7,6,7,7,8
>> +};
>> +
>> +static int k_popcountll (long long x)
>> +{
>> +  int i, ret = 0;
>> +
>> +  for (i = 0; i < 8*sizeof(long long) ; i += 8)
>> +    ret += __popcount_tab[(x >> i) & 0xff];
>> +
>> +  return ret;
>> +}
>> +#endif
>> +
>>  GCOV_LINKAGE void
>>  gcov_read_summary (struct gcov_summary *summary)
>>  {
>> @@ -683,7 +707,11 @@ gcov_read_summary (struct gcov_summary *summary)
>>        for (bv_ix = 0; bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE; bv_ix++)
>>          {
>>            histo_bitvector[bv_ix] = gcov_read_unsigned ();
>> +#ifndef __GCOV_KERNEL__
>>            h_cnt += __builtin_popcountll (histo_bitvector[bv_ix]);
>> +#else
>> +          h_cnt += k_popcountll (histo_bitvector[bv_ix]);
>> +#endif
>
> Is the issue here that __builtin_popcountll is not available? I just
> committed a fix to trunk to deal with a similar issue, can you see if
> this would address the problem here too?
>
> http://gcc.gnu.org/ml/gcc-bugs/2012-12/msg01976.html
>
> Thanks,
> Teresa
>
>>          }
>>        bv_ix = 0;
>>        h_ix = 0;
>> @@ -1137,7 +1165,6 @@ kernel_file_fclose (gcov_kernel_vfile *fp)
>>  long
>>  kernel_file_ftell (gcov_kernel_vfile *fp)
>>  {
>> -  gcc_assert (0);  /* should not reach here */
>>    return 0;
>>  }
>>
>> @@ -1192,8 +1219,9 @@ kernel_file_fwrite (const void *ptr, size_t size,
>>    if (vsize <= vpos)
>>      {
>>        printk (KERN_ERR
>> -         "GCOV_KERNEL: something wrong: vbuf=%p vsize=%u vpos=%u\n",
>> -          vbuf, vsize, vpos);
>> +         "GCOV_KERNEL: something wrong in file %s: vbuf=%p vsize=%u"
>> +         " vpos=%u\n",
>> +          fp->info->filename, vbuf, vsize, vpos);
>>        return 0;
>>      }
>>    len = vsize - vpos;
>> @@ -1207,8 +1235,9 @@ kernel_file_fwrite (const void *ptr, size_t size,
>>
>>    if (len != nitems)
>>      printk (KERN_ERR
>> -        "GCOV_KERNEL: something wrong: size=%lu nitems=%lu ret=%d\n",
>> -        size, nitems, len);
>> +        "GCOV_KERNEL: something wrong in file %s: size=%lu nitems=%lu"
>> +        " ret=%d, vsize=%u vpos=%u \n",
>> +        fp->info->filename, size, nitems, len, vsize, vpos);
>>    return len;
>>  }
>>
>> Index: gcc/gcov-io.h
>> ===================================================================
>> --- gcc/gcov-io.h       (revision 194562)
>> +++ gcc/gcov-io.h       (working copy)
>> @@ -300,6 +300,14 @@ typedef unsigned gcov_type_unsigned __attribute__
>>
>>  #endif  /* BITS_PER_UNIT == 8  */
>>
>> +#if LONG_LONG_TYPE_SIZE > 32
>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_8
>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_8
>> +#else
>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_4
>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_4
>> +#endif
>> +
>>  #undef EXTRACT_MODULE_ID_FROM_GLOBAL_ID
>>  #undef EXTRACT_FUNC_ID_FROM_GLOBAL_ID
>>  #undef GEN_FUNC_GLOBAL_ID
>> @@ -322,6 +330,18 @@ typedef unsigned gcov_type_unsigned __attribute__
>>  typedef unsigned gcov_unsigned_t;
>>  typedef unsigned gcov_position_t;
>>
>> +#if LONG_LONG_TYPE_SIZE > 32
>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_8
>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_8
>> +#else
>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_4
>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_4
>> +#endif
>> +#define PROFILE_GEN_EDGE_ATOMIC (flag_profile_gen_atomic == 1 || \
>> +                                 flag_profile_gen_atomic == 3)
>> +#define PROFILE_GEN_VALUE_ATOMIC (flag_profile_gen_atomic == 2 || \
>> +                                  flag_profile_gen_atomic == 3)
>> +
>>  /* gcov_type is typedef'd elsewhere for the compiler */
>>  #if IN_GCOV
>>  #define GCOV_LINKAGE static
>> @@ -781,8 +801,8 @@ struct gcov_info
>>                                           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 +1008,7 @@ static void gcov_rewrite (void);
>>  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*/,
>>
>> --
>> This patch is available for review at http://codereview.appspot.com/6968046
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

tejohnson

On Fri, Dec 21, 2012 at 9:52 AM, Xinliang David Li <davidxl@google.com> wrote: > Kernel ...

11 years, 4 months ago (2012-12-21 17:55:53 UTC) #10

On Fri, Dec 21, 2012 at 9:52 AM, Xinliang David Li <davidxl@google.com> wrote:
> Kernel build does not link in libgcc, which defines the function.

Then will the same issue occur with the reference to __builtin_clzll
in gcov_histo_index in gcov-io.c?

Teresa

>
> David
>
> On Fri, Dec 21, 2012 at 8:31 AM, Teresa Johnson <tejohnson@google.com> wrote:
>> On Wed, Dec 19, 2012 at 12:11 PM, Rong Xu <xur@google.com> wrote:
>>> Hi,
>>>
>>> This patch updates the support for FDO build in linux kernel for gcc 4.7.
>>> Tested with 2.6.34 kernel and google internal benchmarks.
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>> 2012-12-19  Rong Xu  <xur@google.com>
>>>
>>>         * libgcc/libgcov.c
>>>         (gcov_counter_active): v4.7 kernel fdo support.
>>>         (crc32_unsigned): Include in GCOV_KERNEL build.
>>>         (gcov_alloc_filename): Remove from GCOV_KERNEL build.
>>>         (gcov_sort_icall_topn_counter): Ditto.
>>>         (gcov_dump_module_info): Ditto.
>>>         (gcov_compute_histogram): v4.7 kernel fdo support.
>>>         (gcov_merge_gcda_file): Ditto.
>>>         (gcov_gcda_file_size): Ditto.
>>>         (gcov_write_gcda_file): Ditto.
>>>         (__gcov_topn_value_profiler_body): Ditto.
>>>         (gcov_set_vfile): Ditto.
>>>         (gcov_kernel_dump_gcov_init): Ditto.
>>>         (gcov_kernel_dump_one_gcov): Ditto.
>>>         * gcc/gcov-io.c (gcov_read_string): Ditto.
>>>         (static int k_popcountll): Ditto.
>>>         (gcov_read_summary): Ditto.
>>>         (kernel_file_fclose): Ditto.
>>>         (kernel_file_ftell): Ditto.
>>>         (kernel_file_fwrite): Ditto.
>>>         * gcc/gcov-io.h (struct gcov_info): remove const keyword.
>>>
>>> Index: libgcc/libgcov.c
>>> ===================================================================
>>> --- libgcc/libgcov.c    (revision 194562)
>>> +++ libgcc/libgcov.c    (working copy)
>>> @@ -46,11 +46,11 @@ see the files COPYING3 and COPYING.RUNTIME respect
>>>  #include "tsystem.h"
>>>  #include "coretypes.h"
>>>  #include "tm.h"
>>> -#endif /* __KERNEL__ */
>>>  #include "libgcc_tm.h"
>>>  #include "gthr.h"
>>> +#endif /* __KERNEL__ */
>>>
>>> -#if 1
>>> +#ifndef __KERNEL__
>>>  #define THREAD_PREFIX __thread
>>>  #else
>>>  #define THREAD_PREFIX
>>> @@ -120,6 +120,7 @@ extern int gcov_dump_complete ATTRIBUTE_HIDDEN;
>>>  #ifdef L_gcov
>>>  #include "gcov-io.c"
>>>
>>> +#ifndef __GCOV_KERNEL__
>>>  /* Create a strong reference to these symbols so that they are
>>>     unconditionally pulled into the instrumented binary, even when
>>>     the only reference is a weak reference. This is necessary because
>>> @@ -134,6 +135,7 @@ extern int gcov_dump_complete ATTRIBUTE_HIDDEN;
>>>     these symbols will always need to be resolved.  */
>>>  void (*__gcov_dummy_ref1)() = &__gcov_reset;
>>>  void (*__gcov_dummy_ref2)() = &__gcov_dump;
>>> +#endif /* __GCOV_KERNEL__ */
>>>
>>>  /* Utility function for outputing errors.  */
>>>  static int
>>> @@ -151,6 +153,10 @@ gcov_error (const char *fmt, ...)
>>>    return ret;
>>>  }
>>>
>>> +/* A program checksum allows us to distinguish program data for an
>>> +   object file included in multiple programs.  */
>>> +static gcov_unsigned_t gcov_crc32;
>>> +
>>>  #ifndef __GCOV_KERNEL__
>>>  /* Emitted in coverage.c.  */
>>>  extern char * __gcov_pmu_profile_filename;
>>> @@ -183,10 +189,6 @@ THREAD_PREFIX gcov_unsigned_t __gcov_sample_counte
>>>  /* Chain of per-object gcov structures.  */
>>>  extern struct gcov_info *__gcov_list;
>>>
>>> -/* A program checksum allows us to distinguish program data for an
>>> -   object file included in multiple programs.  */
>>> -static gcov_unsigned_t gcov_crc32;
>>> -
>>>  /* Size of the longest file name. */
>>>  static size_t gcov_max_filename = 0;
>>>
>>> @@ -323,8 +325,6 @@ gcov_counter_active (const struct gcov_info *info,
>>>    return (info->merge[type] != 0);
>>>  }
>>>
>>> -#ifndef __GCOV_KERNEL__
>>> -
>>>  /* Add an unsigned value to the current crc */
>>>
>>>  static gcov_unsigned_t
>>> @@ -344,6 +344,8 @@ crc32_unsigned (gcov_unsigned_t crc32, gcov_unsign
>>>    return crc32;
>>>  }
>>>
>>> +#ifndef __GCOV_KERNEL__
>>> +
>>>  /* Check if VERSION of the info block PTR matches libgcov one.
>>>     Return 1 on success, or zero in case of versions mismatch.
>>>     If FILENAME is not NULL, its value used for reporting purposes
>>> @@ -464,6 +466,8 @@ gcov_alloc_filename (void)
>>>    gi_filename_up = gi_filename + prefix_length;
>>>  }
>>>
>>> +#endif /* __GCOV_KERNEL__ */
>>> +
>>>  /* Sort N entries in VALUE_ARRAY in descending order.
>>>     Each entry in VALUE_ARRAY has two values. The sorting
>>>     is based on the second value.  */
>>> @@ -509,6 +513,8 @@ gcov_sort_icall_topn_counter (const struct gcov_ct
>>>      }
>>>  }
>>>
>>> +#ifndef __GCOV_KERNEL__
>>> +
>>>  /* Write imported files (auxiliary modules) for primary module GI_PTR
>>>     into file GI_FILENAME.  */
>>>
>>> @@ -586,6 +592,8 @@ gcov_dump_module_info (void)
>>>    __gcov_finalize_dyn_callgraph ();
>>>  }
>>>
>>> +#endif /* __GCOV_KERNEL__ */
>>> +
>>>  /* Insert counter VALUE into HISTOGRAM.  */
>>>
>>>  static void
>>> @@ -656,6 +664,8 @@ gcov_compute_histogram (struct gcov_summary *sum)
>>>      }
>>>  }
>>>
>>> +#ifndef __GCOV_KERNEL__
>>> +
>>>  /* Dump the coverage counts. We merge with existing counts when
>>>     possible, to avoid growing the .da files ad infinitum. We use this
>>>     program's checksum to make sure we only accumulate whole program
>>> @@ -1013,12 +1023,7 @@ gcov_merge_gcda_file (struct gcov_info *gi_ptr)
>>>               goto read_error;
>>>         }
>>>       if (tag && tag != GCOV_TAG_MODULE_INFO)
>>> -       {
>>> -         read_mismatch:;
>>> -        fprintf (stderr, "profiling:%s:Merge mismatch for %s\n",
>>> -                 gi_filename, f_ix + 1 ? "function" : "summaries");
>>> -         goto read_fatal;
>>> -       }
>>> +       goto read_mismatch;
>>>      }
>>>    goto rewrite;
>>>
>>> @@ -1031,6 +1036,11 @@ read_error:;
>>>
>>>      goto rewrite;
>>>
>>> +read_mismatch:;
>>> +    gcov_error ("profiling:%s:Merge mismatch for %s\n",
>>> +            gi_filename, f_ix + 1 ? "function" : "summaries");
>>> +    goto read_fatal;
>>> +
>>>  read_fatal:;
>>>      gcov_close ();
>>>      return 1;
>>> @@ -1076,7 +1086,7 @@ rewrite:;
>>>                                sizeof (*cs_all) - (sizeof
(gcov_bucket_type)
>>>                                                    * GCOV_HISTOGRAM_SIZE)))
>>>            {
>>> -            fprintf (stderr, "profiling:%s:Invocation mismatch - "
>>> +            gcov_error ("profiling:%s:Invocation mismatch - "
>>>                  "some data files may have been removed%s\n",
>>>              gi_filename, GCOV_LOCKED
>>>              ? "" : " or concurrent update without locking support");
>>> @@ -1093,14 +1103,14 @@ rewrite:;
>>>     the size is in units of gcov_type.  */
>>>
>>>  GCOV_LINKAGE unsigned
>>> -gcov_gcda_file_size (struct gcov_info *gi_ptr,
>>> -                     struct gcov_summary *sum)
>>> +gcov_gcda_file_size (struct gcov_info *gi_ptr)
>>>  {
>>>    unsigned size;
>>>    const struct gcov_fn_info *fi_ptr;
>>>    unsigned f_ix, t_ix, h_ix, h_cnt = 0;
>>>    unsigned n_counts;
>>>    const struct gcov_ctr_info *ci_ptr;
>>> +  struct gcov_summary *sum = &this_program;
>>>    const struct gcov_ctr_summary *csum;
>>>
>>>    /* GCOV_DATA_MAGIC, GCOV_VERSION and time_stamp.  */
>>> @@ -1181,13 +1191,14 @@ gcov_write_gcda_file (struct gcov_info *gi_ptr)
>>>        ci_ptr = gfi_ptr->ctrs;
>>>        for (t_ix = 0; t_ix < GCOV_COUNTERS; t_ix++)
>>>          {
>>> +          gcov_type *c_ptr;
>>>            if (!gi_ptr->merge[t_ix])
>>>              continue;
>>>
>>>            n_counts = ci_ptr->num;
>>>            gcov_write_tag_length (GCOV_TAG_FOR_COUNTER (t_ix),
>>>                                   GCOV_TAG_COUNTER_LENGTH (n_counts));
>>> -          gcov_type *c_ptr = ci_ptr->values;
>>> +          c_ptr = ci_ptr->values;
>>>            while (n_counts--)
>>>              gcov_write_counter (*c_ptr++);
>>>            ci_ptr++;
>>> @@ -1692,7 +1703,7 @@ __gcov_topn_value_profiler_body (gcov_type *counte
>>>       {
>>>         unsigned i, j;
>>>         gcov_type *p, minv;
>>> -       gcov_type* tmp_cnts
>>> +       gcov_type* tmp_cnts
>>>             = (gcov_type *)alloca (topn_val * sizeof(gcov_type));
>>>
>>>         *num_eviction = 0;
>>> @@ -2050,15 +2061,20 @@ gcov_set_vfile (gcov_kernel_vfile *file)
>>>    gcov_current_file = file;
>>>  }
>>>
>>> +/* Init function before dumping the gcda file in kernel.  */
>>> +
>>> +void
>>> +gcov_kernel_dump_gcov_init (void)
>>> +{
>>> +  gcov_exit_init ();
>>> +}
>>> +
>>>  /* Dump one entry in the gcov_info list (for one object) in kernel.  */
>>>
>>>  void
>>>  gcov_kernel_dump_one_gcov (struct gcov_info *info)
>>>  {
>>>    gcc_assert (gcov_current_file);
>>> -
>>> -  gcov_exit_init ();
>>> -
>>>    gcov_dump_one_gcov (info);
>>>  }
>>>
>>> Index: gcc/gcov-io.c
>>> ===================================================================
>>> --- gcc/gcov-io.c       (revision 194562)
>>> +++ gcc/gcov-io.c       (working copy)
>>> @@ -662,6 +662,30 @@ gcov_read_string (void)
>>>    return (const char *) gcov_read_words (length);
>>>  }
>>>
>>> +#ifdef __GCOV_KERNEL__
>>> +static const unsigned char __popcount_tab[256] =
>>> +{
>>> +    0,1,1,2,1,2,2,3,1,2,2,3,2,3,3,4,1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,
>>> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
>>> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
>>> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
>>> +    1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,
>>> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
>>> +    2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,
>>> +    3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,4,5,5,6,5,6,6,7,5,6,6,7,6,7,7,8
>>> +};
>>> +
>>> +static int k_popcountll (long long x)
>>> +{
>>> +  int i, ret = 0;
>>> +
>>> +  for (i = 0; i < 8*sizeof(long long) ; i += 8)
>>> +    ret += __popcount_tab[(x >> i) & 0xff];
>>> +
>>> +  return ret;
>>> +}
>>> +#endif
>>> +
>>>  GCOV_LINKAGE void
>>>  gcov_read_summary (struct gcov_summary *summary)
>>>  {
>>> @@ -683,7 +707,11 @@ gcov_read_summary (struct gcov_summary *summary)
>>>        for (bv_ix = 0; bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE; bv_ix++)
>>>          {
>>>            histo_bitvector[bv_ix] = gcov_read_unsigned ();
>>> +#ifndef __GCOV_KERNEL__
>>>            h_cnt += __builtin_popcountll (histo_bitvector[bv_ix]);
>>> +#else
>>> +          h_cnt += k_popcountll (histo_bitvector[bv_ix]);
>>> +#endif
>>
>> Is the issue here that __builtin_popcountll is not available? I just
>> committed a fix to trunk to deal with a similar issue, can you see if
>> this would address the problem here too?
>>
>> http://gcc.gnu.org/ml/gcc-bugs/2012-12/msg01976.html
>>
>> Thanks,
>> Teresa
>>
>>>          }
>>>        bv_ix = 0;
>>>        h_ix = 0;
>>> @@ -1137,7 +1165,6 @@ kernel_file_fclose (gcov_kernel_vfile *fp)
>>>  long
>>>  kernel_file_ftell (gcov_kernel_vfile *fp)
>>>  {
>>> -  gcc_assert (0);  /* should not reach here */
>>>    return 0;
>>>  }
>>>
>>> @@ -1192,8 +1219,9 @@ kernel_file_fwrite (const void *ptr, size_t size,
>>>    if (vsize <= vpos)
>>>      {
>>>        printk (KERN_ERR
>>> -         "GCOV_KERNEL: something wrong: vbuf=%p vsize=%u vpos=%u\n",
>>> -          vbuf, vsize, vpos);
>>> +         "GCOV_KERNEL: something wrong in file %s: vbuf=%p vsize=%u"
>>> +         " vpos=%u\n",
>>> +          fp->info->filename, vbuf, vsize, vpos);
>>>        return 0;
>>>      }
>>>    len = vsize - vpos;
>>> @@ -1207,8 +1235,9 @@ kernel_file_fwrite (const void *ptr, size_t size,
>>>
>>>    if (len != nitems)
>>>      printk (KERN_ERR
>>> -        "GCOV_KERNEL: something wrong: size=%lu nitems=%lu ret=%d\n",
>>> -        size, nitems, len);
>>> +        "GCOV_KERNEL: something wrong in file %s: size=%lu nitems=%lu"
>>> +        " ret=%d, vsize=%u vpos=%u \n",
>>> +        fp->info->filename, size, nitems, len, vsize, vpos);
>>>    return len;
>>>  }
>>>
>>> Index: gcc/gcov-io.h
>>> ===================================================================
>>> --- gcc/gcov-io.h       (revision 194562)
>>> +++ gcc/gcov-io.h       (working copy)
>>> @@ -300,6 +300,14 @@ typedef unsigned gcov_type_unsigned __attribute__
>>>
>>>  #endif  /* BITS_PER_UNIT == 8  */
>>>
>>> +#if LONG_LONG_TYPE_SIZE > 32
>>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_8
>>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_8
>>> +#else
>>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_4
>>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_4
>>> +#endif
>>> +
>>>  #undef EXTRACT_MODULE_ID_FROM_GLOBAL_ID
>>>  #undef EXTRACT_FUNC_ID_FROM_GLOBAL_ID
>>>  #undef GEN_FUNC_GLOBAL_ID
>>> @@ -322,6 +330,18 @@ typedef unsigned gcov_type_unsigned __attribute__
>>>  typedef unsigned gcov_unsigned_t;
>>>  typedef unsigned gcov_position_t;
>>>
>>> +#if LONG_LONG_TYPE_SIZE > 32
>>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_8
>>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_8
>>> +#else
>>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD_FN __sync_fetch_and_add_4
>>> +#define GCOV_TYPE_SYNC_FETCH_AND_ADD BUILT_IN_SYNC_FETCH_AND_ADD_4
>>> +#endif
>>> +#define PROFILE_GEN_EDGE_ATOMIC (flag_profile_gen_atomic == 1 || \
>>> +                                 flag_profile_gen_atomic == 3)
>>> +#define PROFILE_GEN_VALUE_ATOMIC (flag_profile_gen_atomic == 2 || \
>>> +                                  flag_profile_gen_atomic == 3)
>>> +
>>>  /* gcov_type is typedef'd elsewhere for the compiler */
>>>  #if IN_GCOV
>>>  #define GCOV_LINKAGE static
>>> @@ -781,8 +801,8 @@ struct gcov_info
>>>                                           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 +1008,7 @@ static void gcov_rewrite (void);
>>>  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*/,
>>>
>>> --
>>> This patch is available for review at http://codereview.appspot.com/6968046
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

xur

11 years, 4 months ago (2012-12-21 19:57:57 UTC) #11

that's in the histrogram merge. we disable the merge in kernel fdo
(instead, we do offline merge).

-Rong

On Fri, Dec 21, 2012 at 9:55 AM, Teresa Johnson <tejohnson@google.com> wrote:
> clzll

Expand All Messages | Collapse All Messages