On 6/2/20 10:27 AM, Jan Hubicka wrote:
The patch looks good (and is OK for mainline). I am bit concerned about
two things.
Hello.
Thank you for the review!
1) I think we should add support to pre-allocate memory pool so we hide
the problem with instrumenting malloc (I think with big enough memory
pool the Firefox malloc issue should disappear since we populate the
counters used to profile malloc before we run out of it).
I think this can be done by adding comdat symbol so one can specify
size of the pool at compile time.
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?
2) This seems like bit too big hammer for profiling values for division
etc. We will see how perfromance will go - in worst case I guess we
could put back the original counter TOPN counter and use it for those
cases.
We'll see about this and we can possibly reduce number of tracked values
for this kind of profiling.
Thanks,
Martin
>From 12e0fe1be6490a90d45c9a20a98c096ba6c03e07 Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Tue, 2 Jun 2020 13:31:48 +0200
Subject: [PATCH] libgcov: support overloaded malloc
gcc/ChangeLog:
* gcov-io.h (GCOV_PREALLOCATED_KVP): New.
* gcov-tool.c: Add __gcov_kvp_pool
and __gcov_kvp_pool_index variables.
libgcc/ChangeLog:
* libgcov-driver.c: Add __gcov_kvp_pool
and __gcov_kvp_pool_index variables.
* libgcov.h (allocate_gcov_kvp): New.
(gcov_topn_add_value): Use it.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-prof/indir-call-prof-malloc.c: New test.
---
gcc/gcov-io.h | 3 ++
.../gcc.dg/tree-prof/indir-call-prof-malloc.c | 49 +++++++++++++++++++
libgcc/libgcov-driver.c | 6 +++
libgcc/libgcov.h | 35 ++++++++++++-
4 files changed, 91 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index 940eb7d8561..4dba01c78ce 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -292,6 +292,9 @@ GCOV_COUNTERS
/* Maximum number of tracked TOP N value profiles. */
#define GCOV_TOPN_MAXIMUM_TRACKED_VALUES 32
+/* Number of pre-allocated gcov_kvp structures. */
+#define GCOV_PREALLOCATED_KVP 16
+
/* Convert a counter index to a tag. */
#define GCOV_TAG_FOR_COUNTER(COUNT) \
(GCOV_TAG_COUNTER_BASE + ((gcov_unsigned_t)(COUNT) << 17))
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" } */
+
+#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 8348d9f33ec..f2383e9d198 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -576,6 +576,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 7c037a97878..0c3f46e9a04 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -236,6 +236,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;
@@ -401,6 +403,36 @@ gcov_counter_set_if_null (gcov_type *counter, struct gcov_kvp *node,
}
}
+/* 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 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
+ = __atomic_fetch_add (&__gcov_kvp_pool_index, 1, __ATOMIC_RELAXED);
+ if (index < GCOV_PREALLOCATED_KVP)
+ new_node = &__gcov_kvp_pool[index];
+ else
+ gcc_unreachable ();
+ }
+ 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. */
@@ -442,8 +474,7 @@ 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 ();
new_node->value = value;
new_node->count = count;
--
2.26.2
