This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/4] Cherry-pick fprofile-generate-atomic from google/gcc-4_9 branch
On 08/01/2016 02:22 PM, Nathan Sidwell wrote:
> As I just wrote, this patch needs work. the general points are:
Thank for the comments.
> 1) exposing integers 0-3 to the user as switch values. Don't do that, give them names. In this case a comma separated list of orthogonal names seems appropriate. But see below.
> 2) Poor documentation. How might the user might choose an appropriate setting? (what happens if compilations need to use different settings). What are 'edge' and 'value' counters. Why might one want different settings for them?
Sure, fully agree that it currently doesn't make sense to distinguish between individual types of profiles (edge, value).
>
> I think this is jumping too deep into a solution with insufficient evidence. Particularly, why two edges and values can be set differently. It doesn't lend itself to extending to TLS, if that proves to be a good solution (trading memory for time). Something along the lines of '-fprofile-update={single,atomic,threaded},[edge,value]' might be better. I.e. set the scheme as part of the option value, followed by a list of the things it applies to. (and as I hope I've implied, it'd be good not to have that separate list until proven otherwise).
Yes.
>
>
> On 07/28/16 08:32, marxin wrote:
>> libgcc/ChangeLog:
>>
>> 2016-07-28 Martin Liska <mliska@suse.cz>
>
> Shouldn't the original authors be named here too? (applies to the other patches too).
Adding a cherry-pick entry to the original commit of the functionality.
>
>
>> --- a/gcc/gcov-io.h
>> +++ b/gcc/gcov-io.h
>> @@ -169,6 +169,19 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
>
>> +
>> +#if LONG_LONG_TYPE_SIZE > 32
>> +#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
>> +#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
>> +#else
>> +#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
>> +#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
>> +#endif
> ...
>> #if IN_LIBGCOV
>> +
>> +#if LONG_LONG_TYPE_SIZE > 32
>> +#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
>> +#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
>> +#else
>> +#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
>> +#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
>> +#endif
>
>
> BTW, these two blocks look stunningly similar.
Fixed.
I also added a small hunk that describes problematic of app having not-joined (or detached) threads,
can you please take a look at documentation change, maybe it would need some transformation?
Martin
>
> nathan
>From 7e19e28f3d6e227bb67fb770575831d637abe3aa Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 28 Jul 2016 14:32:47 +0200
Subject: [PATCH 1/4] Cherry-pick fprofile-generate-atomic from google/gcc-4_9
branch
libgcc/ChangeLog:
2016-07-28 Martin Liska <mliska@suse.cz>
Cherry picked (and modified) from google-4_7 branch
2012-12-26 Rong Xu <xur@google.com>
* Makefile.in: Add functions to LIBGCOV_PROFILER.
* libgcov-profiler.c (__gcov_one_value_profiler_body_atomic):
New function.
(__gcov_one_value_profiler_atomic): Likewise.
(__gcov_indirect_call_profiler_v2): Fix GNU coding style.
(__gcov_indirect_call_profiler_v2_atomic): New function.
* libgcov.h: Declare __gcov_indirect_call_profiler_v2_atomic and
__gcov_one_value_profiler_body_atomic.
gcc/ChangeLog:
2016-07-28 Martin Liska <mliska@suse.cz>
Cherry picked (and modified) from google-4_7 branch
2012-12-26 Rong Xu <xur@google.com>
* common.opt (fprofile-update): Add new flag.
* gcov-io.h: Declare GCOV_TYPE_ATOMIC_FETCH_ADD and
GCOV_TYPE_ATOMIC_FETCH_ADD_FN.
* tree-profile.c (gimple_init_edge_profiler): Generate
also atomic profiler update.
(gimple_gen_edge_profiler): Likewise.
* doc/invoke.texi: Document -fprofile-update.
gcc/testsuite/ChangeLog:
2016-07-28 Martin Liska <mliska@suse.cz>
* g++.dg/gcov/gcov-threads-1.C: New test.
---
gcc/common.opt | 13 +++++++
gcc/coretypes.h | 6 +++
gcc/doc/invoke.texi | 12 ++++++
gcc/gcov-io.h | 8 ++++
gcc/testsuite/g++.dg/gcov/gcov-threads-1.C | 46 ++++++++++++++++++++++
gcc/tree-profile.c | 61 ++++++++++++++++++++----------
libgcc/Makefile.in | 4 +-
libgcc/libgcov-profiler.c | 42 +++++++++++++++++++-
libgcc/libgcov.h | 2 +
9 files changed, 173 insertions(+), 21 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/gcov/gcov-threads-1.C
diff --git a/gcc/common.opt b/gcc/common.opt
index 8a292ed..44adae8 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1916,6 +1916,19 @@ fprofile-correction
Common Report Var(flag_profile_correction)
Enable correction of flow inconsistent profile data input.
+fprofile-update=
+Common Joined RejectNegative Enum(profile_update) Var(flag_profile_update) Init(PROFILE_UPDATE_SINGLE)
+-fprofile-update=[single|atomic] Set the profile update method.
+
+Enum
+Name(profile_update) Type(enum profile_update) UnknownError(unknown profile update method %qs)
+
+EnumValue
+Enum(profile_update) String(single) Value(PROFILE_UPDATE_SINGLE)
+
+EnumValue
+Enum(profile_update) String(atomic) Value(PROFILE_UPDATE_ATOMIC)
+
fprofile-generate
Common
Enable common options for generating profile info for profile feedback directed optimizations.
diff --git a/gcc/coretypes.h b/gcc/coretypes.h
index b3a91a6..fe1e984 100644
--- a/gcc/coretypes.h
+++ b/gcc/coretypes.h
@@ -174,6 +174,12 @@ enum offload_abi {
OFFLOAD_ABI_ILP32
};
+/* Types of profile update methods. */
+enum profile_update {
+ PROFILE_UPDATE_SINGLE,
+ PROFILE_UPDATE_ATOMIC
+};
+
/* Types of unwind/exception handling info that can be generated. */
enum unwind_info_type
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 22001f9..1cfaae7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9933,6 +9933,18 @@ the profile feedback data files. See @option{-fprofile-dir}.
To optimize the program based on the collected profile information, use
@option{-fprofile-use}. @xref{Optimize Options}, for more information.
+@item -fprofile-update=@var{method}
+@opindex fprofile-update
+
+Alter the update method for an application instrumented for profile
+feedback based optimization. The @var{method} argument should be one of
+@samp{single} or @samp{atomic}. The first one is useful for single-threaded
+applications, while the second one prevents profile corruption by emitting
+thread-safe code.
+
+@strong{Warning:} When an application does not properly join all threads
+(or creates an detached thread), a profile file can be still corrupted.
+
@item -fsanitize=address
@opindex fsanitize=address
Enable AddressSanitizer, a fast memory error detector.
diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index bbf013a..afd00ac 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -164,6 +164,14 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
#ifndef GCC_GCOV_IO_H
#define GCC_GCOV_IO_H
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
+#else
+#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
+#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
+#endif
+
#ifndef IN_LIBGCOV
/* About the host */
diff --git a/gcc/testsuite/g++.dg/gcov/gcov-threads-1.C b/gcc/testsuite/g++.dg/gcov/gcov-threads-1.C
new file mode 100644
index 0000000..a4a6f0a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/gcov-threads-1.C
@@ -0,0 +1,46 @@
+/* { dg-options "-fprofile-arcs -ftest-coverage -pthread -fprofile-update=atomic" } */
+/* { dg-do run { target native } } */
+
+#include <stdint.h>
+#include <pthread.h>
+#include <assert.h>
+
+#define NR 5
+
+pthread_mutex_t cndMs[NR];
+static void *ContentionNoDeadlock_thread(void *start)
+{
+ for (uint32_t k = 0; k < 100000; ++k) /* count(500005) */
+ {
+ int starti = *(int*)start; /* count(500000) */
+ for (uint32_t i = starti; i < NR; ++i)
+ pthread_mutex_lock (&cndMs[i]);
+ for (int32_t i = NR - 1; i >= starti; --i)
+ pthread_mutex_unlock (&cndMs[i]);
+ }
+}
+int main(int argc, char **argv) {
+ for (unsigned i = 0; i < NR; i++)
+ cndMs[i] = PTHREAD_MUTEX_INITIALIZER;
+
+ pthread_t t[NR];
+ int ids[NR];
+
+ for (int i = 0; i < NR; i++)
+ {
+ ids[i] = i;
+ int r = pthread_create (&t[i], NULL, ContentionNoDeadlock_thread, &ids[i]);
+ assert (r == 0); /* count(5) */
+ }
+
+ int ret;
+ for (int i = 0; i < NR; i++)
+ {
+ int r = pthread_join (t[i], (void**)&ret);
+ assert (r == 0); /* count(5) */
+ }
+
+ return 0; /* count(1) */
+}
+
+/* { dg-final { run-gcov gcov-threads-1.C } } */
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index 39fe15f..a2c86ac 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -164,7 +164,12 @@ gimple_init_edge_profiler (void)
= build_function_type_list (void_type_node,
gcov_type_ptr, gcov_type_node,
NULL_TREE);
- tree_one_value_profiler_fn
+ if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
+ tree_one_value_profiler_fn
+ = build_fn_decl ("__gcov_one_value_profiler_atomic",
+ one_value_profiler_fn_type);
+ else
+ tree_one_value_profiler_fn
= build_fn_decl ("__gcov_one_value_profiler",
one_value_profiler_fn_type);
TREE_NOTHROW (tree_one_value_profiler_fn) = 1;
@@ -180,11 +185,14 @@ gimple_init_edge_profiler (void)
gcov_type_node,
ptr_void,
NULL_TREE);
+ const char *profiler_fn_name = "__gcov_indirect_call_profiler_v2";
+ if (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE))
+ profiler_fn_name = "__gcov_indirect_call_topn_profiler";
+ if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
+ profiler_fn_name = "__gcov_indirect_call_profiler_v2_atomic";
+
tree_indirect_call_profiler_fn
- = build_fn_decl ( (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
- "__gcov_indirect_call_topn_profiler":
- "__gcov_indirect_call_profiler_v2"),
- ic_profiler_fn_type);
+ = build_fn_decl (profiler_fn_name, ic_profiler_fn_type);
TREE_NOTHROW (tree_indirect_call_profiler_fn) = 1;
DECL_ATTRIBUTES (tree_indirect_call_profiler_fn)
@@ -241,22 +249,37 @@ gimple_init_edge_profiler (void)
void
gimple_gen_edge_profiler (int edgeno, edge e)
{
- tree ref, one, gcov_type_tmp_var;
- gassign *stmt1, *stmt2, *stmt3;
+ tree one;
- ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
one = build_int_cst (gcov_type_node, 1);
- gcov_type_tmp_var = make_temp_ssa_name (gcov_type_node,
- NULL, "PROF_edge_counter");
- stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
- gcov_type_tmp_var = make_temp_ssa_name (gcov_type_node,
- NULL, "PROF_edge_counter");
- stmt2 = gimple_build_assign (gcov_type_tmp_var, PLUS_EXPR,
- gimple_assign_lhs (stmt1), one);
- stmt3 = gimple_build_assign (unshare_expr (ref), gimple_assign_lhs (stmt2));
- gsi_insert_on_edge (e, stmt1);
- gsi_insert_on_edge (e, stmt2);
- gsi_insert_on_edge (e, stmt3);
+
+ if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
+ {
+ /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
+ tree addr = tree_coverage_counter_addr (GCOV_COUNTER_ARCS, edgeno);
+ gcall *stmt
+ = gimple_build_call (builtin_decl_explicit (GCOV_TYPE_ATOMIC_FETCH_ADD),
+ 3, addr, one,
+ build_int_cst (integer_type_node,
+ MEMMODEL_RELAXED));
+ gsi_insert_on_edge (e, stmt);
+ }
+ else
+ {
+ tree ref = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
+ tree gcov_type_tmp_var = make_temp_ssa_name (gcov_type_node,
+ NULL, "PROF_edge_counter");
+ gassign *stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
+ gcov_type_tmp_var = make_temp_ssa_name (gcov_type_node,
+ NULL, "PROF_edge_counter");
+ gassign *stmt2 = gimple_build_assign (gcov_type_tmp_var, PLUS_EXPR,
+ gimple_assign_lhs (stmt1), one);
+ gassign *stmt3 = gimple_build_assign (unshare_expr (ref),
+ gimple_assign_lhs (stmt2));
+ gsi_insert_on_edge (e, stmt1);
+ gsi_insert_on_edge (e, stmt2);
+ gsi_insert_on_edge (e, stmt3);
+ }
}
/* Emits code to get VALUE to instrument at GSI, and returns the
diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index f09b39b..8b0fdd9 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -860,8 +860,10 @@ LIBGCOV_MERGE = _gcov_merge_add _gcov_merge_single _gcov_merge_delta \
_gcov_merge_ior _gcov_merge_time_profile _gcov_merge_icall_topn
LIBGCOV_PROFILER = _gcov_interval_profiler _gcov_pow2_profiler \
_gcov_one_value_profiler _gcov_indirect_call_profiler \
+ _gcov_one_value_profiler_atomic \
_gcov_average_profiler _gcov_ior_profiler \
- _gcov_indirect_call_profiler_v2 _gcov_time_profiler \
+ _gcov_indirect_call_profiler_v2 \
+ _gcov_time_profiler \
_gcov_indirect_call_topn_profiler
LIBGCOV_INTERFACE = _gcov_dump _gcov_flush _gcov_fork \
_gcov_execl _gcov_execlp \
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index e947188..1b307ac 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -93,6 +93,31 @@ __gcov_one_value_profiler (gcov_type *counters, gcov_type value)
}
#endif
+/* Atomic update version of __gcov_one_value_profile_body(). */
+
+static inline void
+__gcov_one_value_profiler_body_atomic (gcov_type *counters, gcov_type value)
+{
+ if (value == counters[0])
+ GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], 1, MEMMODEL_RELAXED);
+ else if (counters[1] == 0)
+ {
+ counters[1] = 1;
+ counters[0] = value;
+ }
+ else
+ GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[1], -1, MEMMODEL_RELAXED);
+ GCOV_TYPE_ATOMIC_FETCH_ADD_FN (&counters[2], 1, MEMMODEL_RELAXED);
+}
+
+#ifdef L_gcov_one_value_profiler_atomic
+void
+__gcov_one_value_profiler_atomic (gcov_type *counters, gcov_type value)
+{
+ __gcov_one_value_profiler_body_atomic (counters, value);
+}
+#endif
+
#ifdef L_gcov_indirect_call_topn_profiler
/* Tries to keep track the most frequent N values in the counters where
N is specified by parameter TOPN_VAL. To track top N values, 2*N counter
@@ -229,6 +254,7 @@ __gcov_indirect_call_topn_profiler (gcov_type value, void* cur_func)
&& *(void **) cur_func == *(void **) callee_func))
__gcov_topn_value_profiler_body (__gcov_indirect_call_topn_counters, value);
}
+
#endif
#ifdef L_gcov_indirect_call_profiler
@@ -291,9 +317,23 @@ __gcov_indirect_call_profiler_v2 (gcov_type value, void* cur_func)
the descriptors to see if they point to the same function. */
if (cur_func == __gcov_indirect_call_callee
|| (__LIBGCC_VTABLE_USES_DESCRIPTORS__ && __gcov_indirect_call_callee
- && *(void **) cur_func == *(void **) __gcov_indirect_call_callee))
+ && *(void **) cur_func == *(void **) __gcov_indirect_call_callee))
__gcov_one_value_profiler_body (__gcov_indirect_call_counters, value);
}
+
+/* Atomic update version of __gcov_indirect_call_profiler_v2(). */
+void
+__gcov_indirect_call_profiler_v2_atomic (gcov_type value, void* cur_func)
+{
+ /* If the C++ virtual tables contain function descriptors then one
+ function may have multiple descriptors and we need to dereference
+ the descriptors to see if they point to the same function. */
+ if (cur_func == __gcov_indirect_call_callee
+ || (__LIBGCC_VTABLE_USES_DESCRIPTORS__ && __gcov_indirect_call_callee
+ && *(void **) cur_func == *(void **) __gcov_indirect_call_callee))
+ __gcov_one_value_profiler_body_atomic (__gcov_indirect_call_counters,
+ value);
+}
#endif
#ifdef L_gcov_time_profiler
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index ae77998..0bd905b 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -270,9 +270,11 @@ extern void __gcov_merge_icall_topn (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
extern void __gcov_interval_profiler (gcov_type *, gcov_type, int, unsigned);
extern void __gcov_pow2_profiler (gcov_type *, gcov_type);
extern void __gcov_one_value_profiler (gcov_type *, gcov_type);
+extern void __gcov_one_value_profiler_atomic (gcov_type *, gcov_type);
extern void __gcov_indirect_call_profiler (gcov_type*, gcov_type,
void*, void*);
extern void __gcov_indirect_call_profiler_v2 (gcov_type, void *);
+extern void __gcov_indirect_call_profiler_v2_atomic (gcov_type, void *);
extern void __gcov_time_profiler (gcov_type *);
extern void __gcov_average_profiler (gcov_type *, gcov_type);
extern void __gcov_ior_profiler (gcov_type *, gcov_type);
--
2.9.2