> On 6/3/20 8:28 AM, Martin Liška wrote:
> > On 6/2/20 3:19 PM, Martin Liška wrote:
> > > I'm suggesting to pre-allocate 16 gcov_kvp in the gcov run-time library.
> > > Please take a look at the attached patch. I also added a test-case that
> > > stresses that. I've just finished LTO PGO bootstrap of the GCC.
> > >
> > > Ready for master?
> >
> > There's V2 of the patch:
> >
> > - guard __atomic_fetch_add in GCOV_SUPPORTS_ATOMIC
> >
> > Martin
>
> There's V3 of the patch:
> - TLS is used for in_recursion variable
> - when we run out of statically allocated buffers, do not bail out
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> diff --git a/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
> b/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
> new file mode 100644
> index 00000000000..454e224c95f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
> @@ -0,0 +1,49 @@
> +/* { dg-options "-O2 -ldl" } */
I think this needs to be restricted to targets that have dl library....
Otherwise the patch looks good to me.
We may try to add the testcase doing division by all possible integers
in two threads or so to stress the race conditions in link list
management.
Thank you,
Honza
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <dlfcn.h>
> +
> +int global;
> +int global2;
> +
> +void report1 (size_t size)
> +{
> + global++;
> +}
> +
> +void report2 (size_t size)
> +{
> + global2++;
> +}
> +
> +typedef void (*tp) (size_t);
> +static tp fns[] = {report1, report2};
> +
> +void* malloc(size_t size)
> +{
> + static void* (*real_malloc)(size_t) = NULL;
> + if (!real_malloc)
> + real_malloc = dlsym(RTLD_NEXT, "malloc");
> +
> + void *p = real_malloc (size);
> + fns[size % 2] (size);
> + // fprintf(stderr, "malloc(%d) = %p\n", size, p);
> + return p;
> +}
> +
> +void *calloc (size_t n, size_t size)
> +{
> + void *ptr = malloc (n * size);
> + __builtin_memset (ptr, 0, n * size);
> + return ptr;
> +}
> +
> +void *ptr;
> +
> +int main()
> +{
> + ptr = malloc (16);
> + return 0;
> +}
> diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
> index 2590593a58a..58914268d4e 100644
> --- a/libgcc/libgcov-driver.c
> +++ b/libgcc/libgcov-driver.c
> @@ -588,6 +588,12 @@ struct gcov_root __gcov_root;
> struct gcov_master __gcov_master =
> {GCOV_VERSION, 0};
>
> +/* Pool of pre-allocated gcov_kvp strutures. */
> +struct gcov_kvp __gcov_kvp_pool[GCOV_PREALLOCATED_KVP];
> +
> +/* Index to first free gcov_kvp in the pool. */
> +unsigned __gcov_kvp_pool_index;
> +
> void
> __gcov_exit (void)
> {
> diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
> index 81e18950a50..8be5bebcac0 100644
> --- a/libgcc/libgcov.h
> +++ b/libgcc/libgcov.h
> @@ -250,6 +250,8 @@ struct indirect_call_tuple
>
> /* Exactly one of these will be active in the process. */
> extern struct gcov_master __gcov_master;
> +extern struct gcov_kvp __gcov_kvp_pool[GCOV_PREALLOCATED_KVP];
> +extern unsigned __gcov_kvp_pool_index;
>
> /* Dump a set of gcov objects. */
> extern void __gcov_dump_one (struct gcov_root *) ATTRIBUTE_HIDDEN;
> @@ -402,6 +404,47 @@ gcov_counter_add (gcov_type *counter, gcov_type value,
> *counter += value;
> }
>
> +/* Allocate gcov_kvp from heap. If we are recursively called, then allocate
> + it from a list of pre-allocated pool. */
> +
> +static inline struct gcov_kvp *
> +allocate_gcov_kvp (void)
> +{
> + struct gcov_kvp *new_node = NULL;
> +
> + static
> +#if defined(HAVE_CC_TLS)
> +__thread
> +#endif
> + volatile unsigned in_recursion ATTRIBUTE_UNUSED = 0;
> +
> +#if !defined(IN_GCOV_TOOL) && !defined(L_gcov_merge_topn)
> + if (__builtin_expect (in_recursion, 0))
> + {
> + unsigned index;
> +#if GCOV_SUPPORTS_ATOMIC
> + index
> + = __atomic_fetch_add (&__gcov_kvp_pool_index, 1, __ATOMIC_RELAXED);
> +#else
> + index = __gcov_kvp_pool_index++;
> +#endif
> + if (index < GCOV_PREALLOCATED_KVP)
> + new_node = &__gcov_kvp_pool[index];
> + else
> + /* Do not crash in the situation. */
> + return NULL;
> + }
> + else
> +#endif
> + {
> + in_recursion = 1;
> + new_node = (struct gcov_kvp *)xcalloc (1, sizeof (struct gcov_kvp));
> + in_recursion = 0;
> + }
> +
> + return new_node;
> +}
> +
> /* Add key value pair VALUE:COUNT to a top N COUNTERS. When INCREMENT_TOTAL
> is true, add COUNT to total of the TOP counter. If USE_ATOMIC is true,
> do it in atomic way. */
> @@ -443,8 +486,10 @@ gcov_topn_add_value (gcov_type *counters, gcov_type
> value, gcov_type count,
> }
> else
> {
> - struct gcov_kvp *new_node
> - = (struct gcov_kvp *)xcalloc (1, sizeof (struct gcov_kvp));
> + struct gcov_kvp *new_node = allocate_gcov_kvp ();
> + if (new_node == NULL)
> + return;
> +
> new_node->value = value;
> new_node->count = count;
>
> --
> 2.27.0
>
