public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/4] gcov: Improve -fprofile-update=atomic
@ 2023-11-14 22:08 Sebastian Huber
  2023-11-14 22:08 ` [PATCH 1/4] gcov: Remove TARGET_GCOV_TYPE_SIZE target hook Sebastian Huber
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Sebastian Huber @ 2023-11-14 22:08 UTC (permalink / raw)
  To: gcc-patches

Sebastian Huber (4):
  gcov: Remove TARGET_GCOV_TYPE_SIZE target hook
  Add TARGET_HAVE_LIBATOMIC
  gcov: Add gen_counter_update()
  gcov: Improve -fprofile-update=atomic

 gcc/c-family/c-cppbuiltin.cc |   4 +-
 gcc/config/rtems.h           |   2 +
 gcc/config/sparc/rtemself.h  |   2 -
 gcc/config/sparc/sparc.cc    |  11 --
 gcc/coverage.cc              |   2 +-
 gcc/doc/invoke.texi          |  19 ++-
 gcc/doc/tm.texi              |  16 +--
 gcc/doc/tm.texi.in           |   4 +-
 gcc/target.def               |  20 ++-
 gcc/targhooks.cc             |   7 --
 gcc/targhooks.h              |   2 -
 gcc/tree-profile.cc          | 232 +++++++++++++++++++++++------------
 libgcc/libgcov.h             |  16 +--
 13 files changed, 197 insertions(+), 140 deletions(-)

-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] gcov: Remove TARGET_GCOV_TYPE_SIZE target hook
  2023-11-14 22:08 [PATCH 0/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
@ 2023-11-14 22:08 ` Sebastian Huber
  2023-11-14 22:08 ` [PATCH 2/4] Add TARGET_HAVE_LIBATOMIC Sebastian Huber
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Sebastian Huber @ 2023-11-14 22:08 UTC (permalink / raw)
  To: gcc-patches

This reverts commit 8cdcea51c0fd753e6a652c9b236e91b3a6e0911c.

gcc/c-family/ChangeLog:

	* c-cppbuiltin.cc (c_cpp_builtins): Do not define
	__LIBGCC_GCOV_TYPE_SIZE.

gcc/ChangeLog:

	* config/sparc/rtemself.h (SPARC_GCOV_TYPE_SIZE): Remove.
	* config/sparc/sparc.cc (sparc_gcov_type_size): Likewise.
	(TARGET_GCOV_TYPE_SIZE): Likewise.
	* coverage.cc (get_gcov_type): Use LONG_LONG_TYPE_SIZE instead
	of removed target hook.
	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in (TARGET_GCOV_TYPE_SIZE): Remove.
	* target.def: Likewise.
	* targhooks.cc (default_gcov_type_size): Likewise.
	* targhooks.h (default_gcov_type_size): Likewise.

libgcc/ChangeLog:

	* libgcov.h (gcov_type): Use LONG_LONG_TYPE_SIZE.
	(gcov_type_unsigned): Likewise.
---
 gcc/c-family/c-cppbuiltin.cc |  2 --
 gcc/config/sparc/rtemself.h  |  2 --
 gcc/config/sparc/sparc.cc    | 11 -----------
 gcc/coverage.cc              |  2 +-
 gcc/doc/tm.texi              | 11 -----------
 gcc/doc/tm.texi.in           |  2 --
 gcc/target.def               | 12 ------------
 gcc/targhooks.cc             |  7 -------
 gcc/targhooks.h              |  2 --
 libgcc/libgcov.h             |  6 +++---
 10 files changed, 4 insertions(+), 53 deletions(-)

diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc
index 5d64625fcd7a..cdf9850cb19e 100644
--- a/gcc/c-family/c-cppbuiltin.cc
+++ b/gcc/c-family/c-cppbuiltin.cc
@@ -1538,8 +1538,6 @@ c_cpp_builtins (cpp_reader *pfile)
       /* For libgcov.  */
       builtin_define_with_int_value ("__LIBGCC_VTABLE_USES_DESCRIPTORS__",
 				     TARGET_VTABLE_USES_DESCRIPTORS);
-      builtin_define_with_int_value ("__LIBGCC_GCOV_TYPE_SIZE",
-				     targetm.gcov_type_size());
     }
 
   /* For use in assembly language.  */
diff --git a/gcc/config/sparc/rtemself.h b/gcc/config/sparc/rtemself.h
index bf4682238aea..28400a36126c 100644
--- a/gcc/config/sparc/rtemself.h
+++ b/gcc/config/sparc/rtemself.h
@@ -40,5 +40,3 @@
 
 /* Use the default */
 #undef LINK_GCC_C_SEQUENCE_SPEC
-
-#define SPARC_GCOV_TYPE_SIZE 32
diff --git a/gcc/config/sparc/sparc.cc b/gcc/config/sparc/sparc.cc
index 8c0c9dce9714..fd21281aee83 100644
--- a/gcc/config/sparc/sparc.cc
+++ b/gcc/config/sparc/sparc.cc
@@ -971,17 +971,6 @@ char sparc_hard_reg_printed[8];
 #undef TARGET_ZERO_CALL_USED_REGS
 #define TARGET_ZERO_CALL_USED_REGS sparc_zero_call_used_regs
 
-#ifdef SPARC_GCOV_TYPE_SIZE
-static HOST_WIDE_INT
-sparc_gcov_type_size (void)
-{
-  return SPARC_GCOV_TYPE_SIZE;
-}
-
-#undef TARGET_GCOV_TYPE_SIZE
-#define TARGET_GCOV_TYPE_SIZE sparc_gcov_type_size
-#endif
-
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 /* Return the memory reference contained in X if any, zero otherwise.  */
diff --git a/gcc/coverage.cc b/gcc/coverage.cc
index 7ed3a5d4cebf..ad55f0f19096 100644
--- a/gcc/coverage.cc
+++ b/gcc/coverage.cc
@@ -138,7 +138,7 @@ tree
 get_gcov_type (void)
 {
   scalar_int_mode mode
-    = smallest_int_mode_for_size (targetm.gcov_type_size ());
+    = smallest_int_mode_for_size (LONG_LONG_TYPE_SIZE > 32 ? 64 : 32);
   return lang_hooks.types.type_for_mode (mode, false);
 }
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index a660e33739bd..d93b33099dd4 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12431,17 +12431,6 @@ Store the result in @var{target} if convenient.
 The default clears the top byte of the original pointer.
 @end deftypefn
 
-@deftypefn {Target Hook} HOST_WIDE_INT TARGET_GCOV_TYPE_SIZE (void)
-Returns the gcov type size in bits.  This type is used for example for
-counters incremented by profiling and code-coverage events.  The default
-value is 64, if the type size of long long is greater than 32, otherwise the
-default value is 32.  A 64-bit type is recommended to avoid overflows of the
-counters.  If the @option{-fprofile-update=atomic} is used, then the
-counters are incremented using atomic operations.  Targets not supporting
-64-bit atomic operations may override the default value and request a 32-bit
-type.
-@end deftypefn
-
 @deftypevr {Target Hook} bool TARGET_HAVE_SHADOW_CALL_STACK
 This value is true if the target platform supports
 @option{-fsanitize=shadow-call-stack}.  The default value is false.
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index f7ab5d48a634..9061a5b26af9 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7981,6 +7981,4 @@ maintainer is familiar with.
 
 @hook TARGET_MEMTAG_UNTAGGED_POINTER
 
-@hook TARGET_GCOV_TYPE_SIZE
-
 @hook TARGET_HAVE_SHADOW_CALL_STACK
diff --git a/gcc/target.def b/gcc/target.def
index 171bbd1caf15..5c10c702a26e 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -7118,18 +7118,6 @@ DEFHOOK
  void, (void),
  NULL)
 
-DEFHOOK
-(gcov_type_size,
- "Returns the gcov type size in bits.  This type is used for example for\n\
-counters incremented by profiling and code-coverage events.  The default\n\
-value is 64, if the type size of long long is greater than 32, otherwise the\n\
-default value is 32.  A 64-bit type is recommended to avoid overflows of the\n\
-counters.  If the @option{-fprofile-update=atomic} is used, then the\n\
-counters are incremented using atomic operations.  Targets not supporting\n\
-64-bit atomic operations may override the default value and request a 32-bit\n\
-type.",
- HOST_WIDE_INT, (void), default_gcov_type_size)
-
 /* This value represents whether the shadow call stack is implemented on
    the target platform.  */
 DEFHOOKPOD
diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
index 51bf3fb7a82f..2f60f721a505 100644
--- a/gcc/targhooks.cc
+++ b/gcc/targhooks.cc
@@ -2700,11 +2700,4 @@ default_memtag_untagged_pointer (rtx tagged_pointer, rtx target)
   return untagged_base;
 }
 
-/* The default implementation of TARGET_GCOV_TYPE_SIZE.  */
-HOST_WIDE_INT
-default_gcov_type_size (void)
-{
-  return TYPE_PRECISION (long_long_integer_type_node) > 32 ? 64 : 32;
-}
-
 #include "gt-targhooks.h"
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index cf3d3107a0db..29cb31975518 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -298,6 +298,4 @@ extern rtx default_memtag_set_tag (rtx, rtx, rtx);
 extern rtx default_memtag_extract_tag (rtx, rtx);
 extern rtx default_memtag_untagged_pointer (rtx, rtx);
 
-extern HOST_WIDE_INT default_gcov_type_size (void);
-
 #endif /* GCC_TARGHOOKS_H */
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index 92df440d4012..763118ea5b52 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -52,7 +52,7 @@
 #if __CHAR_BIT__ == 8
 typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI)));
 typedef unsigned gcov_position_t __attribute__ ((mode (SI)));
-#if __LIBGCC_GCOV_TYPE_SIZE > 32
+#if LONG_LONG_TYPE_SIZE > 32
 typedef signed gcov_type __attribute__ ((mode (DI)));
 typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI)));
 #else
@@ -63,7 +63,7 @@ typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI)));
 #if __CHAR_BIT__ == 16
 typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI)));
 typedef unsigned gcov_position_t __attribute__ ((mode (HI)));
-#if __LIBGCC_GCOV_TYPE_SIZE > 32
+#if LONG_LONG_TYPE_SIZE > 32
 typedef signed gcov_type __attribute__ ((mode (SI)));
 typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI)));
 #else
@@ -73,7 +73,7 @@ typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI)));
 #else
 typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI)));
 typedef unsigned gcov_position_t __attribute__ ((mode (QI)));
-#if __LIBGCC_GCOV_TYPE_SIZE > 32
+#if LONG_LONG_TYPE_SIZE > 32
 typedef signed gcov_type __attribute__ ((mode (HI)));
 typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI)));
 #else
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/4] Add TARGET_HAVE_LIBATOMIC
  2023-11-14 22:08 [PATCH 0/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
  2023-11-14 22:08 ` [PATCH 1/4] gcov: Remove TARGET_GCOV_TYPE_SIZE target hook Sebastian Huber
@ 2023-11-14 22:08 ` Sebastian Huber
  2023-11-14 22:08 ` [PATCH 3/4] gcov: Add gen_counter_update() Sebastian Huber
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Sebastian Huber @ 2023-11-14 22:08 UTC (permalink / raw)
  To: gcc-patches

Add target data to indicate if libatomic is available.

gcc/ChangeLog:

	* config/rtems.h (TARGET_HAVE_LIBATOMIC): Define.
	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in (TARGET_HAVE_LIBATOMIC): Add.
	* target.def (have_libatomic): New.
---
 gcc/config/rtems.h | 2 ++
 gcc/doc/tm.texi    | 5 +++++
 gcc/doc/tm.texi.in | 2 ++
 gcc/target.def     | 8 ++++++++
 4 files changed, 17 insertions(+)

diff --git a/gcc/config/rtems.h b/gcc/config/rtems.h
index 8641ea15271d..2c006e4c1e54 100644
--- a/gcc/config/rtems.h
+++ b/gcc/config/rtems.h
@@ -53,6 +53,8 @@
 
 #define TARGET_POSIX_IO
 
+#define TARGET_HAVE_LIBATOMIC true
+
 /* Prefer int for int32_t (see stdint-newlib.h).  */
 #undef STDINT_LONG32
 #define STDINT_LONG32 (INT_TYPE_SIZE != 32 && LONG_TYPE_SIZE == 32)
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index d93b33099dd4..04cd12de792c 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12435,3 +12435,8 @@ The default clears the top byte of the original pointer.
 This value is true if the target platform supports
 @option{-fsanitize=shadow-call-stack}.  The default value is false.
 @end deftypevr
+
+@deftypevr {Target Hook} bool TARGET_HAVE_LIBATOMIC
+This value is true if the target platform supports
+libatomic.  The default value is false.
+@end deftypevr
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 9061a5b26af9..19d21999317a 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7982,3 +7982,5 @@ maintainer is familiar with.
 @hook TARGET_MEMTAG_UNTAGGED_POINTER
 
 @hook TARGET_HAVE_SHADOW_CALL_STACK
+
+@hook TARGET_HAVE_LIBATOMIC
diff --git a/gcc/target.def b/gcc/target.def
index 5c10c702a26e..bf0d6e0aeda1 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -7126,6 +7126,14 @@ DEFHOOKPOD
 @option{-fsanitize=shadow-call-stack}.  The default value is false.",
  bool, false)
 
+/* This value represents whether libatomic is available on
+   the target platform.  */
+DEFHOOKPOD
+(have_libatomic,
+ "This value is true if the target platform supports\n\
+libatomic.  The default value is false.",
+ bool, false)
+
 /* Close the 'struct gcc_target' definition.  */
 HOOK_VECTOR_END (C90_EMPTY_HACK)
 
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] gcov: Add gen_counter_update()
  2023-11-14 22:08 [PATCH 0/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
  2023-11-14 22:08 ` [PATCH 1/4] gcov: Remove TARGET_GCOV_TYPE_SIZE target hook Sebastian Huber
  2023-11-14 22:08 ` [PATCH 2/4] Add TARGET_HAVE_LIBATOMIC Sebastian Huber
@ 2023-11-14 22:08 ` Sebastian Huber
  2023-11-19  9:00   ` Dimitar Dimitrov
  2023-11-14 22:08 ` [PATCH 4/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
  2023-11-15 23:32 ` [PATCH 0/4] " Jeff Law
  4 siblings, 1 reply; 10+ messages in thread
From: Sebastian Huber @ 2023-11-14 22:08 UTC (permalink / raw)
  To: gcc-patches

Move the counter update to the new gen_counter_update() helper function.  Use
it in gimple_gen_edge_profiler() and gimple_gen_time_profiler().  The resulting
gimple instructions should be identical with the exception of the removed
unshare_expr() call.  The unshare_expr() call was used in
gimple_gen_edge_profiler().

gcc/ChangeLog:

	* tree-profile.cc (gen_assign_counter_update): New.
	(gen_counter_update): Likewise.
 	(gimple_gen_edge_profiler): Use gen_counter_update().
	(gimple_gen_time_profiler): Likewise.
---
 gcc/tree-profile.cc | 133 +++++++++++++++++++++-----------------------
 1 file changed, 62 insertions(+), 71 deletions(-)

diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
index da300d5f9e8d..24805ff905c7 100644
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -235,47 +235,78 @@ gimple_init_gcov_profiler (void)
     }
 }
 
-/* Output instructions as GIMPLE trees to increment the edge
-   execution count, and insert them on E.  We rely on
-   gsi_insert_on_edge to preserve the order.  */
+/* If RESULT is not null, then output instructions as GIMPLE trees to assign
+   the updated counter from CALL of FUNC to RESULT.  Insert the CALL and the
+   optional assignment instructions to GSI.  Use NAME for temporary values.  */
 
-void
-gimple_gen_edge_profiler (int edgeno, edge e)
+static inline void
+gen_assign_counter_update (gimple_stmt_iterator *gsi, gcall *call, tree func,
+			   tree result, const char *name)
 {
-  tree one;
+  if (result)
+    {
+      tree result_type = TREE_TYPE (TREE_TYPE (func));
+      tree tmp = make_temp_ssa_name (result_type, NULL, name);
+      gimple_set_lhs (call, tmp);
+      gsi_insert_after (gsi, call, GSI_NEW_STMT);
+      gassign *assign = gimple_build_assign (result, tmp);
+      gsi_insert_after (gsi, assign, GSI_NEW_STMT);
+    }
+  else
+    gsi_insert_after (gsi, call, GSI_NEW_STMT);
+}
 
-  one = build_int_cst (gcov_type_node, 1);
+/* Output instructions as GIMPLE trees to increment the COUNTER.  If RESULT is
+   not null, then assign the updated counter value to RESULT.  Insert the
+   instructions to GSI.  Use NAME for temporary values.  */
+
+static inline void
+gen_counter_update (gimple_stmt_iterator *gsi, tree counter, tree result,
+		    const char *name)
+{
+  tree type = gcov_type_node;
+  tree addr = build_fold_addr_expr (counter);
+  tree one = build_int_cst (type, 1);
+  tree relaxed = build_int_cst (integer_type_node, MEMMODEL_RELAXED);
 
   if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
     {
       /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
-      tree addr = tree_coverage_counter_addr (GCOV_COUNTER_ARCS, edgeno);
-      tree f = builtin_decl_explicit (TYPE_PRECISION (gcov_type_node) > 32
-				      ? BUILT_IN_ATOMIC_FETCH_ADD_8:
-				      BUILT_IN_ATOMIC_FETCH_ADD_4);
-      gcall *stmt = gimple_build_call (f, 3, addr, one,
-				       build_int_cst (integer_type_node,
-						      MEMMODEL_RELAXED));
-      gsi_insert_on_edge (e, stmt);
+      tree f = builtin_decl_explicit (TYPE_PRECISION (type) > 32
+				      ? BUILT_IN_ATOMIC_ADD_FETCH_8:
+				      BUILT_IN_ATOMIC_ADD_FETCH_4);
+      gcall *call = gimple_build_call (f, 3, addr, one, relaxed);
+      gen_assign_counter_update (gsi, call, f, result, name);
     }
   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);
+      tree tmp1 = make_temp_ssa_name (type, NULL, name);
+      gassign *assign1 = gimple_build_assign (tmp1, counter);
+      gsi_insert_after (gsi, assign1, GSI_NEW_STMT);
+      tree tmp2 = make_temp_ssa_name (type, NULL, name);
+      gassign *assign2 = gimple_build_assign (tmp2, PLUS_EXPR, tmp1, one);
+      gsi_insert_after (gsi, assign2, GSI_NEW_STMT);
+      gassign *assign3 = gimple_build_assign (counter, tmp2);
+      gsi_insert_after (gsi, assign3, GSI_NEW_STMT);
+      if (result)
+	{
+	  gassign *assign4 = gimple_build_assign (result, tmp2);
+	  gsi_insert_after (gsi, assign4, GSI_NEW_STMT);
+	}
     }
 }
 
+/* Output instructions as GIMPLE trees to increment the edge
+   execution count, and insert them on E.  */
+
+void
+gimple_gen_edge_profiler (int edgeno, edge e)
+{
+  gimple_stmt_iterator gsi = gsi_last (PENDING_STMT (e));
+  tree counter = tree_coverage_counter_ref (GCOV_COUNTER_ARCS, edgeno);
+  gen_counter_update (&gsi, counter, NULL_TREE, "PROF_edge_counter");
+}
+
 /* Emits code to get VALUE to instrument at GSI, and returns the
    variable containing the value.  */
 
@@ -507,56 +538,16 @@ gimple_gen_time_profiler (unsigned tag)
   tree original_ref = tree_coverage_counter_ref (tag, 0);
   tree ref = force_gimple_operand_gsi (&gsi, original_ref, true, NULL_TREE,
 				       true, GSI_SAME_STMT);
-  tree one = build_int_cst (type, 1);
 
   /* Emit: if (counters[0] != 0).  */
   gcond *cond = gimple_build_cond (EQ_EXPR, ref, build_int_cst (type, 0),
 				   NULL, NULL);
   gsi_insert_before (&gsi, cond, GSI_NEW_STMT);
 
-  gsi = gsi_start_bb (update_bb);
-
   /* Emit: counters[0] = ++__gcov_time_profiler_counter.  */
-  if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
-    {
-      tree ptr = make_temp_ssa_name (build_pointer_type (type), NULL,
-				     "PROF_time_profiler_counter_ptr");
-      tree addr = build1 (ADDR_EXPR, TREE_TYPE (ptr),
-			  tree_time_profiler_counter);
-      gassign *assign = gimple_build_assign (ptr, NOP_EXPR, addr);
-      gsi_insert_before (&gsi, assign, GSI_NEW_STMT);
-      tree f = builtin_decl_explicit (TYPE_PRECISION (gcov_type_node) > 32
-				      ? BUILT_IN_ATOMIC_ADD_FETCH_8:
-				      BUILT_IN_ATOMIC_ADD_FETCH_4);
-      gcall *stmt = gimple_build_call (f, 3, ptr, one,
-				       build_int_cst (integer_type_node,
-						      MEMMODEL_RELAXED));
-      tree result_type = TREE_TYPE (TREE_TYPE (f));
-      tree tmp = make_temp_ssa_name (result_type, NULL, "PROF_time_profile");
-      gimple_set_lhs (stmt, tmp);
-      gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
-      tmp = make_temp_ssa_name (type, NULL, "PROF_time_profile");
-      assign = gimple_build_assign (tmp, NOP_EXPR,
-				    gimple_call_lhs (stmt));
-      gsi_insert_after (&gsi, assign, GSI_NEW_STMT);
-      assign = gimple_build_assign (original_ref, tmp);
-      gsi_insert_after (&gsi, assign, GSI_NEW_STMT);
-    }
-  else
-    {
-      tree tmp = make_temp_ssa_name (type, NULL, "PROF_time_profile");
-      gassign *assign = gimple_build_assign (tmp, tree_time_profiler_counter);
-      gsi_insert_before (&gsi, assign, GSI_NEW_STMT);
-
-      tmp = make_temp_ssa_name (type, NULL, "PROF_time_profile");
-      assign = gimple_build_assign (tmp, PLUS_EXPR, gimple_assign_lhs (assign),
-				    one);
-      gsi_insert_after (&gsi, assign, GSI_NEW_STMT);
-      assign = gimple_build_assign (original_ref, tmp);
-      gsi_insert_after (&gsi, assign, GSI_NEW_STMT);
-      assign = gimple_build_assign (tree_time_profiler_counter, tmp);
-      gsi_insert_after (&gsi, assign, GSI_NEW_STMT);
-    }
+  gsi = gsi_start_bb (update_bb);
+  gen_counter_update (&gsi, tree_time_profiler_counter, original_ref,
+		      "PROF_time_profile");
 }
 
 /* Output instructions as GIMPLE trees to increment the average histogram
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/4] gcov: Improve -fprofile-update=atomic
  2023-11-14 22:08 [PATCH 0/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
                   ` (2 preceding siblings ...)
  2023-11-14 22:08 ` [PATCH 3/4] gcov: Add gen_counter_update() Sebastian Huber
@ 2023-11-14 22:08 ` Sebastian Huber
  2023-11-15  5:51   ` Sebastian Huber
  2023-11-15 23:32 ` [PATCH 0/4] " Jeff Law
  4 siblings, 1 reply; 10+ messages in thread
From: Sebastian Huber @ 2023-11-14 22:08 UTC (permalink / raw)
  To: gcc-patches

The code coverage support uses counters to determine which edges in the control
flow graph were executed.  If a counter overflows, then the code coverage
information is invalid.  Therefore the counter type should be a 64-bit integer.
In multi-threaded applications, it is important that the counter increments are
atomic.  This is not the case by default.  The user can enable atomic counter
increments through the -fprofile-update=atomic and
-fprofile-update=prefer-atomic options.

If the target supports 64-bit atomic operations, then everything is fine.  If
not and -fprofile-update=prefer-atomic was chosen by the user, then non-atomic
counter increments will be used.  However, if the target does not support the
required atomic operations and -fprofile-atomic=update was chosen by the user,
then a warning was issued and as a forced fallback to non-atomic operations was
done.  This is probably not what a user wants.  There is still hardware on the
market which does not have atomic operations and is used for multi-threaded
applications.  A user which selects -fprofile-update=atomic wants consistent
code coverage data and not random data.

This patch removes the fallback to non-atomic operations for
-fprofile-update=atomic the target platform supports libatomic.  To
mitigate potential performance issues an optimization for systems which
only support 32-bit atomic operations is provided.  Here, the edge
counter increments are done like this:

  low = __atomic_add_fetch_4 (&counter.low, 1, MEMMODEL_RELAXED);
  high_inc = low == 0 ? 1 : 0;
  __atomic_add_fetch_4 (&counter.high, high_inc, MEMMODEL_RELAXED);

In gimple_gen_time_profiler() this split operation cannot be used, since the
updated counter value is also required.  Here, a library call is emitted.  This
is not a performance issue since the update is only done if counters[0] == 0.

gcc/c-family/ChangeLog:

	* c-cppbuiltin.cc (c_cpp_builtins):  Define
	__LIBGCC_HAVE_LIBATOMIC for libgcov.

gcc/ChangeLog:

	* doc/invoke.texi (-fprofile-update): Clarify default method.  Document
	the atomic method behaviour.
	* tree-profile.cc (enum counter_update_method): New.
	(counter_update): Likewise.
	(gen_counter_update): Use counter_update_method.  Split the
	atomic counter update in two 32-bit atomic operations if
	necessary.
	(tree_profiling): Select counter_update_method.

libgcc/ChangeLog:

	* libgcov.h (GCOV_SUPPORTS_ATOMIC): Always define it.
	Set it also to 1, if __LIBGCC_HAVE_LIBATOMIC is defined.
---
 gcc/c-family/c-cppbuiltin.cc |  2 +
 gcc/doc/invoke.texi          | 19 ++++++-
 gcc/tree-profile.cc          | 99 +++++++++++++++++++++++++++++++++---
 libgcc/libgcov.h             | 10 ++--
 4 files changed, 114 insertions(+), 16 deletions(-)

diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc
index cdf9850cb19e..e8576737fafb 100644
--- a/gcc/c-family/c-cppbuiltin.cc
+++ b/gcc/c-family/c-cppbuiltin.cc
@@ -1538,6 +1538,8 @@ c_cpp_builtins (cpp_reader *pfile)
       /* For libgcov.  */
       builtin_define_with_int_value ("__LIBGCC_VTABLE_USES_DESCRIPTORS__",
 				     TARGET_VTABLE_USES_DESCRIPTORS);
+      builtin_define_with_int_value ("__LIBGCC_HAVE_LIBATOMIC",
+				     TARGET_HAVE_LIBATOMIC);
     }
 
   /* For use in assembly language.  */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index de40f62e219c..8fe3c86ad419 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -16603,7 +16603,24 @@ while the second one prevents profile corruption by emitting thread-safe code.
 Using @samp{prefer-atomic} would be transformed either to @samp{atomic},
 when supported by a target, or to @samp{single} otherwise.  The GCC driver
 automatically selects @samp{prefer-atomic} when @option{-pthread}
-is present in the command line.
+is present in the command line, otherwise the default method is @samp{single}.
+
+If @samp{atomic} is selected, then the profile information is updated using
+atomic operations on a best-effort basis.  Ideally, the profile information is
+updated through atomic operations in hardware.  If the target platform does not
+support the required atomic operations in hardware, however, @file{libatomic}
+is available, then the profile information is updated through calls to
+@file{libatomic}.  If the target platform neither supports the required atomic
+operations in hardware nor @file{libatomic}, then the profile information is
+not atomically updated and a warning is issued.  In this case, the obtained
+profiling information may be corrupt for multi-threaded applications.
+
+For performance reasons, if 64-bit counters are used for the profiling
+information and the target platform only supports 32-bit atomic operations in
+hardware, then the performance critical profiling updates are done using two
+32-bit atomic operations for each counter update.  If a signal interrupts these
+two operations updating a counter, then the profiling information may be in an
+inconsistent state.
 
 @opindex fprofile-filter-files
 @item -fprofile-filter-files=@var{regex}
diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
index 24805ff905c7..12255f06f992 100644
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -73,6 +73,41 @@ static GTY(()) tree ic_tuple_var;
 static GTY(()) tree ic_tuple_counters_field;
 static GTY(()) tree ic_tuple_callee_field;
 
+/* Types of counter update methods.
+
+   By default, the counter updates are done for a single threaded system
+   (COUNTER_UPDATE_SINGLE_THREAD).
+
+   If the user selected atomic profile counter updates
+   (-fprofile-update=atomic), then the counter updates will be done atomically
+   on a best-effort basis.  One of three methods to do the counter updates is
+   selected according to the target capabilities.
+
+   Ideally, the counter updates are done through atomic operations in hardware
+   (COUNTER_UPDATE_ATOMIC_BUILTIN).
+
+   If the target supports only 32-bit atomic increments and gcov_type_node is a
+   64-bit integer type, then for the profile edge counters the increment is
+   performed through two separate 32-bit atomic increments
+   (COUNTER_UPDATE_ATOMIC_SPLIT or COUNTER_UPDATE_ATOMIC_PARTIAL).  If the
+   target supports libatomic (TARGET_HAVE_LIBATOMIC), then other counter
+   updates are carried out by libatomic calls (COUNTER_UPDATE_ATOMIC_SPLIT).
+   If the target does not support libatomic, then the other counter updates are
+   not done atomically (COUNTER_UPDATE_ATOMIC_PARTIAL) and a warning is
+   issued.
+
+   If the target does not support atomic operations in hardware, however,  it
+   supports libatomic, then all updates are carried out by libatomic calls
+   (COUNTER_UPDATE_ATOMIC_BUILTIN).  */
+enum counter_update_method {
+  COUNTER_UPDATE_SINGLE_THREAD,
+  COUNTER_UPDATE_ATOMIC_BUILTIN,
+  COUNTER_UPDATE_ATOMIC_SPLIT,
+  COUNTER_UPDATE_ATOMIC_PARTIAL
+};
+
+static counter_update_method counter_update = COUNTER_UPDATE_SINGLE_THREAD;
+
 /* Do initialization work for the edge profiler.  */
 
 /* Add code:
@@ -269,7 +304,8 @@ gen_counter_update (gimple_stmt_iterator *gsi, tree counter, tree result,
   tree one = build_int_cst (type, 1);
   tree relaxed = build_int_cst (integer_type_node, MEMMODEL_RELAXED);
 
-  if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
+  if (counter_update == COUNTER_UPDATE_ATOMIC_BUILTIN ||
+      (result && counter_update == COUNTER_UPDATE_ATOMIC_SPLIT))
     {
       /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
       tree f = builtin_decl_explicit (TYPE_PRECISION (type) > 32
@@ -278,6 +314,38 @@ gen_counter_update (gimple_stmt_iterator *gsi, tree counter, tree result,
       gcall *call = gimple_build_call (f, 3, addr, one, relaxed);
       gen_assign_counter_update (gsi, call, f, result, name);
     }
+  else if (!result && (counter_update == COUNTER_UPDATE_ATOMIC_SPLIT ||
+		       counter_update == COUNTER_UPDATE_ATOMIC_PARTIAL))
+    {
+      /* low = __atomic_add_fetch_4 (addr, 1, MEMMODEL_RELAXED);
+	 high_inc = low == 0 ? 1 : 0;
+	 __atomic_add_fetch_4 (addr_high, high_inc, MEMMODEL_RELAXED); */
+      tree zero32 = build_zero_cst (uint32_type_node);
+      tree one32 = build_one_cst (uint32_type_node);
+      tree addr_high = make_temp_ssa_name (TREE_TYPE (addr), NULL, name);
+      tree four = build_int_cst (size_type_node, 4);
+      gassign *assign1 = gimple_build_assign (addr_high, POINTER_PLUS_EXPR,
+					      addr, four);
+      gsi_insert_after (gsi, assign1, GSI_NEW_STMT);
+      if (WORDS_BIG_ENDIAN)
+	std::swap (addr, addr_high);
+      tree f = builtin_decl_explicit (BUILT_IN_ATOMIC_ADD_FETCH_4);
+      gcall *call1 = gimple_build_call (f, 3, addr, one, relaxed);
+      tree low = make_temp_ssa_name (uint32_type_node, NULL, name);
+      gimple_call_set_lhs (call1, low);
+      gsi_insert_after (gsi, call1, GSI_NEW_STMT);
+      tree is_zero = make_temp_ssa_name (boolean_type_node, NULL, name);
+      gassign *assign2 = gimple_build_assign (is_zero, EQ_EXPR, low,
+					      zero32);
+      gsi_insert_after (gsi, assign2, GSI_NEW_STMT);
+      tree high_inc = make_temp_ssa_name (uint32_type_node, NULL, name);
+      gassign *assign3 = gimple_build_assign (high_inc, COND_EXPR,
+					      is_zero, one32, zero32);
+      gsi_insert_after (gsi, assign3, GSI_NEW_STMT);
+      gcall *call2 = gimple_build_call (f, 3, addr_high, high_inc,
+					relaxed);
+      gsi_insert_after (gsi, call2, GSI_NEW_STMT);
+    }
   else
     {
       tree tmp1 = make_temp_ssa_name (type, NULL, name);
@@ -689,15 +757,20 @@ tree_profiling (void)
   struct cgraph_node *node;
 
   /* Verify whether we can utilize atomic update operations.  */
-  bool can_support_atomic = false;
+  bool can_support_atomic = TARGET_HAVE_LIBATOMIC;
   unsigned HOST_WIDE_INT gcov_type_size
     = tree_to_uhwi (TYPE_SIZE_UNIT (get_gcov_type ()));
-  if (gcov_type_size == 4)
-    can_support_atomic
-      = HAVE_sync_compare_and_swapsi || HAVE_atomic_compare_and_swapsi;
-  else if (gcov_type_size == 8)
-    can_support_atomic
-      = HAVE_sync_compare_and_swapdi || HAVE_atomic_compare_and_swapdi;
+  bool have_atomic_4
+    = HAVE_sync_compare_and_swapsi || HAVE_atomic_compare_and_swapsi;
+  bool have_atomic_8
+    = HAVE_sync_compare_and_swapdi || HAVE_atomic_compare_and_swapdi;
+  if (!can_support_atomic)
+    {
+      if (gcov_type_size == 4)
+	can_support_atomic = have_atomic_4;
+      else if (gcov_type_size == 8)
+	can_support_atomic = have_atomic_8;
+    }
 
   if (flag_profile_update == PROFILE_UPDATE_ATOMIC
       && !can_support_atomic)
@@ -710,6 +783,16 @@ tree_profiling (void)
     flag_profile_update = can_support_atomic
       ? PROFILE_UPDATE_ATOMIC : PROFILE_UPDATE_SINGLE;
 
+  if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
+    {
+      if (gcov_type_size == 8 && !have_atomic_8 && have_atomic_4)
+	counter_update = COUNTER_UPDATE_ATOMIC_SPLIT;
+      else
+	counter_update = COUNTER_UPDATE_ATOMIC_BUILTIN;
+    }
+  else if (gcov_type_size == 8 && have_atomic_4)
+      counter_update = COUNTER_UPDATE_ATOMIC_PARTIAL;
+
   /* This is a small-ipa pass that gets called only once, from
      cgraphunit.cc:ipa_passes().  */
   gcc_assert (symtab->state == IPA_SSA);
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index 763118ea5b52..d04c070d0cfa 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -95,18 +95,14 @@ typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI)));
 #define GCOV_LOCKED_WITH_LOCKING 0
 #endif
 
-#ifndef GCOV_SUPPORTS_ATOMIC
 /* Detect whether target can support atomic update of profilers.  */
-#if __SIZEOF_LONG_LONG__ == 4 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
-#define GCOV_SUPPORTS_ATOMIC 1
-#else
-#if __SIZEOF_LONG_LONG__ == 8 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
+#if (__SIZEOF_LONG_LONG__ == 4 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4) || \
+    (__SIZEOF_LONG_LONG__ == 8 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8) || \
+    defined (__LIBGCC_HAVE_LIBATOMIC)
 #define GCOV_SUPPORTS_ATOMIC 1
 #else
 #define GCOV_SUPPORTS_ATOMIC 0
 #endif
-#endif
-#endif
 
 /* In libgcov we need these functions to be extern, so prefix them with
    __gcov.  In libgcov they must also be hidden so that the instance in
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] gcov: Improve -fprofile-update=atomic
  2023-11-14 22:08 ` [PATCH 4/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
@ 2023-11-15  5:51   ` Sebastian Huber
  2023-11-21  9:53     ` Jakub Jelinek
  0 siblings, 1 reply; 10+ messages in thread
From: Sebastian Huber @ 2023-11-15  5:51 UTC (permalink / raw)
  To: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 559 bytes --]


Hello, 
  
sorry, in the patch I should use targetm.have_atomic instead of TARGET_HAVE_LIBATOMIC. 
  
---  
 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/  

[-- Attachment #2: v2-0001-gcov-Improve-fprofile-update-atomic.patch --]
[-- Type: application/mbox, Size: 12442 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4] gcov: Improve -fprofile-update=atomic
  2023-11-14 22:08 [PATCH 0/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
                   ` (3 preceding siblings ...)
  2023-11-14 22:08 ` [PATCH 4/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
@ 2023-11-15 23:32 ` Jeff Law
  4 siblings, 0 replies; 10+ messages in thread
From: Jeff Law @ 2023-11-15 23:32 UTC (permalink / raw)
  To: Sebastian Huber, gcc-patches



On 11/14/23 15:08, Sebastian Huber wrote:
> Sebastian Huber (4):
>    gcov: Remove TARGET_GCOV_TYPE_SIZE target hook
>    Add TARGET_HAVE_LIBATOMIC
>    gcov: Add gen_counter_update()
>    gcov: Improve -fprofile-update=atomic
> 
>   gcc/c-family/c-cppbuiltin.cc |   4 +-
>   gcc/config/rtems.h           |   2 +
>   gcc/config/sparc/rtemself.h  |   2 -
>   gcc/config/sparc/sparc.cc    |  11 --
>   gcc/coverage.cc              |   2 +-
>   gcc/doc/invoke.texi          |  19 ++-
>   gcc/doc/tm.texi              |  16 +--
>   gcc/doc/tm.texi.in           |   4 +-
>   gcc/target.def               |  20 ++-
>   gcc/targhooks.cc             |   7 --
>   gcc/targhooks.h              |   2 -
>   gcc/tree-profile.cc          | 232 +++++++++++++++++++++++------------
>   libgcc/libgcov.h             |  16 +--
>   13 files changed, 197 insertions(+), 140 deletions(-)
This series as a whole is OK with the targetm.have_atomic instead of 
TARGET_HAVE_LIBATOMIC fix you mentioned after posting the series.

jeff

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] gcov: Add gen_counter_update()
  2023-11-14 22:08 ` [PATCH 3/4] gcov: Add gen_counter_update() Sebastian Huber
@ 2023-11-19  9:00   ` Dimitar Dimitrov
  2023-11-19 14:43     ` Sebastian Huber
  0 siblings, 1 reply; 10+ messages in thread
From: Dimitar Dimitrov @ 2023-11-19  9:00 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: gcc-patches

On Tue, Nov 14, 2023 at 11:08:24PM +0100, Sebastian Huber wrote:
> Move the counter update to the new gen_counter_update() helper function.  Use
> it in gimple_gen_edge_profiler() and gimple_gen_time_profiler().  The resulting
> gimple instructions should be identical with the exception of the removed
> unshare_expr() call.  The unshare_expr() call was used in
> gimple_gen_edge_profiler().
> 
> gcc/ChangeLog:
> 
> 	* tree-profile.cc (gen_assign_counter_update): New.
> 	(gen_counter_update): Likewise.
>  	(gimple_gen_edge_profiler): Use gen_counter_update().
> 	(gimple_gen_time_profiler): Likewise.
> ---
>  gcc/tree-profile.cc | 133 +++++++++++++++++++++-----------------------
>  1 file changed, 62 insertions(+), 71 deletions(-)
> 

Hi Sebastian,

This patch caused a bunch of test failures on arm-none-eabi and
pru-unknown-elf targets.  One example:

/home/dinux/projects/pru/testbot-workspace/gcc/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-1.c: In function 'main':
/home/dinux/projects/pru/testbot-workspace/gcc/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-1.c:19:1: error: incorrect sharing of tree nodes
__gcov0.main[0]
# .MEM_12 = VDEF <.MEM_9>
__gcov0.main[0] = PROF_edge_counter_4;
during IPA pass: profile
/home/dinux/projects/pru/testbot-workspace/gcc/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-1.c:19:1: internal compiler error: verify_gimple failed
0xfd9c7d verify_gimple_in_cfg(function*, bool, bool)
        /home/dinux/projects/pru/testbot-workspace/gcc/gcc/tree-cfg.cc:5662
0xe586a4 execute_function_todo
        /home/dinux/projects/pru/testbot-workspace/gcc/gcc/passes.cc:2088
0xe58ba2 do_per_function
        /home/dinux/projects/pru/testbot-workspace/gcc/gcc/passes.cc:1694
0xe58ba2 do_per_function
        /home/dinux/projects/pru/testbot-workspace/gcc/gcc/passes.cc:1684
0xe58bfe execute_todo
        /home/dinux/projects/pru/testbot-workspace/gcc/gcc/passes.cc:2142
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
compiler exited with status 1
FAIL: gcc.dg/no_profile_instrument_function-attr-1.c (internal compiler error: verify_gimple failed)
FAIL: gcc.dg/no_profile_instrument_function-attr-1.c (test for excess errors)


I'm using the following script to build and test:
  https://github.com/dinuxbg/gnupru/blob/master/testing/manual-test-pru.sh

Regards,
Dimitar

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] gcov: Add gen_counter_update()
  2023-11-19  9:00   ` Dimitar Dimitrov
@ 2023-11-19 14:43     ` Sebastian Huber
  0 siblings, 0 replies; 10+ messages in thread
From: Sebastian Huber @ 2023-11-19 14:43 UTC (permalink / raw)
  To: Dimitar Dimitrov; +Cc: gcc-patches

Hello Dimitar,

On 19.11.23 10:00, Dimitar Dimitrov wrote:
> On Tue, Nov 14, 2023 at 11:08:24PM +0100, Sebastian Huber wrote:
>> Move the counter update to the new gen_counter_update() helper function.  Use
>> it in gimple_gen_edge_profiler() and gimple_gen_time_profiler().  The resulting
>> gimple instructions should be identical with the exception of the removed
>> unshare_expr() call.  The unshare_expr() call was used in
>> gimple_gen_edge_profiler().
>>
>> gcc/ChangeLog:
>>
>> 	* tree-profile.cc (gen_assign_counter_update): New.
>> 	(gen_counter_update): Likewise.
>>   	(gimple_gen_edge_profiler): Use gen_counter_update().
>> 	(gimple_gen_time_profiler): Likewise.
>> ---
>>   gcc/tree-profile.cc | 133 +++++++++++++++++++++-----------------------
>>   1 file changed, 62 insertions(+), 71 deletions(-)
>>
> Hi Sebastian,
> 
> This patch caused a bunch of test failures on arm-none-eabi and
> pru-unknown-elf targets.

thanks for the report. I will have a look at this next week. I guess it 
has something to do with the removed unshare_expr() call. I don't really 
know what it does, but I will try to figure this out.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] gcov: Improve -fprofile-update=atomic
  2023-11-15  5:51   ` Sebastian Huber
@ 2023-11-21  9:53     ` Jakub Jelinek
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Jelinek @ 2023-11-21  9:53 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: gcc-patches

On Wed, Nov 15, 2023 at 06:51:10AM +0100, Sebastian Huber wrote:
> sorry, in the patch I should use targetm.have_atomic instead of TARGET_HAVE_LIBATOMIC. 

I've noticed the r14-5579 commit introduced some formatting issues,
this patch fixes what I saw.

In particular, operators don't go at the end of line but at the start of
next one.

Committed to trunk as obvious.

2023-11-21  Jakub Jelinek  <jakub@redhat.com>

gcc/
	* tree-profile.cc (gen_counter_update, tree_profiling): Formatting
	fixes.
libgcc/
	* libgcov.h (GCOV_SUPPORTS_ATOMIC): Formatting fixes.

--- gcc/tree-profile.cc.jj	2023-11-21 09:31:36.349387476 +0100
+++ gcc/tree-profile.cc	2023-11-21 10:45:26.035499140 +0100
@@ -304,8 +304,8 @@ gen_counter_update (gimple_stmt_iterator
   tree one = build_int_cst (type, 1);
   tree relaxed = build_int_cst (integer_type_node, MEMMODEL_RELAXED);
 
-  if (counter_update == COUNTER_UPDATE_ATOMIC_BUILTIN ||
-      (result && counter_update == COUNTER_UPDATE_ATOMIC_SPLIT))
+  if (counter_update == COUNTER_UPDATE_ATOMIC_BUILTIN
+      || (result && counter_update == COUNTER_UPDATE_ATOMIC_SPLIT))
     {
       /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
       tree f = builtin_decl_explicit (TYPE_PRECISION (type) > 32
@@ -314,8 +314,8 @@ gen_counter_update (gimple_stmt_iterator
       gcall *call = gimple_build_call (f, 3, addr, one, relaxed);
       gen_assign_counter_update (gsi, call, f, result, name);
     }
-  else if (!result && (counter_update == COUNTER_UPDATE_ATOMIC_SPLIT ||
-		       counter_update == COUNTER_UPDATE_ATOMIC_PARTIAL))
+  else if (!result && (counter_update == COUNTER_UPDATE_ATOMIC_SPLIT
+		       || counter_update == COUNTER_UPDATE_ATOMIC_PARTIAL))
     {
       /* low = __atomic_add_fetch_4 (addr, 1, MEMMODEL_RELAXED);
 	 high_inc = low == 0 ? 1 : 0;
@@ -780,8 +780,8 @@ tree_profiling (void)
       flag_profile_update = PROFILE_UPDATE_SINGLE;
     }
   else if (flag_profile_update == PROFILE_UPDATE_PREFER_ATOMIC)
-    flag_profile_update = can_support_atomic
-      ? PROFILE_UPDATE_ATOMIC : PROFILE_UPDATE_SINGLE;
+    flag_profile_update
+      = can_support_atomic ? PROFILE_UPDATE_ATOMIC : PROFILE_UPDATE_SINGLE;
 
   if (flag_profile_update == PROFILE_UPDATE_ATOMIC)
     {
@@ -791,7 +791,7 @@ tree_profiling (void)
 	counter_update = COUNTER_UPDATE_ATOMIC_BUILTIN;
     }
   else if (gcov_type_size == 8 && have_atomic_4)
-      counter_update = COUNTER_UPDATE_ATOMIC_PARTIAL;
+    counter_update = COUNTER_UPDATE_ATOMIC_PARTIAL;
 
   /* This is a small-ipa pass that gets called only once, from
      cgraphunit.cc:ipa_passes().  */
--- libgcc/libgcov.h.jj	2023-11-20 09:50:08.434204617 +0100
+++ libgcc/libgcov.h	2023-11-21 10:47:50.320481543 +0100
@@ -96,9 +96,9 @@ typedef unsigned gcov_type_unsigned __at
 #endif
 
 /* Detect whether target can support atomic update of profilers.  */
-#if (__SIZEOF_LONG_LONG__ == 4 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4) || \
-    (__SIZEOF_LONG_LONG__ == 8 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8) || \
-    defined (__LIBGCC_HAVE_LIBATOMIC)
+#if (__SIZEOF_LONG_LONG__ == 4 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4) \
+    || (__SIZEOF_LONG_LONG__ == 8 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8) \
+    || defined (__LIBGCC_HAVE_LIBATOMIC)
 #define GCOV_SUPPORTS_ATOMIC 1
 #else
 #define GCOV_SUPPORTS_ATOMIC 0

	Jakub


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-11-21  9:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-14 22:08 [PATCH 0/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
2023-11-14 22:08 ` [PATCH 1/4] gcov: Remove TARGET_GCOV_TYPE_SIZE target hook Sebastian Huber
2023-11-14 22:08 ` [PATCH 2/4] Add TARGET_HAVE_LIBATOMIC Sebastian Huber
2023-11-14 22:08 ` [PATCH 3/4] gcov: Add gen_counter_update() Sebastian Huber
2023-11-19  9:00   ` Dimitar Dimitrov
2023-11-19 14:43     ` Sebastian Huber
2023-11-14 22:08 ` [PATCH 4/4] gcov: Improve -fprofile-update=atomic Sebastian Huber
2023-11-15  5:51   ` Sebastian Huber
2023-11-21  9:53     ` Jakub Jelinek
2023-11-15 23:32 ` [PATCH 0/4] " Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).