diff mbox

Make direct emission of time profiler counter

Message ID e2055cc0-3158-7e45-6846-7c91f7277cf7@suse.cz
State Accepted
Commit 7d29f8e3dc105bcc948377a3c68250f555119d8c
Headers show

Commit Message

Martin Liška Nov. 3, 2016, 2:52 p.m. UTC 
Hello.

As Honza noticed we spent quite some time in __gcov_time_profiler:

perf report for Postgres make check command:
   4.10%  postgres  postgres[.]__gcov_time_profiler

Thus I rewrote the profiling code directly to GIMPLE statements:

  _4 = __gcov7.main[0];
  if (_4 == 0)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 3>:
  time_profile_5 = __gcov_time_profiler_counter;
  time_profile_6 = time_profile_5 + 1;
  __gcov7.main[0] = time_profile_6;
  __gcov_time_profiler_counter = time_profile_6;

thread-safe variant of the patch:

  _3 = __gcov7.foo[0];
  if (_3 == 0)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 3>:
  time_profiler_counter_ptr_4 = (long int) &__gcov_time_profiler_counter;
  time_profile_5 = __atomic_add_fetch_8 (time_profiler_counter_ptr_4, 1, 0);
  time_profile_6 = (long int) time_profile_5;
  __gcov7.foo[0] = time_profile_6;


Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

Comments

Jan Hubicka Nov. 3, 2016, 3:11 p.m. UTC | #1 
> 

> 2016-10-31  Martin Liska  <mliska@suse.cz>

> 

> 	* libgcov-profiler.c (__gcov_time_profiler): Remove.

> 	(__gcov_time_profiler_atomic): Likewise.

> 

> gcc/ChangeLog:

> 

> 2016-10-31  Martin Liska  <mliska@suse.cz>

> 

> 	* profile.c (instrument_values): Fix coding style.

> 	(branch_prob): Use renamed function.

> 	* tree-profile.c (init_ic_make_global_vars): Likewise.

> 	(gimple_init_edge_profiler): Rename to

> 	gimple_init_gcov_profiler.

> 	tree_time_profiler_counter variable declaration.

> 	(gimple_gen_time_profiler): Rewrite to do a direct gimple code

> 	emission.

> 	* value-prof.h: Remove an argument.

> 

> gcc/testsuite/ChangeLog:

> 

> 2016-11-03  Martin Liska  <mliska@suse.cz>

> 

> 	* gcc.dg/no_profile_instrument_function-attr-1.c: Update scanned

> 	output.

> 	* gcc.dg/tree-prof/time-profiler-3.c: New test.


OK,
Thanks!
Honza
Christophe Lyon Nov. 4, 2016, 9:18 a.m. UTC | #2 
On 3 November 2016 at 16:11, Jan Hubicka <hubicka@ucw.cz> wrote:
>>

>> 2016-10-31  Martin Liska  <mliska@suse.cz>

>>

>>       * libgcov-profiler.c (__gcov_time_profiler): Remove.

>>       (__gcov_time_profiler_atomic): Likewise.

>>

>> gcc/ChangeLog:

>>

>> 2016-10-31  Martin Liska  <mliska@suse.cz>

>>

>>       * profile.c (instrument_values): Fix coding style.

>>       (branch_prob): Use renamed function.

>>       * tree-profile.c (init_ic_make_global_vars): Likewise.

>>       (gimple_init_edge_profiler): Rename to

>>       gimple_init_gcov_profiler.

>>       tree_time_profiler_counter variable declaration.

>>       (gimple_gen_time_profiler): Rewrite to do a direct gimple code

>>       emission.

>>       * value-prof.h: Remove an argument.

>>

>> gcc/testsuite/ChangeLog:

>>

>> 2016-11-03  Martin Liska  <mliska@suse.cz>

>>

>>       * gcc.dg/no_profile_instrument_function-attr-1.c: Update scanned

>>       output.

>>       * gcc.dg/tree-prof/time-profiler-3.c: New test.

>

> OK,

> Thanks!

> Honza


Hi,

It seems this patch causes an ICE when compiling
gcc.dg/gomp/pr27573.c
for instance on arm-linux-gnueabi --wtih-cpu=cortex-a9

The backtrace is:
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/gomp/pr27573.c:
In function 'main._omp_fn.0':
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/gomp/pr27573.c:12:9:
internal compiler error: in convert_memory_address_addr_sp
ace_1, at explow.c:284
0x79b19e convert_memory_address_addr_space_1(machine_mode, rtx_def*,
unsigned char, bool, bool)
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/explow.c:284
0x670903 get_builtin_sync_mem
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/builtins.c:4933
0x671fef expand_builtin_atomic_fetch_op
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/builtins.c:5452
0x674aa4 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/builtins.c:6859
0x7b729a expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/expr.c:10773
0x7c1460 store_expr_with_bounds(tree_node*, rtx_def*, int, bool, bool,
tree_node*)
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/expr.c:5551
0x7c7451 expand_assignment(tree_node*, tree_node*, bool)
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/expr.c:5317
0x69a0de expand_call_stmt
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:2666
0x69b264 expand_gimple_stmt_1
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:3581
0x69b264 expand_gimple_stmt
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:3747
0x69cc8a expand_gimple_basic_block
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:5754
0x69fefe execute
        /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:6368
Please submit a full bug report,

Christophe
diff mbox

Patch

From 1a277ec6ef25cddbf0518b679fa8f1928cd3d891 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 27 Oct 2016 09:37:34 +0200
Subject: [PATCH] Make direct emission of time profiler counter

libgcc/ChangeLog:

2016-10-31  Martin Liska  <mliska@suse.cz>

	* libgcov-profiler.c (__gcov_time_profiler): Remove.
	(__gcov_time_profiler_atomic): Likewise.

gcc/ChangeLog:

2016-10-31  Martin Liska  <mliska@suse.cz>

	* profile.c (instrument_values): Fix coding style.
	(branch_prob): Use renamed function.
	* tree-profile.c (init_ic_make_global_vars): Likewise.
	(gimple_init_edge_profiler): Rename to
	gimple_init_gcov_profiler.
	tree_time_profiler_counter variable declaration.
	(gimple_gen_time_profiler): Rewrite to do a direct gimple code
	emission.
	* value-prof.h: Remove an argument.

gcc/testsuite/ChangeLog:

2016-11-03  Martin Liska  <mliska@suse.cz>

	* gcc.dg/no_profile_instrument_function-attr-1.c: Update scanned
	output.
	* gcc.dg/tree-prof/time-profiler-3.c: New test.
---
 gcc/profile.c                                      |  14 +--
 .../gcc.dg/no_profile_instrument_function-attr-1.c |   2 +-
 gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c   |  22 +++++
 gcc/tree-profile.c                                 | 106 ++++++++++++++++-----
 gcc/value-prof.h                                   |   5 +-
 libgcc/libgcov-profiler.c                          |  23 +----
 6 files changed, 111 insertions(+), 61 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c

diff --git a/gcc/profile.c b/gcc/profile.c
index 2564f07..ef38f98 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -192,15 +192,9 @@  instrument_values (histogram_values values)
 	  gimple_gen_ior_profiler (hist, t, 0);
 	  break;
 
-  case HIST_TYPE_TIME_PROFILE:
-    {
-      basic_block bb =
-     split_edge (single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
-      gimple_stmt_iterator gsi = gsi_start_bb (bb);
-
-      gimple_gen_time_profiler (t, 0, gsi);
-      break;
-    }
+	case HIST_TYPE_TIME_PROFILE:
+	  gimple_gen_time_profiler (t, 0);
+	  break;
 
 	default:
 	  gcc_unreachable ();
@@ -1305,7 +1299,7 @@  branch_prob (void)
     {
       unsigned n_instrumented;
 
-      gimple_init_edge_profiler ();
+      gimple_init_gcov_profiler ();
 
       n_instrumented = instrument_edges (el);
 
diff --git a/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-1.c b/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-1.c
index c93d171..e0c2600 100644
--- a/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-1.c
+++ b/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-1.c
@@ -19,5 +19,5 @@  int main ()
 
 /* { dg-final { scan-tree-dump-times "__gcov0\\.main.* = PROF_edge_counter" 1 "optimized"} } */
 /* { dg-final { scan-tree-dump-times "__gcov_indirect_call_profiler_v2" 1 "optimized" } } */
-/* { dg-final { scan-tree-dump-times "__gcov_time_profiler" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__gcov_time_profiler_counter = " 1 "optimized" } } */
 /* { dg-final { scan-tree-dump-times "__gcov_init" 1 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c b/gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c
new file mode 100644
index 0000000..69ce026
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-prof/time-profiler-3.c
@@ -0,0 +1,22 @@ 
+/* { dg-options "-O2 -fdump-ipa-profile -fprofile-update=atomic" } */
+/* { dg-require-effective-target profile_update_atomic } */
+
+__attribute__ ((noinline))
+int foo()
+{
+  return 0;
+}
+
+__attribute__ ((noinline))
+int bar()
+{
+  return 1;
+}
+
+int main ()
+{
+  return foo ();
+}
+/* { dg-final-use-not-autofdo { scan-ipa-dump-times "Read tp_first_run: 0" 1 "profile"} } */
+/* { dg-final-use-not-autofdo { scan-ipa-dump-times "Read tp_first_run: 1" 1 "profile"} } */
+/* { dg-final-use-not-autofdo { scan-ipa-dump-times "Read tp_first_run: 2" 1 "profile"} } */
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index abeee92..09a702f 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -56,9 +56,9 @@  static GTY(()) tree tree_interval_profiler_fn;
 static GTY(()) tree tree_pow2_profiler_fn;
 static GTY(()) tree tree_one_value_profiler_fn;
 static GTY(()) tree tree_indirect_call_profiler_fn;
-static GTY(()) tree tree_time_profiler_fn;
 static GTY(()) tree tree_average_profiler_fn;
 static GTY(()) tree tree_ior_profiler_fn;
+static GTY(()) tree tree_time_profiler_counter;
 
 
 static GTY(()) tree ic_void_ptr_var;
@@ -75,7 +75,7 @@  static GTY(()) tree ptr_void;
 static void
 init_ic_make_global_vars (void)
 {
-  tree  gcov_type_ptr;
+  tree gcov_type_ptr;
 
   ptr_void = build_pointer_type (void_type_node);
 
@@ -119,7 +119,7 @@  init_ic_make_global_vars (void)
 /* Create the type and function decls for the interface with gcov.  */
 
 void
-gimple_init_edge_profiler (void)
+gimple_init_gcov_profiler (void)
 {
   tree interval_profiler_fn_type;
   tree pow2_profiler_fn_type;
@@ -127,7 +127,6 @@  gimple_init_edge_profiler (void)
   tree gcov_type_ptr;
   tree ic_profiler_fn_type;
   tree average_profiler_fn_type;
-  tree time_profiler_fn_type;
   const char *profiler_fn_name;
   const char *fn_name;
 
@@ -201,17 +200,17 @@  gimple_init_edge_profiler (void)
 	= tree_cons (get_identifier ("leaf"), NULL,
 		     DECL_ATTRIBUTES (tree_indirect_call_profiler_fn));
 
-      /* void (*) (gcov_type *, gcov_type, void *)  */
-      time_profiler_fn_type
-	       = build_function_type_list (void_type_node,
-					  gcov_type_ptr, NULL_TREE);
-      fn_name = concat ("__gcov_time_profiler", fn_suffix, NULL);
-      tree_time_profiler_fn = build_fn_decl (fn_name, time_profiler_fn_type);
-      free (CONST_CAST (char *, fn_name));
-      TREE_NOTHROW (tree_time_profiler_fn) = 1;
-      DECL_ATTRIBUTES (tree_time_profiler_fn)
-	= tree_cons (get_identifier ("leaf"), NULL,
-		     DECL_ATTRIBUTES (tree_time_profiler_fn));
+      tree_time_profiler_counter
+	= build_decl (UNKNOWN_LOCATION, VAR_DECL,
+		      get_identifier ("__gcov_time_profiler_counter"),
+		      get_gcov_type ());
+      TREE_PUBLIC (tree_time_profiler_counter) = 1;
+      DECL_EXTERNAL (tree_time_profiler_counter) = 1;
+      TREE_STATIC (tree_time_profiler_counter) = 1;
+      DECL_ARTIFICIAL (tree_time_profiler_counter) = 1;
+      DECL_INITIAL (tree_time_profiler_counter) = NULL;
+
+      varpool_node::finalize_decl (tree_time_profiler_counter);
 
       /* void (*) (gcov_type *, gcov_type)  */
       average_profiler_fn_type
@@ -239,7 +238,6 @@  gimple_init_edge_profiler (void)
       DECL_ASSEMBLER_NAME (tree_pow2_profiler_fn);
       DECL_ASSEMBLER_NAME (tree_one_value_profiler_fn);
       DECL_ASSEMBLER_NAME (tree_indirect_call_profiler_fn);
-      DECL_ASSEMBLER_NAME (tree_time_profiler_fn);
       DECL_ASSEMBLER_NAME (tree_average_profiler_fn);
       DECL_ASSEMBLER_NAME (tree_ior_profiler_fn);
     }
@@ -426,7 +424,7 @@  gimple_gen_ic_func_profiler (void)
   if (c_node->only_called_directly_p ())
     return;
 
-  gimple_init_edge_profiler ();
+  gimple_init_gcov_profiler ();
 
   /* Insert code:
 
@@ -460,16 +458,74 @@  gimple_gen_ic_func_profiler (void)
    counter position and GSI is the iterator we place the counter.  */
 
 void
-gimple_gen_time_profiler (unsigned tag, unsigned base,
-                          gimple_stmt_iterator &gsi)
+gimple_gen_time_profiler (unsigned tag, unsigned base)
 {
-  tree ref_ptr = tree_coverage_counter_addr (tag, base);
-  gcall *call;
+  tree type = get_gcov_type ();
+  basic_block cond_bb
+    = split_edge (single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
+
+  basic_block update_bb = split_edge (single_succ_edge (cond_bb));
+
+  edge true_edge = single_succ_edge (cond_bb);
+  true_edge->flags = EDGE_TRUE_VALUE;
+  true_edge->probability = PROB_VERY_UNLIKELY;
+  edge e
+    = make_edge (cond_bb, single_succ_edge (update_bb)->dest, EDGE_FALSE_VALUE);
+  e->probability = REG_BR_PROB_BASE - true_edge->probability;
+
+  gimple_stmt_iterator gsi = gsi_start_bb (cond_bb);
+  tree original_ref = tree_coverage_counter_ref (tag, base);
+  tree ref = force_gimple_operand_gsi (&gsi, original_ref, true, NULL_TREE,
+				       true, GSI_SAME_STMT);
+  tree one = build_int_cst (type, 1);
 
-  ref_ptr = force_gimple_operand_gsi (&gsi, ref_ptr,
-				      true, NULL_TREE, true, GSI_SAME_STMT);
-  call = gimple_build_call (tree_time_profiler_fn, 1, ref_ptr);
-  gsi_insert_before (&gsi, call, GSI_NEW_STMT);
+  /* 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 (type, NULL, "time_profiler_counter_ptr");
+      tree addr = build1 (ADDR_EXPR, build_pointer_type (type),
+			  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 (LONG_LONG_TYPE_SIZE > 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, "time_profile");
+      gimple_set_lhs (stmt, tmp);
+      gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
+      tmp = make_temp_ssa_name (type, NULL, "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, "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, "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);
+    }
 }
 
 /* Output instructions as GIMPLE trees to increment the average histogram
diff --git a/gcc/value-prof.h b/gcc/value-prof.h
index 07e2b3b..02220ac 100644
--- a/gcc/value-prof.h
+++ b/gcc/value-prof.h
@@ -96,15 +96,14 @@  bool check_ic_target (gcall *, struct cgraph_node *);
 
 
 /* In tree-profile.c.  */
-extern void gimple_init_edge_profiler (void);
+extern void gimple_init_gcov_profiler (void);
 extern void gimple_gen_edge_profiler (int, edge);
 extern void gimple_gen_interval_profiler (histogram_value, unsigned, unsigned);
 extern void gimple_gen_pow2_profiler (histogram_value, unsigned, unsigned);
 extern void gimple_gen_one_value_profiler (histogram_value, unsigned, unsigned);
 extern void gimple_gen_ic_profiler (histogram_value, unsigned, unsigned);
 extern void gimple_gen_ic_func_profiler (void);
-extern void gimple_gen_time_profiler (unsigned, unsigned,
-                                      gimple_stmt_iterator &);
+extern void gimple_gen_time_profiler (unsigned, unsigned);
 extern void gimple_gen_average_profiler (histogram_value, unsigned, unsigned);
 extern void gimple_gen_ior_profiler (histogram_value, unsigned, unsigned);
 extern void stream_out_histogram_value (struct output_block *, histogram_value);
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 38ed5f1..4f0a406 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -342,30 +342,9 @@  __gcov_indirect_call_profiler_v2 (gcov_type value, void* cur_func)
 #ifdef L_gcov_time_profiler
 
 /* Counter for first visit of each function.  */
-static gcov_type function_counter;
+gcov_type __gcov_time_profiler_counter ATTRIBUTE_HIDDEN;
 
-/* Sets corresponding COUNTERS if there is no value.  */
-
-void
-__gcov_time_profiler (gcov_type* counters)
-{
-  if (!counters[0])
-    counters[0] = ++function_counter;
-}
-
-#if GCOV_SUPPORTS_ATOMIC
-/* Sets corresponding COUNTERS if there is no value.
-   Function is thread-safe.  */
-
-void
-__gcov_time_profiler_atomic (gcov_type* counters)
-{
-  if (!counters[0])
-    counters[0] = __atomic_add_fetch (&function_counter, 1, __ATOMIC_RELAXED);
-}
 #endif
-#endif
-
 
 #ifdef L_gcov_average_profiler
 /* Increase corresponding COUNTER by VALUE.  FIXME: Perhaps we want
-- 
2.10.1