[PATCH 2/4] Implement N disk counters for single value and indirect call counters.

[PATCH 2/4] Implement N disk counters for single value and indirect call counters.

[marxin]

[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]


gcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* gcov-io.h (GCOV_DISK_SINGLE_VALUES): New.
	(GCOV_SINGLE_VALUE_COUNTERS): Likewise.
	* ipa-profile.c (ipa_profile_generate_summary):
	Use get_most_common_single_value.
	* tree-profile.c (gimple_init_gcov_profiler):
	Instrument with __gcov_one_value_profiler_v2
	and __gcov_indirect_call_profiler_v4.
	* value-prof.c (dump_histogram_value):
	Print all values for HIST_TYPE_SINGLE_VALUE.
	(stream_in_histogram_value): Set number of
	counters for HIST_TYPE_SINGLE_VALUE.
	(get_most_common_single_value): New.
	(gimple_divmod_fixed_value_transform):
	Use get_most_common_single_value.
	(gimple_ic_transform): Likewise.
	(gimple_stringops_transform): Likewise.
	(gimple_find_values_to_profile): Set number
	of counters for HIST_TYPE_SINGLE_VALUE.
	* value-prof.h (get_most_common_single_value):
	New.

libgcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* Makefile.in: Add __gcov_one_value_profiler_v2 and
	__gcov_indirect_call_profiler_v4.
	* libgcov-merge.c (__gcov_merge_single): Change
	function signature.
	(merge_single_value_set): New.
	* libgcov-profiler.c (__gcov_one_value_profiler_body):
	Do not update counters[2].
	(__gcov_one_value_profiler): Remove.
	(__gcov_one_value_profiler_atomic): Rename to ...
	(__gcov_one_value_profiler_v2): ... this.
	(__gcov_indirect_call_profiler_v3): Rename to ...
	(__gcov_indirect_call_profiler_v4): ... this.
	* libgcov.h (__gcov_one_value_profiler): Remove.
	(__gcov_one_value_profiler_atomic): Remove.
	(__gcov_indirect_call_profiler_v3): Remove.
	(__gcov_one_value_profiler_v2): New.
	(__gcov_indirect_call_profiler_v4): New.
---
 gcc/gcov-io.h             |   7 +++
 gcc/ipa-profile.c         |  13 +++--
 gcc/tree-profile.c        |   9 ++-
 gcc/value-prof.c          | 120 ++++++++++++++++++++------------------
 gcc/value-prof.h          |   2 +
 libgcc/Makefile.in        |   5 +-
 libgcc/libgcov-merge.c    |  77 ++++++++++++++++--------
 libgcc/libgcov-profiler.c |  43 +++-----------
 libgcc/libgcov.h          |   5 +-
 9 files changed, 147 insertions(+), 134 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Implement-N-disk-counters-for-single-value-and-indir.patch --]
[-- Type: text/x-patch; name="0002-Implement-N-disk-counters-for-single-value-and-indir.patch", Size: 18669 bytes --]

diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index 69c9a73dba8..161518176a0 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -266,6 +266,13 @@ GCOV_COUNTERS
 #define GCOV_N_VALUE_COUNTERS \
   (GCOV_LAST_VALUE_COUNTER - GCOV_FIRST_VALUE_COUNTER + 1)
 
+/* Number of single value histogram values that live
+   on disk representation.  */
+#define GCOV_DISK_SINGLE_VALUES 4
+
+/* Total number of single value counters.  */
+#define GCOV_SINGLE_VALUE_COUNTERS (2 * GCOV_DISK_SINGLE_VALUES)
+
 /* 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/ipa-profile.c b/gcc/ipa-profile.c
index de9563d808c..fc2ffbd84f7 100644
--- a/gcc/ipa-profile.c
+++ b/gcc/ipa-profile.c
@@ -191,17 +191,18 @@ ipa_profile_generate_summary (void)
 		     takes away bad histograms.  */
 		  if (h)
 		    {
-		      /* counter 0 is target, counter 1 is number of execution we called target,
-			 counter 2 is total number of executions.  */
-		      if (h->hvalue.counters[2])
+		      gcov_type val, count;
+		      if (get_most_common_single_value (h, &val, &count))
 			{
 			  struct cgraph_edge * e = node->get_edge (stmt);
 			  if (e && !e->indirect_unknown_callee)
 			    continue;
-			  e->indirect_info->common_target_id
-			    = h->hvalue.counters [0];
+
+			  gcov_type all
+			    = gimple_bb (stmt)->count.ipa ().to_gcov_type ();
+			  e->indirect_info->common_target_id = val;
 			  e->indirect_info->common_target_probability
-			    = GCOV_COMPUTE_SCALE (h->hvalue.counters [1], h->hvalue.counters [2]);
+			    = GCOV_COMPUTE_SCALE (count, all);
 			  if (e->indirect_info->common_target_probability > REG_BR_PROB_BASE)
 			    {
 			      if (dump_file)
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index f2cf4047579..008a1271979 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -165,10 +165,9 @@ gimple_init_gcov_profiler (void)
 	      = build_function_type_list (void_type_node,
 					  gcov_type_ptr, gcov_type_node,
 					  NULL_TREE);
-      fn_name = concat ("__gcov_one_value_profiler", fn_suffix, NULL);
-      tree_one_value_profiler_fn = build_fn_decl (fn_name,
-						  one_value_profiler_fn_type);
-      free (CONST_CAST (char *, fn_name));
+      tree_one_value_profiler_fn
+	= build_fn_decl ("__gcov_one_value_profiler_v2",
+			 one_value_profiler_fn_type);
       TREE_NOTHROW (tree_one_value_profiler_fn) = 1;
       DECL_ATTRIBUTES (tree_one_value_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -182,7 +181,7 @@ gimple_init_gcov_profiler (void)
 					  gcov_type_node,
 					  ptr_type_node,
 					  NULL_TREE);
-      profiler_fn_name = "__gcov_indirect_call_profiler_v3";
+      profiler_fn_name = "__gcov_indirect_call_profiler_v4";
 
       tree_indirect_call_profiler_fn
 	      = build_fn_decl (profiler_fn_name, ic_profiler_fn_type);
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index 1e14e532070..e893ca084c9 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -259,15 +259,19 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
       break;
 
     case HIST_TYPE_SINGLE_VALUE:
-      fprintf (dump_file, "Single value ");
+    case HIST_TYPE_INDIR_CALL:
+      fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
+			   ? "Single values " : "Indirect call "));
       if (hist->hvalue.counters)
 	{
-	   fprintf (dump_file, "value:%" PRId64
-		    " match:%" PRId64
-		    " wrong:%" PRId64,
-		    (int64_t) hist->hvalue.counters[0],
-		    (int64_t) hist->hvalue.counters[1],
-		    (int64_t) hist->hvalue.counters[2]);
+	  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
+	    {
+	      fprintf (dump_file, "[%" PRId64 ":%" PRId64 "]",
+		       (int64_t) hist->hvalue.counters[2 * i],
+		       (int64_t) hist->hvalue.counters[2 * i + 1]);
+	      if (i != GCOV_DISK_SINGLE_VALUES - 1)
+		fprintf (dump_file, ", ");
+	    }
 	}
       fprintf (dump_file, ".\n");
       break;
@@ -294,19 +298,6 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
       fprintf (dump_file, ".\n");
       break;
 
-    case HIST_TYPE_INDIR_CALL:
-      fprintf (dump_file, "Indirect call ");
-      if (hist->hvalue.counters)
-	{
-	   fprintf (dump_file, "value:%" PRId64
-		    " match:%" PRId64
-		    " all:%" PRId64,
-		    (int64_t) hist->hvalue.counters[0],
-		    (int64_t) hist->hvalue.counters[1],
-		    (int64_t) hist->hvalue.counters[2]);
-	}
-      fprintf (dump_file, ".\n");
-      break;
     case HIST_TYPE_TIME_PROFILE:
       fprintf (dump_file, "Time profile ");
       if (hist->hvalue.counters)
@@ -392,7 +383,7 @@ stream_in_histogram_value (struct lto_input_block *ib, gimple *stmt)
 
 	case HIST_TYPE_SINGLE_VALUE:
 	case HIST_TYPE_INDIR_CALL:
-	  ncounters = 3;
+	  ncounters = GCOV_SINGLE_VALUE_COUNTERS;
 	  break;
 
 	case HIST_TYPE_IOR:
@@ -729,6 +720,36 @@ gimple_divmod_fixed_value (gassign *stmt, tree value, profile_probability prob,
   return tmp2;
 }
 
+/* Return most common value of SINGLE_VALUE histogram.  If
+   there's a unique value, return true and set VALUE and COUNT
+   arguments.  */
+
+bool
+get_most_common_single_value (histogram_value hist,
+			      gcov_type *value, gcov_type *count)
+{
+  if (hist->hvalue.counters[1] == -1)
+    return false;
+
+  *count = 0;
+  *value = 0;
+
+  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
+    {
+      gcov_type v = hist->hvalue.counters[2 * i];
+      gcov_type c = hist->hvalue.counters[2 * i + 1];
+      if (c > *count)
+	{
+	  *value = v;
+	  *count = c;
+	}
+      else if (c == *count && v > *value)
+	*value = v;
+    }
+
+  return true;
+}
+
 /* Do transform 1) on INSN if applicable.  */
 
 static bool
@@ -758,23 +779,19 @@ gimple_divmod_fixed_value_transform (gimple_stmt_iterator *si)
   if (!histogram)
     return false;
 
+  if (!get_most_common_single_value (histogram, &val, &count))
+    return false;
+
   value = histogram->hvalue.value;
-  val = histogram->hvalue.counters[0];
-  count = histogram->hvalue.counters[1];
-  all = histogram->hvalue.counters[2];
+  all = gimple_bb (stmt)->count.ipa ().to_gcov_type ();
   gimple_remove_histogram_value (cfun, stmt, histogram);
 
-  /* We require that count is at least half of all; this means
-     that for the transformation to fire the value must be constant
-     at least 50% of time (and 75% gives the guarantee of usage).  */
+  /* We require that count is at least half of all.  */
   if (simple_cst_equal (gimple_assign_rhs2 (stmt), value) != 1
       || 2 * count < all
       || optimize_bb_for_size_p (gimple_bb (stmt)))
     return false;
 
-  if (check_counter (stmt, "value", &count, &all, gimple_bb (stmt)->count))
-    return false;
-
   /* Compute probability of taking the optimal path.  */
   if (all > 0)
     prob = profile_probability::probability_in_gcov_type (count, all);
@@ -1401,7 +1418,7 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
 {
   gcall *stmt;
   histogram_value histogram;
-  gcov_type val, count, all, bb_all;
+  gcov_type val, count, all;
   struct cgraph_node *direct_call;
 
   stmt = dyn_cast <gcall *> (gsi_stmt (*gsi));
@@ -1418,21 +1435,10 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
   if (!histogram)
     return false;
 
-  val = histogram->hvalue.counters [0];
-  count = histogram->hvalue.counters [1];
-  all = histogram->hvalue.counters [2];
-
-  bb_all = gimple_bb (stmt)->count.ipa ().to_gcov_type ();
-  /* The order of CHECK_COUNTER calls is important -
-     since check_counter can correct the third parameter
-     and we want to make count <= all <= bb_all. */
-  if (check_counter (stmt, "ic", &all, &bb_all, gimple_bb (stmt)->count)
-      || check_counter (stmt, "ic", &count, &all,
-		        profile_count::from_gcov_type (all)))
-    {
-      gimple_remove_histogram_value (cfun, stmt, histogram);
-      return false;
-    }
+  if (!get_most_common_single_value (histogram, &val, &count))
+    return false;
+
+  all = gimple_bb (stmt)->count.ipa ().to_gcov_type ();
 
   if (4 * count <= 3 * all)
     return false;
@@ -1644,19 +1650,19 @@ gimple_stringops_transform (gimple_stmt_iterator *gsi)
   if (TREE_CODE (blck_size) == INTEGER_CST)
     return false;
 
-  histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_SINGLE_VALUE);
+  histogram = gimple_histogram_value_of_type (cfun, stmt,
+					      HIST_TYPE_SINGLE_VALUE);
   if (!histogram)
     return false;
 
-  val = histogram->hvalue.counters[0];
-  count = histogram->hvalue.counters[1];
-  all = histogram->hvalue.counters[2];
+  if (!get_most_common_single_value (histogram, &val, &count))
+    return false;
+
+  all = gimple_bb (stmt)->count.ipa ().to_gcov_type ();
   gimple_remove_histogram_value (cfun, stmt, histogram);
 
-  /* We require that count is at least half of all; this means
-     that for the transformation to fire the value must be constant
-     at least 80% of time.  */
-  if ((6 * count / 5) < all || optimize_bb_for_size_p (gimple_bb (stmt)))
+  /* We require that count is at least half of all.  */
+  if (2 * count < all || optimize_bb_for_size_p (gimple_bb (stmt)))
     return false;
   if (check_counter (stmt, "value", &count, &all, gimple_bb (stmt)->count))
     return false;
@@ -1928,11 +1934,11 @@ gimple_find_values_to_profile (histogram_values *values)
 	  break;
 
 	case HIST_TYPE_SINGLE_VALUE:
-	  hist->n_counters = 3;
+	  hist->n_counters = GCOV_SINGLE_VALUE_COUNTERS;
 	  break;
 
- 	case HIST_TYPE_INDIR_CALL:
- 	  hist->n_counters = 3;
+	case HIST_TYPE_INDIR_CALL:
+	  hist->n_counters = GCOV_SINGLE_VALUE_COUNTERS;
 	  break;
 
         case HIST_TYPE_TIME_PROFILE:
diff --git a/gcc/value-prof.h b/gcc/value-prof.h
index a54024b48de..948f879c495 100644
--- a/gcc/value-prof.h
+++ b/gcc/value-prof.h
@@ -90,6 +90,8 @@ void free_histograms (function *);
 void stringop_block_profile (gimple *, unsigned int *, HOST_WIDE_INT *);
 gcall *gimple_ic (gcall *, struct cgraph_node *, profile_probability);
 bool check_ic_target (gcall *, struct cgraph_node *);
+bool get_most_common_single_value (histogram_value hist,
+				   gcov_type *value, gcov_type *count);
 
 
 /* In tree-profile.c.  */
diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index fb77881145e..2ccde55f547 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -893,13 +893,12 @@ LIBGCOV_PROFILER = _gcov_interval_profiler				\
 	_gcov_interval_profiler_atomic					\
 	_gcov_pow2_profiler						\
 	_gcov_pow2_profiler_atomic					\
-	_gcov_one_value_profiler					\
-	_gcov_one_value_profiler_atomic					\
+	_gcov_one_value_profiler_v2					\
 	_gcov_average_profiler						\
 	_gcov_average_profiler_atomic					\
 	_gcov_ior_profiler						\
 	_gcov_ior_profiler_atomic					\
-	_gcov_indirect_call_profiler_v3					\
+	_gcov_indirect_call_profiler_v4					\
 	_gcov_time_profiler
 LIBGCOV_INTERFACE = _gcov_dump _gcov_flush _gcov_fork			\
 	_gcov_execl _gcov_execlp					\
diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
index 702a69f8349..41eac461c98 100644
--- a/libgcc/libgcov-merge.c
+++ b/libgcc/libgcov-merge.c
@@ -34,8 +34,9 @@ void __gcov_merge_add (gcov_type *counters  __attribute__ ((unused)),
 #endif
 
 #ifdef L_gcov_merge_single
-void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)),
-                          unsigned n_counters __attribute__ ((unused))) {}
+void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
+{
+}
 #endif
 
 #else
@@ -85,40 +86,66 @@ __gcov_merge_time_profile (gcov_type *counters, unsigned n_counters)
 #endif /* L_gcov_merge_time_profile */
 
 #ifdef L_gcov_merge_single
+
+static void
+merge_single_value_set (gcov_type *counters)
+{
+  unsigned j;
+  gcov_type value, counter;
+  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
+    {
+      value = gcov_get_counter_target ();
+      counter = gcov_get_counter ();
+
+      if (counter == -1)
+	{
+	  counters[1] = -1;
+	  /* We can't return as we need to read all counters.  */
+	  continue;
+	}
+      else if (counter == 0 || counters[1] == -1)
+	{
+	  /* We can't return as we need to read all counters.  */
+	  continue;
+	}
+
+      for (j = 0; j < GCOV_DISK_SINGLE_VALUES; j++)
+	{
+	  if (counters[2 * j] == value)
+	    {
+	      counters[2 * j + 1] += counter;
+	      break;
+	    }
+	  else if (counters[2 * j + 1] == 0)
+	    {
+	      counters[2 * j] = value;
+	      counters[2 * j + 1] = counter;
+	      break;
+	    }
+	}
+
+      /* We haven't found a free slot for the value, mark overflow.  */
+      if (j == GCOV_DISK_SINGLE_VALUES)
+	counters[1] = -1;
+    }
+}
+
 /* The profile merging function for choosing the most common value.
    It is given an array COUNTERS of N_COUNTERS old counters and it
    reads the same number of counters from the gcov file.  The counters
-   are split into 3-tuples where the members of the tuple have
+   are split into pairs where the members of the tuple have
    meanings:
 
    -- the stored candidate on the most common value of the measured entity
    -- counter
-   -- total number of evaluations of the value  */
+   */
 void
 __gcov_merge_single (gcov_type *counters, unsigned n_counters)
 {
-  unsigned i, n_measures;
-  gcov_type value, counter, all;
+  gcc_assert (!(n_counters % GCOV_SINGLE_VALUE_COUNTERS));
 
-  gcc_assert (!(n_counters % 3));
-  n_measures = n_counters / 3;
-  for (i = 0; i < n_measures; i++, counters += 3)
-    {
-      value = gcov_get_counter_target ();
-      counter = gcov_get_counter ();
-      all = gcov_get_counter ();
-
-      if (counters[0] == value)
-        counters[1] += counter;
-      else if (counter > counters[1])
-        {
-          counters[0] = value;
-          counters[1] = counter - counters[1];
-        }
-      else
-        counters[1] -= counter;
-      counters[2] += all;
-    }
+  for (unsigned i = 0; i < (n_counters / GCOV_SINGLE_VALUE_COUNTERS); i++)
+    merge_single_value_set (counters + (i * GCOV_SINGLE_VALUE_COUNTERS));
 }
 #endif /* L_gcov_merge_single */
 
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 40f0858a174..d7bf72fdf31 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -112,14 +112,10 @@ __gcov_pow2_profiler_atomic (gcov_type *counters, gcov_type value)
    COUNTERS[1] is decremented.  Otherwise COUNTERS[1] is set to one and
    VALUE is stored to COUNTERS[0].  This algorithm guarantees that if this
    function is called more than 50% of the time with one value, this value
-   will be in COUNTERS[0] in the end.
-
-   In any case, COUNTERS[2] is incremented.  If USE_ATOMIC is set to 1,
-   COUNTERS[2] is updated with an atomic instruction.  */
+   will be in COUNTERS[0] in the end.  */
 
 static inline void
-__gcov_one_value_profiler_body (gcov_type *counters, gcov_type value,
-				int use_atomic)
+__gcov_one_value_profiler_body (gcov_type *counters, gcov_type value)
 {
   if (value == counters[0])
     counters[1]++;
@@ -130,40 +126,17 @@ __gcov_one_value_profiler_body (gcov_type *counters, gcov_type value,
     }
   else
     counters[1]--;
-
-  if (use_atomic)
-    __atomic_fetch_add (&counters[2], 1, __ATOMIC_RELAXED);
-  else
-    counters[2]++;
 }
 
-#ifdef L_gcov_one_value_profiler
-void
-__gcov_one_value_profiler (gcov_type *counters, gcov_type value)
-{
-  __gcov_one_value_profiler_body (counters, value, 0);
-}
-#endif
-
-#if defined(L_gcov_one_value_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
-
-/* Update one value profilers (COUNTERS) for a given VALUE.
-
-   CAVEAT: Following function is not thread-safe, only total number
-   of executions (COUNTERS[2]) is update with an atomic instruction.
-   Problem is that one cannot atomically update two counters
-   (COUNTERS[0] and COUNTERS[1]), for more information please read
-   following email thread:
-   https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00024.html.  */
-
+#ifdef L_gcov_one_value_profiler_v2
 void
-__gcov_one_value_profiler_atomic (gcov_type *counters, gcov_type value)
+__gcov_one_value_profiler_v2 (gcov_type *counters, gcov_type value)
 {
-  __gcov_one_value_profiler_body (counters, value, 1);
+  __gcov_one_value_profiler_body (counters, value);
 }
 #endif
 
-#ifdef L_gcov_indirect_call_profiler_v3
+#ifdef L_gcov_indirect_call_profiler_v4
 
 /* These two variables are used to actually track caller and callee.  Keep
    them in TLS memory so races are not common (they are written to often).
@@ -185,7 +158,7 @@ struct indirect_call_tuple __gcov_indirect_call;
 
 /* Tries to determine the most common value among its inputs. */
 void
-__gcov_indirect_call_profiler_v3 (gcov_type value, void* cur_func)
+__gcov_indirect_call_profiler_v4 (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
@@ -193,7 +166,7 @@ __gcov_indirect_call_profiler_v3 (gcov_type value, void* cur_func)
   if (cur_func == __gcov_indirect_call.callee
       || (__LIBGCC_VTABLE_USES_DESCRIPTORS__
 	  && *(void **) cur_func == *(void **) __gcov_indirect_call.callee))
-    __gcov_one_value_profiler_body (__gcov_indirect_call.counters, value, 0);
+    __gcov_one_value_profiler_body (__gcov_indirect_call.counters, value);
 
   __gcov_indirect_call.callee = NULL;
 }
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index b4f1ec576fc..b6224b3d192 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -271,9 +271,8 @@ extern void __gcov_interval_profiler_atomic (gcov_type *, gcov_type, int,
 					     unsigned);
 extern void __gcov_pow2_profiler (gcov_type *, gcov_type);
 extern void __gcov_pow2_profiler_atomic (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_v3 (gcov_type, void *);
+extern void __gcov_one_value_profiler_v2 (gcov_type *, gcov_type);
+extern void __gcov_indirect_call_profiler_v4 (gcov_type, void *);
 extern void __gcov_time_profiler (gcov_type *);
 extern void __gcov_time_profiler_atomic (gcov_type *);
 extern void __gcov_average_profiler (gcov_type *, gcov_type);

[PATCH 0/4] Store multiple values for single value profilers

[Martin Liska]

Hi.

It's becoming more common that a training run happens in parallel environment.
That can lead to a not reproducible builds caused by different order of merging
of .gcda files. So that I'm suggesting to store up to 4 values for HIST_TYPE_SINGLE_VALUE
and HIST_TYPE_INDIR_CALL on disk. If the capacity is exceeded the whole counter is
marked as unstable (not reproducible).

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

marxin (4):
  Remove indirect call top N counter type.
  Implement N disk counters for single value and indirect call counters.
  Dump histograms only if present.
  Update a bit dump format.

 gcc/doc/invoke.texi       |   3 -
 gcc/gcov-counter.def      |   3 -
 gcc/gcov-io.h             |   9 +-
 gcc/ipa-profile.c         |  13 ++-
 gcc/params.def            |   8 --
 gcc/profile.c             |   1 -
 gcc/tree-profile.c        |  23 +---
 gcc/value-prof.c          | 224 ++++++++++++++++----------------------
 gcc/value-prof.h          |   4 +-
 libgcc/Makefile.in        |  10 +-
 libgcc/libgcov-driver.c   |  80 --------------
 libgcc/libgcov-merge.c    | 139 +++++++++--------------
 libgcc/libgcov-profiler.c | 176 ++----------------------------
 libgcc/libgcov-util.c     |  19 ----
 libgcc/libgcov.h          |  12 +-
 15 files changed, 179 insertions(+), 545 deletions(-)

-- 
2.21.0

[PATCH 3/4] Dump histograms only if present.

[marxin]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]


gcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* value-prof.c (dump_histogram_value): Print histogram values
	only if present.
---
 gcc/value-prof.c | 72 +++++++++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 44 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Dump-histograms-only-if-present.patch --]
[-- Type: text/x-patch; name="0003-Dump-histograms-only-if-present.patch", Size: 3838 bytes --]

diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index e893ca084c9..25b957d0c0a 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -228,42 +228,38 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
   switch (hist->type)
     {
     case HIST_TYPE_INTERVAL:
-      fprintf (dump_file, "Interval counter range %d -- %d",
-	       hist->hdata.intvl.int_start,
-	       (hist->hdata.intvl.int_start
-	        + hist->hdata.intvl.steps - 1));
       if (hist->hvalue.counters)
 	{
-	   unsigned int i;
-	   fprintf (dump_file, " [");
-           for (i = 0; i < hist->hdata.intvl.steps; i++)
-	     fprintf (dump_file, " %d:%" PRId64,
-		      hist->hdata.intvl.int_start + i,
-		      (int64_t) hist->hvalue.counters[i]);
-	   fprintf (dump_file, " ] outside range:%" PRId64,
-		    (int64_t) hist->hvalue.counters[i]);
+	  fprintf (dump_file, "Interval counter range %d -- %d",
+		   hist->hdata.intvl.int_start,
+		   (hist->hdata.intvl.int_start
+		    + hist->hdata.intvl.steps - 1));
+
+	  unsigned int i;
+	  fprintf (dump_file, " [");
+	  for (i = 0; i < hist->hdata.intvl.steps; i++)
+	    fprintf (dump_file, " %d:%" PRId64,
+		     hist->hdata.intvl.int_start + i,
+		     (int64_t) hist->hvalue.counters[i]);
+	  fprintf (dump_file, " ] outside range:%" PRId64 ".\n",
+		   (int64_t) hist->hvalue.counters[i]);
 	}
-      fprintf (dump_file, ".\n");
       break;
 
     case HIST_TYPE_POW2:
-      fprintf (dump_file, "Pow2 counter ");
       if (hist->hvalue.counters)
-	{
-	   fprintf (dump_file, "pow2:%" PRId64
-		    " nonpow2:%" PRId64,
-		    (int64_t) hist->hvalue.counters[1],
-		    (int64_t) hist->hvalue.counters[0]);
-	}
-      fprintf (dump_file, ".\n");
+	fprintf (dump_file, "Pow2 counter pow2:%" PRId64
+		 " nonpow2:%" PRId64 ".\n",
+		 (int64_t) hist->hvalue.counters[1],
+		 (int64_t) hist->hvalue.counters[0]);
       break;
 
     case HIST_TYPE_SINGLE_VALUE:
     case HIST_TYPE_INDIR_CALL:
-      fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
-			   ? "Single values " : "Indirect call "));
       if (hist->hvalue.counters)
 	{
+	  fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
+			       ? "Single values " : "Indirect call "));
 	  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
 	    {
 	      fprintf (dump_file, "[%" PRId64 ":%" PRId64 "]",
@@ -272,40 +268,28 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
 	      if (i != GCOV_DISK_SINGLE_VALUES - 1)
 		fprintf (dump_file, ", ");
 	    }
+	  fprintf (dump_file, ".\n");
 	}
-      fprintf (dump_file, ".\n");
       break;
 
     case HIST_TYPE_AVERAGE:
-      fprintf (dump_file, "Average value ");
       if (hist->hvalue.counters)
-	{
-	   fprintf (dump_file, "sum:%" PRId64
-		    " times:%" PRId64,
-		    (int64_t) hist->hvalue.counters[0],
-		    (int64_t) hist->hvalue.counters[1]);
-	}
-      fprintf (dump_file, ".\n");
+	fprintf (dump_file, "Average value sum:%" PRId64
+		 " times:%" PRId64 ".\n",
+		 (int64_t) hist->hvalue.counters[0],
+		 (int64_t) hist->hvalue.counters[1]);
       break;
 
     case HIST_TYPE_IOR:
-      fprintf (dump_file, "IOR value ");
       if (hist->hvalue.counters)
-	{
-	   fprintf (dump_file, "ior:%" PRId64,
-		    (int64_t) hist->hvalue.counters[0]);
-	}
-      fprintf (dump_file, ".\n");
+	fprintf (dump_file, "IOR value ior:%" PRId64 ".\n",
+		 (int64_t) hist->hvalue.counters[0]);
       break;
 
     case HIST_TYPE_TIME_PROFILE:
-      fprintf (dump_file, "Time profile ");
       if (hist->hvalue.counters)
-      {
-        fprintf (dump_file, "time:%" PRId64,
-                 (int64_t) hist->hvalue.counters[0]);
-      }
-      fprintf (dump_file, ".\n");
+	fprintf (dump_file, "Time profile time:%" PRId64 ".\n",
+		 (int64_t) hist->hvalue.counters[0]);
       break;
     case HIST_TYPE_MAX:
       gcc_unreachable ();

[PATCH 4/4] Update a bit dump format.

[marxin]

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]


gcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* value-prof.c (dump_histogram_value): Change dump format.
	(gimple_mod_subtract_transform): Remove legacy comment.
---
 gcc/value-prof.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-Update-a-bit-dump-format.patch --]
[-- Type: text/x-patch; name="0004-Update-a-bit-dump-format.patch", Size: 1524 bytes --]

diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index 25b957d0c0a..5de91595811 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -230,18 +230,21 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
     case HIST_TYPE_INTERVAL:
       if (hist->hvalue.counters)
 	{
-	  fprintf (dump_file, "Interval counter range %d -- %d",
+	  fprintf (dump_file, "Interval counter range [%d,%d]: [",
 		   hist->hdata.intvl.int_start,
 		   (hist->hdata.intvl.int_start
 		    + hist->hdata.intvl.steps - 1));
 
 	  unsigned int i;
-	  fprintf (dump_file, " [");
 	  for (i = 0; i < hist->hdata.intvl.steps; i++)
-	    fprintf (dump_file, " %d:%" PRId64,
-		     hist->hdata.intvl.int_start + i,
-		     (int64_t) hist->hvalue.counters[i]);
-	  fprintf (dump_file, " ] outside range:%" PRId64 ".\n",
+	    {
+	      fprintf (dump_file, "%d:%" PRId64,
+		       hist->hdata.intvl.int_start + i,
+		       (int64_t) hist->hvalue.counters[i]);
+	      if (i != hist->hdata.intvl.steps - 1)
+		fprintf (dump_file, ", ");
+	    }
+	  fprintf (dump_file, "] outside range: %" PRId64 ".\n",
 		   (int64_t) hist->hvalue.counters[i]);
 	}
       break;
@@ -1094,7 +1097,6 @@ gimple_mod_subtract_transform (gimple_stmt_iterator *si)
   count1 = histogram->hvalue.counters[0];
   count2 = histogram->hvalue.counters[1];
 
-  /* Compute probability of taking the optimal path.  */
   if (check_counter (stmt, "interval", &count1, &all, gimple_bb (stmt)->count))
     {
       gimple_remove_histogram_value (cfun, stmt, histogram);

[PATCH 1/4] Remove indirect call top N counter type.

[marxin]

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]


gcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* doc/invoke.texi: Remove param.
	* gcov-counter.def (GCOV_COUNTER_ICALL_TOPNV):
	Remove.
	* gcov-io.h (GCOV_ICALL_TOPN_VAL): Likewise.
	(GCOV_ICALL_TOPN_NCOUNTS): Likewise.
	* params.def (PARAM_INDIR_CALL_TOPN_PROFILE): Likewise.
	* profile.c (instrument_values): Remove
	HIST_TYPE_INDIR_CALL_TOPN.
	* tree-profile.c (init_ic_make_global_vars):
	Always build __gcov_indirect_call only.
	(gimple_init_gcov_profiler): Remove usage
	of PARAM_INDIR_CALL_TOPN_PROFILE.
	(gimple_gen_ic_profiler): Likewise.
	* value-prof.c (dump_histogram_value): Likewise.
	(stream_in_histogram_value): Likewise.
	(gimple_indirect_call_to_profile): Likewise.
	(gimple_find_values_to_profile): Likewise.
	* value-prof.h (enum hist_type): Likewise.

libgcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* Makefile.in: Remove usage of
	_gcov_merge_icall_topn.
	* libgcov-driver.c (gcov_sort_n_vals): Remove.
	(gcov_sort_icall_topn_counter): Likewise.
	(gcov_sort_topn_counter_arrays): Likewise.
	(dump_one_gcov): Remove call to gcov_sort_topn_counter_arrays.
	* libgcov-merge.c (__gcov_merge_icall_topn): Remove.
	* libgcov-profiler.c (__gcov_topn_value_profiler_body):
	Likewise.
	(GCOV_ICALL_COUNTER_CLEAR_THRESHOLD): Remove.
	(struct indirect_call_tuple): Remove.
	(__gcov_indirect_call_topn_profiler): Remove.
	* libgcov-util.c (__gcov_icall_topn_counter_op): Remove.
	* libgcov.h (gcov_sort_n_vals): Remove.
	(L_gcov_merge_icall_topn): Likewise.
	(__gcov_merge_icall_topn): Likewise.
	(__gcov_indirect_call_topn_profiler): Likewise.
---
 gcc/doc/invoke.texi       |   3 -
 gcc/gcov-counter.def      |   3 -
 gcc/gcov-io.h             |   6 --
 gcc/params.def            |   8 ---
 gcc/profile.c             |   1 -
 gcc/tree-profile.c        |  14 +---
 gcc/value-prof.c          |  32 +--------
 gcc/value-prof.h          |   2 -
 libgcc/Makefile.in        |   5 +-
 libgcc/libgcov-driver.c   |  80 -----------------------
 libgcc/libgcov-merge.c    |  62 ------------------
 libgcc/libgcov-profiler.c | 133 --------------------------------------
 libgcc/libgcov-util.c     |  19 ------
 libgcc/libgcov.h          |   7 --
 14 files changed, 5 insertions(+), 370 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-indirect-call-top-N-counter-type.patch --]
[-- Type: text/x-patch; name="0001-Remove-indirect-call-top-N-counter-type.patch", Size: 20026 bytes --]

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 91c9bb89651..50e50e39413 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12140,9 +12140,6 @@ will not try to thread through its block.
 Maximum number of nested calls to search for control dependencies
 during uninitialized variable analysis.
 
-@item indir-call-topn-profile
-Track top N target addresses in indirect-call profile.
-
 @item max-once-peeled-insns
 The maximum number of insns of a peeled loop that rolls only once.
 
diff --git a/gcc/gcov-counter.def b/gcc/gcov-counter.def
index 3a0e620987a..b0596c8dc6b 100644
--- a/gcc/gcov-counter.def
+++ b/gcc/gcov-counter.def
@@ -49,6 +49,3 @@ DEF_GCOV_COUNTER(GCOV_COUNTER_IOR, "ior", _ior)
 
 /* Time profile collecting first run of a function */
 DEF_GCOV_COUNTER(GCOV_TIME_PROFILER, "time_profiler", _time_profile)
-
-/* Top N value tracking for indirect calls.  */
-DEF_GCOV_COUNTER(GCOV_COUNTER_ICALL_TOPNV, "indirect_call_topn", _icall_topn)
diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index 9edb2923982..69c9a73dba8 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -266,12 +266,6 @@ GCOV_COUNTERS
 #define GCOV_N_VALUE_COUNTERS \
   (GCOV_LAST_VALUE_COUNTER - GCOV_FIRST_VALUE_COUNTER + 1)
 
-/* The number of hottest callees to be tracked.  */
-#define GCOV_ICALL_TOPN_VAL  2
-
-/* The number of counter entries per icall callsite.  */
-#define GCOV_ICALL_TOPN_NCOUNTS (1 + GCOV_ICALL_TOPN_VAL * 4)
-
 /* 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/params.def b/gcc/params.def
index 6b7f7eb5bae..b4a4e4a4190 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -992,14 +992,6 @@ DEFPARAM (PARAM_PROFILE_FUNC_INTERNAL_ID,
 	  "Use internal function id in profile lookup.",
 	  0, 0, 1)
 
-/* When the parameter is 1, track the most frequent N target
-   addresses in indirect-call profile. This disables
-   indirect_call_profiler_v3 which tracks single target.  */
-DEFPARAM (PARAM_INDIR_CALL_TOPN_PROFILE,
-	  "indir-call-topn-profile",
-	  "Track top N target addresses in indirect-call profile.",
-	  0, 0, 1)
-
 /* Avoid SLP vectorization of large basic blocks.  */
 DEFPARAM (PARAM_SLP_MAX_INSNS_IN_BB,
 	  "slp-max-insns-in-bb",
diff --git a/gcc/profile.c b/gcc/profile.c
index a1dba1ac8fb..9aff9ef2b21 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -172,7 +172,6 @@ instrument_values (histogram_values values)
 	  break;
 
  	case HIST_TYPE_INDIR_CALL:
- 	case HIST_TYPE_INDIR_CALL_TOPN:
  	  gimple_gen_ic_profiler (hist, t, 0);
   	  break;
 
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index 9b6f351a3d6..f2cf4047579 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -101,11 +101,7 @@ init_ic_make_global_vars (void)
 
   ic_tuple_var
     = build_decl (UNKNOWN_LOCATION, VAR_DECL,
-		  get_identifier (
-			  (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
-			   "__gcov_indirect_call_topn" :
-			   "__gcov_indirect_call")),
-		  tuple_type);
+		  get_identifier ("__gcov_indirect_call"), tuple_type);
   TREE_PUBLIC (ic_tuple_var) = 1;
   DECL_ARTIFICIAL (ic_tuple_var) = 1;
   DECL_INITIAL (ic_tuple_var) = NULL;
@@ -187,8 +183,6 @@ gimple_init_gcov_profiler (void)
 					  ptr_type_node,
 					  NULL_TREE);
       profiler_fn_name = "__gcov_indirect_call_profiler_v3";
-      if (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE))
-	profiler_fn_name = "__gcov_indirect_call_topn_profiler";
 
       tree_indirect_call_profiler_fn
 	      = build_fn_decl (profiler_fn_name, ic_profiler_fn_type);
@@ -376,12 +370,6 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag, unsigned base)
   gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
   tree ref_ptr = tree_coverage_counter_addr (tag, base);
 
-  if ( (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) &&
-        tag == GCOV_COUNTER_V_INDIR) ||
-       (!PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) &&
-        tag == GCOV_COUNTER_ICALL_TOPNV))
-    return;
-
   ref_ptr = force_gimple_operand_gsi (&gsi, ref_ptr,
 				      true, NULL_TREE, true, GSI_SAME_STMT);
 
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index d34d2ab7685..1e14e532070 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -316,22 +316,6 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
       }
       fprintf (dump_file, ".\n");
       break;
-    case HIST_TYPE_INDIR_CALL_TOPN:
-      fprintf (dump_file, "Indirect call topn ");
-      if (hist->hvalue.counters)
-	{
-           int i;
-
-           fprintf (dump_file, "accu:%" PRId64, hist->hvalue.counters[0]);
-           for (i = 1; i < (GCOV_ICALL_TOPN_VAL << 2); i += 2)
-             {
-               fprintf (dump_file, " target:%" PRId64 " value:%" PRId64,
-                       (int64_t) hist->hvalue.counters[i],
-                       (int64_t) hist->hvalue.counters[i+1]);
-             }
-        }
-      fprintf (dump_file, ".\n");
-      break;
     case HIST_TYPE_MAX:
       gcc_unreachable ();
    }
@@ -416,10 +400,6 @@ stream_in_histogram_value (struct lto_input_block *ib, gimple *stmt)
 	  ncounters = 1;
 	  break;
 
-        case HIST_TYPE_INDIR_CALL_TOPN:
-          ncounters = (GCOV_ICALL_TOPN_VAL << 2) + 1;
-          break;
-
 	case HIST_TYPE_MAX:
 	  gcc_unreachable ();
 	}
@@ -1865,12 +1845,8 @@ gimple_indirect_call_to_profile (gimple *stmt, histogram_values *values)
 
   values->reserve (3);
 
-  values->quick_push (gimple_alloc_histogram_value (
-                        cfun,
-                        PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
-                          HIST_TYPE_INDIR_CALL_TOPN :
-                          HIST_TYPE_INDIR_CALL,
-			stmt, callee));
+  values->quick_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_INDIR_CALL,
+						    stmt, callee));
 
   return;
 }
@@ -1971,10 +1947,6 @@ gimple_find_values_to_profile (histogram_values *values)
 	  hist->n_counters = 1;
 	  break;
 
-        case HIST_TYPE_INDIR_CALL_TOPN:
-          hist->n_counters = GCOV_ICALL_TOPN_NCOUNTS;
-          break;
-
 	default:
 	  gcc_unreachable ();
 	}
diff --git a/gcc/value-prof.h b/gcc/value-prof.h
index 1251fa95e31..a54024b48de 100644
--- a/gcc/value-prof.h
+++ b/gcc/value-prof.h
@@ -33,8 +33,6 @@ enum hist_type
   HIST_TYPE_AVERAGE,	/* Compute average value (sum of all values).  */
   HIST_TYPE_IOR,	/* Used to compute expected alignment.  */
   HIST_TYPE_TIME_PROFILE, /* Used for time profile */
-  HIST_TYPE_INDIR_CALL_TOPN, /* Tries to identify the top N most frequently
-                                called functions in indirect call.  */
   HIST_TYPE_MAX
 };
 
diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index ea390a5bbea..fb77881145e 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -888,7 +888,7 @@ include $(iterator)
 # Build libgcov components.
 
 LIBGCOV_MERGE = _gcov_merge_add _gcov_merge_single			\
-	_gcov_merge_ior _gcov_merge_time_profile _gcov_merge_icall_topn
+	_gcov_merge_ior _gcov_merge_time_profile
 LIBGCOV_PROFILER = _gcov_interval_profiler				\
 	_gcov_interval_profiler_atomic					\
 	_gcov_pow2_profiler						\
@@ -900,8 +900,7 @@ LIBGCOV_PROFILER = _gcov_interval_profiler				\
 	_gcov_ior_profiler						\
 	_gcov_ior_profiler_atomic					\
 	_gcov_indirect_call_profiler_v3					\
-	_gcov_time_profiler						\
-	_gcov_indirect_call_topn_profiler
+	_gcov_time_profiler
 LIBGCOV_INTERFACE = _gcov_dump _gcov_flush _gcov_fork			\
 	_gcov_execl _gcov_execlp					\
 	_gcov_execle _gcov_execv _gcov_execvp _gcov_execve _gcov_reset
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index 5dc51df914f..f03868e34bb 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -415,84 +415,6 @@ merge_summary (int run_counted, struct gcov_summary *summary,
     }
 }
 
-/* Sort N entries in VALUE_ARRAY in descending order.
-   Each entry in VALUE_ARRAY has two values. The sorting
-   is based on the second value.  */
-
-GCOV_LINKAGE  void
-gcov_sort_n_vals (gcov_type *value_array, int n)
-{
-  int j, k;
-
-  for (j = 2; j < n; j += 2)
-    {
-      gcov_type cur_ent[2];
-
-      cur_ent[0] = value_array[j];
-      cur_ent[1] = value_array[j + 1];
-      k = j - 2;
-      while (k >= 0 && value_array[k + 1] < cur_ent[1])
-        {
-          value_array[k + 2] = value_array[k];
-          value_array[k + 3] = value_array[k+1];
-          k -= 2;
-        }
-      value_array[k + 2] = cur_ent[0];
-      value_array[k + 3] = cur_ent[1];
-    }
-}
-
-/* Sort the profile counters for all indirect call sites. Counters
-   for each call site are allocated in array COUNTERS.  */
-
-static void
-gcov_sort_icall_topn_counter (const struct gcov_ctr_info *counters)
-{
-  int i;
-  gcov_type *values;
-  int n = counters->num;
-
-  gcc_assert (!(n % GCOV_ICALL_TOPN_NCOUNTS));
-  values = counters->values;
-
-  for (i = 0; i < n; i += GCOV_ICALL_TOPN_NCOUNTS)
-    {
-      gcov_type *value_array = &values[i + 1];
-      gcov_sort_n_vals (value_array, GCOV_ICALL_TOPN_NCOUNTS - 1);
-    }
-}
-
-/* Sort topn indirect_call profile counters in GI_PTR.  */
-
-static void
-gcov_sort_topn_counter_arrays (const struct gcov_info *gi_ptr)
-{
-  unsigned int i;
-  int f_ix;
-  const struct gcov_fn_info *gfi_ptr;
-  const struct gcov_ctr_info *ci_ptr;
-
-  if (!gi_ptr->merge[GCOV_COUNTER_ICALL_TOPNV]) 
-    return;
-
-  for (f_ix = 0; (unsigned)f_ix != gi_ptr->n_functions; f_ix++)
-    {
-      gfi_ptr = gi_ptr->functions[f_ix];
-      ci_ptr = gfi_ptr->ctrs;
-      for (i = 0; i < GCOV_COUNTERS; i++)
-        {
-          if (!gi_ptr->merge[i])
-            continue;
-          if (i == GCOV_COUNTER_ICALL_TOPNV)
-            {
-              gcov_sort_icall_topn_counter (ci_ptr);
-              break;
-            }
-          ci_ptr++;
-        }
-    }
-}
-
 /* Dump the coverage counts for one gcov_info object. We merge with existing
    counts when possible, to avoid growing the .da files ad infinitum. We use
    this program's checksum to make sure we only accumulate whole program
@@ -510,8 +432,6 @@ dump_one_gcov (struct gcov_info *gi_ptr, struct gcov_filename *gf,
 
   fn_buffer = 0;
 
-  gcov_sort_topn_counter_arrays (gi_ptr);
-
   error = gcov_exit_open_gcda_file (gi_ptr, gf);
   if (error == -1)
     return;
diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
index 046ed5718bb..702a69f8349 100644
--- a/libgcc/libgcov-merge.c
+++ b/libgcc/libgcov-merge.c
@@ -122,66 +122,4 @@ __gcov_merge_single (gcov_type *counters, unsigned n_counters)
 }
 #endif /* L_gcov_merge_single */
 
-#ifdef L_gcov_merge_icall_topn
-/* The profile merging function used for merging indirect call counts
-   This function is given array COUNTERS of N_COUNTERS old counters and it
-   reads the same number of counters from the gcov file.  */
-
-void
-__gcov_merge_icall_topn (gcov_type *counters, unsigned n_counters)
-{
-  unsigned i, j, k, m;
-
-  gcc_assert (!(n_counters % GCOV_ICALL_TOPN_NCOUNTS));
-  for (i = 0; i < n_counters; i += GCOV_ICALL_TOPN_NCOUNTS)
-    {
-      gcov_type *value_array = &counters[i + 1];
-      unsigned tmp_size = 2 * (GCOV_ICALL_TOPN_NCOUNTS - 1);
-      gcov_type *tmp_array 
-          = (gcov_type *) alloca (tmp_size * sizeof (gcov_type));
-
-      for (j = 0; j < tmp_size; j++)
-        tmp_array[j] = 0;
-
-      for (j = 0; j < GCOV_ICALL_TOPN_NCOUNTS - 1; j += 2)
-        {
-          tmp_array[j] = value_array[j];
-          tmp_array[j + 1] = value_array [j + 1];
-        }
-
-      /* Skip the number_of_eviction entry.  */
-      gcov_get_counter ();
-      for (k = 0; k < GCOV_ICALL_TOPN_NCOUNTS - 1; k += 2)
-        {
-          int found = 0;
-          gcov_type global_id = gcov_get_counter_target ();
-          gcov_type call_count = gcov_get_counter ();
-          for (m = 0; m < j; m += 2)
-            {
-              if (tmp_array[m] == global_id)
-                {
-                  found = 1;
-                  tmp_array[m + 1] += call_count;
-                  break;
-                }
-            }
-          if (!found)
-            {
-              tmp_array[j] = global_id;
-              tmp_array[j + 1] = call_count;
-              j += 2;
-            }
-        }
-      /* Now sort the temp array */
-      gcov_sort_n_vals (tmp_array, j);
-
-      /* Now copy back the top half of the temp array */
-      for (k = 0; k < GCOV_ICALL_TOPN_NCOUNTS - 1; k += 2)
-        {
-          value_array[k] = tmp_array[k];
-          value_array[k + 1] = tmp_array[k + 1];
-        }
-    }
-}
-#endif /* L_gcov_merge_icall_topn */
 #endif /* inhibit_libc */
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 7116330252b..40f0858a174 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -163,139 +163,6 @@ __gcov_one_value_profiler_atomic (gcov_type *counters, gcov_type 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
-   entries are used.
-   counter[0] --- the accumative count of the number of times one entry in
-                  in the counters gets evicted/replaced due to limited capacity.
-                  When this value reaches a threshold, the bottom N values are
-                  cleared.
-   counter[1] through counter[2*N] records the top 2*N values collected so far.
-   Each value is represented by two entries: count[2*i+1] is the ith value, and
-   count[2*i+2] is the number of times the value is seen.  */
-
-static void
-__gcov_topn_value_profiler_body (gcov_type *counters, gcov_type value)
-{
-   unsigned i, found = 0, have_zero_count = 0;
-   gcov_type *entry;
-   gcov_type *lfu_entry = &counters[1];
-   gcov_type *value_array = &counters[1];
-   gcov_type *num_eviction = &counters[0];
-   gcov_unsigned_t topn_val = GCOV_ICALL_TOPN_VAL;
-
-   /* There are 2*topn_val values tracked, each value takes two slots in the
-      counter array.  */
-   for (i = 0; i < (topn_val << 2); i += 2)
-     {
-       entry = &value_array[i];
-       if (entry[0] == value)
-         {
-           entry[1]++ ;
-           found = 1;
-           break;
-         }
-       else if (entry[1] == 0)
-         {
-           lfu_entry = entry;
-           have_zero_count = 1;
-         }
-      else if (entry[1] < lfu_entry[1])
-        lfu_entry = entry;
-     }
-
-   if (found)
-     return;
-
-   /* lfu_entry is either an empty entry or an entry
-      with lowest count, which will be evicted.  */
-   lfu_entry[0] = value;
-   lfu_entry[1] = 1;
-
-#define GCOV_ICALL_COUNTER_CLEAR_THRESHOLD 3000
-
-   /* Too many evictions -- time to clear bottom entries to
-      avoid hot values bumping each other out.  */
-   if (!have_zero_count
-       && ++*num_eviction >= GCOV_ICALL_COUNTER_CLEAR_THRESHOLD)
-     {
-       unsigned i, j;
-       gcov_type *p, minv;
-       gcov_type* tmp_cnts
-           = (gcov_type *)alloca (topn_val * sizeof (gcov_type));
-
-       *num_eviction = 0;
-
-       for (i = 0; i < topn_val; i++)
-         tmp_cnts[i] = 0;
-
-       /* Find the largest topn_val values from the group of
-          2*topn_val values and put them into tmp_cnts.  */
-
-       for (i = 0; i < 2 * topn_val; i += 2)
-         {
-           p = 0;
-           for (j = 0; j < topn_val; j++)
-             {
-               if (!p || tmp_cnts[j] < *p)
-                  p = &tmp_cnts[j];
-             }
-            if (value_array[i + 1] > *p)
-              *p = value_array[i + 1];
-         }
-
-       minv = tmp_cnts[0];
-       for (j = 1; j < topn_val; j++)
-         {
-           if (tmp_cnts[j] < minv)
-             minv = tmp_cnts[j];
-         }
-       /* Zero out low value entries.  */
-       for (i = 0; i < 2 * topn_val; i += 2)
-         {
-           if (value_array[i + 1] < minv)
-             {
-               value_array[i] = 0;
-               value_array[i + 1] = 0;
-             }
-         }
-     }
-}
-
-/* These two variables are used to actually track caller and callee.  Keep
-   them in TLS memory so races are not common (they are written to often).
-   The variables are set directly by GCC instrumented code, so declaration
-   here must match one in tree-profile.c.  */
-
-#if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
-__thread
-#endif
-struct indirect_call_tuple __gcov_indirect_call_topn;
-
-#ifdef TARGET_VTABLE_USES_DESCRIPTORS
-#define VTABLE_USES_DESCRIPTORS 1
-#else
-#define VTABLE_USES_DESCRIPTORS 0
-#endif
-
-/* This fucntion is instrumented at function entry to track topn indirect
-   calls to CUR_FUNC.  */
- 
-void
-__gcov_indirect_call_topn_profiler (gcov_type value, void* cur_func)
-{
-  void *callee_func = __gcov_indirect_call_topn.callee;
-  /* 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 == callee_func
-      || (VTABLE_USES_DESCRIPTORS && callee_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_v3
 
 /* These two variables are used to actually track caller and callee.  Keep
diff --git a/libgcc/libgcov-util.c b/libgcc/libgcov-util.c
index 6be229b094a..c794132c172 100644
--- a/libgcc/libgcov-util.c
+++ b/libgcc/libgcov-util.c
@@ -740,25 +740,6 @@ __gcov_single_counter_op (gcov_type *counters, unsigned n_counters,
     }
 }
 
-/* Performing FN upon indirect-call profile counters.  */
-
-static void
-__gcov_icall_topn_counter_op (gcov_type *counters, unsigned n_counters,
-                              counter_op_fn fn, void *data1, void *data2)
-{
-  unsigned i;
-
-  gcc_assert (!(n_counters % GCOV_ICALL_TOPN_NCOUNTS));
-  for (i = 0; i < n_counters; i += GCOV_ICALL_TOPN_NCOUNTS)
-    {
-      unsigned j;
-      gcov_type *value_array = &counters[i + 1];
-
-      for (j = 0; j < GCOV_ICALL_TOPN_NCOUNTS - 1; j += 2)
-        value_array[j + 1] = fn (value_array[j + 1], data1, data2);
-    }
-}
-
 /* Scaling the counter value V by multiplying *(float*) DATA1.  */
 
 static gcov_type
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index 993db8fb057..b4f1ec576fc 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -102,7 +102,6 @@ typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI)));
 #define gcov_read_unsigned __gcov_read_unsigned
 #define gcov_read_counter __gcov_read_counter
 #define gcov_read_summary __gcov_read_summary
-#define gcov_sort_n_vals __gcov_sort_n_vals
 
 #else /* IN_GCOV_TOOL */
 /* About the host.  */
@@ -130,7 +129,6 @@ typedef unsigned gcov_position_t;
 #define L_gcov_merge_single 1
 #define L_gcov_merge_ior 1
 #define L_gcov_merge_time_profile 1
-#define L_gcov_merge_icall_topn 1
 
 extern gcov_type gcov_read_counter_mem ();
 extern unsigned gcov_get_merge_weight ();
@@ -267,9 +265,6 @@ extern void __gcov_merge_single (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
 /* The merge function that just ors the counters together.  */
 extern void __gcov_merge_ior (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
 
-/* The merge function is used for topn indirect call counters.  */
-extern void __gcov_merge_icall_topn (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
-
 /* The profiler functions.  */
 extern void __gcov_interval_profiler (gcov_type *, gcov_type, int, unsigned);
 extern void __gcov_interval_profiler_atomic (gcov_type *, gcov_type, int,
@@ -285,8 +280,6 @@ extern void __gcov_average_profiler (gcov_type *, gcov_type);
 extern void __gcov_average_profiler_atomic (gcov_type *, gcov_type);
 extern void __gcov_ior_profiler (gcov_type *, gcov_type);
 extern void __gcov_ior_profiler_atomic (gcov_type *, gcov_type);
-extern void __gcov_indirect_call_topn_profiler (gcov_type, void *);
-extern void gcov_sort_n_vals (gcov_type *, int);
 
 #ifndef inhibit_libc
 /* The wrappers around some library functions..  */

Re: [PATCH 0/4] Store multiple values for single value profilers

[Richard Biener]

On Tue, Jun 4, 2019 at 10:44 AM Martin Liska <mliska@suse.cz> wrote:
>
> Hi.
>
> It's becoming more common that a training run happens in parallel environment.
> That can lead to a not reproducible builds caused by different order of merging
> of .gcda files. So that I'm suggesting to store up to 4 values for HIST_TYPE_SINGLE_VALUE
> and HIST_TYPE_INDIR_CALL on disk. If the capacity is exceeded the whole counter is
> marked as unstable (not reproducible).
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

Thanks for working on this, I hope Honza can review and approve it.  Does this
solve the issue of profiledbootstrap results being not reproducible?  (if you
fix genchecksum to not generate different checksums)

I suppose this would also apply to a GCC 9 tree?

Thanks,
Richard.

> Thanks,
> Martin
>
> marxin (4):
>   Remove indirect call top N counter type.
>   Implement N disk counters for single value and indirect call counters.
>   Dump histograms only if present.
>   Update a bit dump format.
>
>  gcc/doc/invoke.texi       |   3 -
>  gcc/gcov-counter.def      |   3 -
>  gcc/gcov-io.h             |   9 +-
>  gcc/ipa-profile.c         |  13 ++-
>  gcc/params.def            |   8 --
>  gcc/profile.c             |   1 -
>  gcc/tree-profile.c        |  23 +---
>  gcc/value-prof.c          | 224 ++++++++++++++++----------------------
>  gcc/value-prof.h          |   4 +-
>  libgcc/Makefile.in        |  10 +-
>  libgcc/libgcov-driver.c   |  80 --------------
>  libgcc/libgcov-merge.c    | 139 +++++++++--------------
>  libgcc/libgcov-profiler.c | 176 ++----------------------------
>  libgcc/libgcov-util.c     |  19 ----
>  libgcc/libgcov.h          |  12 +-
>  15 files changed, 179 insertions(+), 545 deletions(-)
>
> --
> 2.21.0
>

Re: [PATCH 0/4] Store multiple values for single value profilers

[Martin Liška]

On 6/5/19 3:49 PM, Richard Biener wrote:
> On Tue, Jun 4, 2019 at 10:44 AM Martin Liska <mliska@suse.cz> wrote:
>>
>> Hi.
>>
>> It's becoming more common that a training run happens in parallel environment.
>> That can lead to a not reproducible builds caused by different order of merging
>> of .gcda files. So that I'm suggesting to store up to 4 values for HIST_TYPE_SINGLE_VALUE
>> and HIST_TYPE_INDIR_CALL on disk. If the capacity is exceeded the whole counter is
>> marked as unstable (not reproducible).
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> Thanks for working on this, I hope Honza can review and approve it. 

Yes, he'll do it soon.

> Does this
> solve the issue of profiledbootstrap results being not reproducible?  (if you
> fix genchecksum to not generate different checksums)

Hopefully, but it needs to be tested.

> 
> I suppose this would also apply to a GCC 9 tree?

Yes, it applies smoothly. Would you like to see it backported to 9.2?

Martin

> 
> Thanks,
> Richard.
> 
>> Thanks,
>> Martin
>>
>> marxin (4):
>>   Remove indirect call top N counter type.
>>   Implement N disk counters for single value and indirect call counters.
>>   Dump histograms only if present.
>>   Update a bit dump format.
>>
>>  gcc/doc/invoke.texi       |   3 -
>>  gcc/gcov-counter.def      |   3 -
>>  gcc/gcov-io.h             |   9 +-
>>  gcc/ipa-profile.c         |  13 ++-
>>  gcc/params.def            |   8 --
>>  gcc/profile.c             |   1 -
>>  gcc/tree-profile.c        |  23 +---
>>  gcc/value-prof.c          | 224 ++++++++++++++++----------------------
>>  gcc/value-prof.h          |   4 +-
>>  libgcc/Makefile.in        |  10 +-
>>  libgcc/libgcov-driver.c   |  80 --------------
>>  libgcc/libgcov-merge.c    | 139 +++++++++--------------
>>  libgcc/libgcov-profiler.c | 176 ++----------------------------
>>  libgcc/libgcov-util.c     |  19 ----
>>  libgcc/libgcov.h          |  12 +-
>>  15 files changed, 179 insertions(+), 545 deletions(-)
>>
>> --
>> 2.21.0
>>

Re: [PATCH 0/4] Store multiple values for single value profilers

[Richard Biener]

On Thu, Jun 6, 2019 at 10:23 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 6/5/19 3:49 PM, Richard Biener wrote:
> > On Tue, Jun 4, 2019 at 10:44 AM Martin Liska <mliska@suse.cz> wrote:
> >>
> >> Hi.
> >>
> >> It's becoming more common that a training run happens in parallel environment.
> >> That can lead to a not reproducible builds caused by different order of merging
> >> of .gcda files. So that I'm suggesting to store up to 4 values for HIST_TYPE_SINGLE_VALUE
> >> and HIST_TYPE_INDIR_CALL on disk. If the capacity is exceeded the whole counter is
> >> marked as unstable (not reproducible).
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >
> > Thanks for working on this, I hope Honza can review and approve it.
>
> Yes, he'll do it soon.
>
> > Does this
> > solve the issue of profiledbootstrap results being not reproducible?  (if you
> > fix genchecksum to not generate different checksums)
>
> Hopefully, but it needs to be tested.
>
> >
> > I suppose this would also apply to a GCC 9 tree?
>
> Yes, it applies smoothly. Would you like to see it backported to 9.2?

No, but eventually into our package.

Richard.

> Martin
>
> >
> > Thanks,
> > Richard.
> >
> >> Thanks,
> >> Martin
> >>
> >> marxin (4):
> >>   Remove indirect call top N counter type.
> >>   Implement N disk counters for single value and indirect call counters.
> >>   Dump histograms only if present.
> >>   Update a bit dump format.
> >>
> >>  gcc/doc/invoke.texi       |   3 -
> >>  gcc/gcov-counter.def      |   3 -
> >>  gcc/gcov-io.h             |   9 +-
> >>  gcc/ipa-profile.c         |  13 ++-
> >>  gcc/params.def            |   8 --
> >>  gcc/profile.c             |   1 -
> >>  gcc/tree-profile.c        |  23 +---
> >>  gcc/value-prof.c          | 224 ++++++++++++++++----------------------
> >>  gcc/value-prof.h          |   4 +-
> >>  libgcc/Makefile.in        |  10 +-
> >>  libgcc/libgcov-driver.c   |  80 --------------
> >>  libgcc/libgcov-merge.c    | 139 +++++++++--------------
> >>  libgcc/libgcov-profiler.c | 176 ++----------------------------
> >>  libgcc/libgcov-util.c     |  19 ----
> >>  libgcc/libgcov.h          |  12 +-
> >>  15 files changed, 179 insertions(+), 545 deletions(-)
> >>
> >> --
> >> 2.21.0
> >>
>

Re: [PATCH 4/4] Update a bit dump format.

[Jan Hubicka]

> 
> gcc/ChangeLog:
> 
> 2019-06-04  Martin Liska  <mliska@suse.cz>
> 
> 	* value-prof.c (dump_histogram_value): Change dump format.
> 	(gimple_mod_subtract_transform): Remove legacy comment.

OK,
Honza

Re: [PATCH 3/4] Dump histograms only if present.

[Jan Hubicka]

> 
> gcc/ChangeLog:
> 
> 2019-06-04  Martin Liska  <mliska@suse.cz>
> 
> 	* value-prof.c (dump_histogram_value): Print histogram values
> 	only if present.

What is the point of having histogram value when there are no counters?

Honza
> ---
>  gcc/value-prof.c | 72 +++++++++++++++++++-----------------------------
>  1 file changed, 28 insertions(+), 44 deletions(-)
> 

> diff --git a/gcc/value-prof.c b/gcc/value-prof.c
> index e893ca084c9..25b957d0c0a 100644
> --- a/gcc/value-prof.c
> +++ b/gcc/value-prof.c
> @@ -228,42 +228,38 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
>    switch (hist->type)
>      {
>      case HIST_TYPE_INTERVAL:
> -      fprintf (dump_file, "Interval counter range %d -- %d",
> -	       hist->hdata.intvl.int_start,
> -	       (hist->hdata.intvl.int_start
> -	        + hist->hdata.intvl.steps - 1));
>        if (hist->hvalue.counters)
>  	{
> -	   unsigned int i;
> -	   fprintf (dump_file, " [");
> -           for (i = 0; i < hist->hdata.intvl.steps; i++)
> -	     fprintf (dump_file, " %d:%" PRId64,
> -		      hist->hdata.intvl.int_start + i,
> -		      (int64_t) hist->hvalue.counters[i]);
> -	   fprintf (dump_file, " ] outside range:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[i]);
> +	  fprintf (dump_file, "Interval counter range %d -- %d",
> +		   hist->hdata.intvl.int_start,
> +		   (hist->hdata.intvl.int_start
> +		    + hist->hdata.intvl.steps - 1));
> +
> +	  unsigned int i;
> +	  fprintf (dump_file, " [");
> +	  for (i = 0; i < hist->hdata.intvl.steps; i++)
> +	    fprintf (dump_file, " %d:%" PRId64,
> +		     hist->hdata.intvl.int_start + i,
> +		     (int64_t) hist->hvalue.counters[i]);
> +	  fprintf (dump_file, " ] outside range:%" PRId64 ".\n",
> +		   (int64_t) hist->hvalue.counters[i]);
>  	}
> -      fprintf (dump_file, ".\n");
>        break;
>  
>      case HIST_TYPE_POW2:
> -      fprintf (dump_file, "Pow2 counter ");
>        if (hist->hvalue.counters)
> -	{
> -	   fprintf (dump_file, "pow2:%" PRId64
> -		    " nonpow2:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[1],
> -		    (int64_t) hist->hvalue.counters[0]);
> -	}
> -      fprintf (dump_file, ".\n");
> +	fprintf (dump_file, "Pow2 counter pow2:%" PRId64
> +		 " nonpow2:%" PRId64 ".\n",
> +		 (int64_t) hist->hvalue.counters[1],
> +		 (int64_t) hist->hvalue.counters[0]);
>        break;
>  
>      case HIST_TYPE_SINGLE_VALUE:
>      case HIST_TYPE_INDIR_CALL:
> -      fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
> -			   ? "Single values " : "Indirect call "));
>        if (hist->hvalue.counters)
>  	{
> +	  fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
> +			       ? "Single values " : "Indirect call "));
>  	  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
>  	    {
>  	      fprintf (dump_file, "[%" PRId64 ":%" PRId64 "]",
> @@ -272,40 +268,28 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
>  	      if (i != GCOV_DISK_SINGLE_VALUES - 1)
>  		fprintf (dump_file, ", ");
>  	    }
> +	  fprintf (dump_file, ".\n");
>  	}
> -      fprintf (dump_file, ".\n");
>        break;
>  
>      case HIST_TYPE_AVERAGE:
> -      fprintf (dump_file, "Average value ");
>        if (hist->hvalue.counters)
> -	{
> -	   fprintf (dump_file, "sum:%" PRId64
> -		    " times:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[0],
> -		    (int64_t) hist->hvalue.counters[1]);
> -	}
> -      fprintf (dump_file, ".\n");
> +	fprintf (dump_file, "Average value sum:%" PRId64
> +		 " times:%" PRId64 ".\n",
> +		 (int64_t) hist->hvalue.counters[0],
> +		 (int64_t) hist->hvalue.counters[1]);
>        break;
>  
>      case HIST_TYPE_IOR:
> -      fprintf (dump_file, "IOR value ");
>        if (hist->hvalue.counters)
> -	{
> -	   fprintf (dump_file, "ior:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[0]);
> -	}
> -      fprintf (dump_file, ".\n");
> +	fprintf (dump_file, "IOR value ior:%" PRId64 ".\n",
> +		 (int64_t) hist->hvalue.counters[0]);
>        break;
>  
>      case HIST_TYPE_TIME_PROFILE:
> -      fprintf (dump_file, "Time profile ");
>        if (hist->hvalue.counters)
> -      {
> -        fprintf (dump_file, "time:%" PRId64,
> -                 (int64_t) hist->hvalue.counters[0]);
> -      }
> -      fprintf (dump_file, ".\n");
> +	fprintf (dump_file, "Time profile time:%" PRId64 ".\n",
> +		 (int64_t) hist->hvalue.counters[0]);
>        break;
>      case HIST_TYPE_MAX:
>        gcc_unreachable ();

Re: [PATCH 3/4] Dump histograms only if present.

[Martin Liška]

On 6/6/19 2:16 PM, Jan Hubicka wrote:
> What is the point of having histogram value when there are no counters?

Because we first create histograms in branch_prob -> gimple_find_values_to_profile and
later we read profile from file. At that point we know which are empty, but we don't
remove them.

Martin

Re: [PATCH 2/4] Implement N disk counters for single value and indirect call counters.

[Jan Hubicka]

> 
> gcc/ChangeLog:
> 
> 2019-06-04  Martin Liska  <mliska@suse.cz>
> 
> 	* gcov-io.h (GCOV_DISK_SINGLE_VALUES): New.
> 	(GCOV_SINGLE_VALUE_COUNTERS): Likewise.
> 	* ipa-profile.c (ipa_profile_generate_summary):
> 	Use get_most_common_single_value.
> 	* tree-profile.c (gimple_init_gcov_profiler):
> 	Instrument with __gcov_one_value_profiler_v2
> 	and __gcov_indirect_call_profiler_v4.
> 	* value-prof.c (dump_histogram_value):
> 	Print all values for HIST_TYPE_SINGLE_VALUE.
> 	(stream_in_histogram_value): Set number of
> 	counters for HIST_TYPE_SINGLE_VALUE.
> 	(get_most_common_single_value): New.
> 	(gimple_divmod_fixed_value_transform):
> 	Use get_most_common_single_value.
> 	(gimple_ic_transform): Likewise.
> 	(gimple_stringops_transform): Likewise.
> 	(gimple_find_values_to_profile): Set number
> 	of counters for HIST_TYPE_SINGLE_VALUE.
> 	* value-prof.h (get_most_common_single_value):
> 	New.
> 
> libgcc/ChangeLog:
> 
> 2019-06-04  Martin Liska  <mliska@suse.cz>
> 
> 	* Makefile.in: Add __gcov_one_value_profiler_v2 and
> 	__gcov_indirect_call_profiler_v4.
> 	* libgcov-merge.c (__gcov_merge_single): Change
> 	function signature.
> 	(merge_single_value_set): New.
> 	* libgcov-profiler.c (__gcov_one_value_profiler_body):
> 	Do not update counters[2].
> 	(__gcov_one_value_profiler): Remove.
> 	(__gcov_one_value_profiler_atomic): Rename to ...
> 	(__gcov_one_value_profiler_v2): ... this.
> 	(__gcov_indirect_call_profiler_v3): Rename to ...
> 	(__gcov_indirect_call_profiler_v4): ... this.
> 	* libgcov.h (__gcov_one_value_profiler): Remove.
> 	(__gcov_one_value_profiler_atomic): Remove.
> 	(__gcov_indirect_call_profiler_v3): Remove.
> 	(__gcov_one_value_profiler_v2): New.
> 	(__gcov_indirect_call_profiler_v4): New.
> ---
>  gcc/gcov-io.h             |   7 +++
>  gcc/ipa-profile.c         |  13 +++--
>  gcc/tree-profile.c        |   9 ++-
>  gcc/value-prof.c          | 120 ++++++++++++++++++++------------------
>  gcc/value-prof.h          |   2 +
>  libgcc/Makefile.in        |   5 +-
>  libgcc/libgcov-merge.c    |  77 ++++++++++++++++--------
>  libgcc/libgcov-profiler.c |  43 +++-----------
>  libgcc/libgcov.h          |   5 +-
>  9 files changed, 147 insertions(+), 134 deletions(-)
> 

> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
> index f2cf4047579..008a1271979 100644
> --- a/gcc/tree-profile.c
> +++ b/gcc/tree-profile.c
> @@ -165,10 +165,9 @@ gimple_init_gcov_profiler (void)
>  	      = build_function_type_list (void_type_node,
>  					  gcov_type_ptr, gcov_type_node,
>  					  NULL_TREE);
> -      fn_name = concat ("__gcov_one_value_profiler", fn_suffix, NULL);
> -      tree_one_value_profiler_fn = build_fn_decl (fn_name,
> -						  one_value_profiler_fn_type);
> -      free (CONST_CAST (char *, fn_name));
> +      tree_one_value_profiler_fn
> +	= build_fn_decl ("__gcov_one_value_profiler_v2",
> +			 one_value_profiler_fn_type);

Why you no longer need the optional atomic suffix here?
> diff --git a/gcc/value-prof.c b/gcc/value-prof.c
> index 1e14e532070..e893ca084c9 100644
> --- a/gcc/value-prof.c
> +++ b/gcc/value-prof.c
> @@ -259,15 +259,19 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
>        break;
>  
>      case HIST_TYPE_SINGLE_VALUE:
> -      fprintf (dump_file, "Single value ");
> +    case HIST_TYPE_INDIR_CALL:
> +      fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
> +			   ? "Single values " : "Indirect call "));
>        if (hist->hvalue.counters)
>  	{
> -	   fprintf (dump_file, "value:%" PRId64
> -		    " match:%" PRId64
> -		    " wrong:%" PRId64,
> -		    (int64_t) hist->hvalue.counters[0],
> -		    (int64_t) hist->hvalue.counters[1],
> -		    (int64_t) hist->hvalue.counters[2]);
> +	  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
> +	    {
> +	      fprintf (dump_file, "[%" PRId64 ":%" PRId64 "]",
> +		       (int64_t) hist->hvalue.counters[2 * i],
> +		       (int64_t) hist->hvalue.counters[2 * i + 1]);
> +	      if (i != GCOV_DISK_SINGLE_VALUES - 1)
> +		fprintf (dump_file, ", ");
> +	    }
Unless there are some compelling reasons, I would still include in dump what is
meaning of the values - not everyone is fluent in that ;)
> @@ -758,23 +779,19 @@ gimple_divmod_fixed_value_transform (gimple_stmt_iterator *si)
>    if (!histogram)
>      return false;
>  
> +  if (!get_most_common_single_value (histogram, &val, &count))
> +    return false;
> +
>    value = histogram->hvalue.value;
> -  val = histogram->hvalue.counters[0];
> -  count = histogram->hvalue.counters[1];
> -  all = histogram->hvalue.counters[2];
> +  all = gimple_bb (stmt)->count.ipa ().to_gcov_type ();

Here it is safe to use gimple_bb (stmt)->count.ipa ().to_gcov_type () under an
assumptions that profile is consistent - I am not sure how often one gets
mismatches with -fprofile-correction at multithreaded programs, maybe it would
be worth to check before we drop the code logging it to dumps.

In ipa-profile it is not quite the case since we do CFG optimization and other
transforms between profile instrumentaiton and IPA passes.  I would remember
probability within stmt histogram (like we do for speculative edges) that is
safe WRT BB profile updates.

So in your implementation we store 4 times as many vlaue counters in memory
during runtime?  Originally I had in mind a variant where in memory everything
works as before, but in on-disk format one save the section for indirect
call counters multiple times implementing the update logic.
This should not be that hard do something:

 for secno = 1...4
   for every counter
     read counter2 from disk
     if (counter has non-zero exec count)
       if counter2 is invalid
	 clear counter
       else if counter and counter2 values match
         increase counter2 hitrate
	 clear counter
       else if counter2 has zero count
	 rewrite counter2 by counter
	 clear counter
       else if secno==5
	 mark counter2 as invalid
	 clear ocunter
     write counter2 to disk

So you do not really need to keep duplicates in memory at runtime.
You will still be able to tell if there was too many counters by checking
the last copy (instead of first as you do in your implementation)
> +      value = gcov_get_counter_target ();
> +      counter = gcov_get_counter ();
> +
> +      if (counter == -1)
> +	{
> +	  counters[1] = -1;
> +	  /* We can't return as we need to read all counters.  */
> +	  continue;
> +	}
> +      else if (counter == 0 || counters[1] == -1)
> +	{
> +	  /* We can't return as we need to read all counters.  */
> +	  continue;
> +	}
gcov_get_counter scales by gcov_get_merge_weight and thus it will
not reliably read -1 when you store -1

Honza

Re: [PATCH 1/4] Remove indirect call top N counter type.

[Jan Hubicka]

Hi,
so the only point of removing this is the fact that builds would be
not reproducible with indir-call-topn-profile?
I still kind of thing it may be useful to track multiple most common
values, so I would be in favour of keeping it just updating the documentation
of indir-call-topn-profile that it is currently incomplete and does not
lead to reproducible builds and does not handle speculation...

Honza
> 
> gcc/ChangeLog:
> 
> 2019-06-04  Martin Liska  <mliska@suse.cz>
> 
> 	* doc/invoke.texi: Remove param.
> 	* gcov-counter.def (GCOV_COUNTER_ICALL_TOPNV):
> 	Remove.
> 	* gcov-io.h (GCOV_ICALL_TOPN_VAL): Likewise.
> 	(GCOV_ICALL_TOPN_NCOUNTS): Likewise.
> 	* params.def (PARAM_INDIR_CALL_TOPN_PROFILE): Likewise.
> 	* profile.c (instrument_values): Remove
> 	HIST_TYPE_INDIR_CALL_TOPN.
> 	* tree-profile.c (init_ic_make_global_vars):
> 	Always build __gcov_indirect_call only.
> 	(gimple_init_gcov_profiler): Remove usage
> 	of PARAM_INDIR_CALL_TOPN_PROFILE.
> 	(gimple_gen_ic_profiler): Likewise.
> 	* value-prof.c (dump_histogram_value): Likewise.
> 	(stream_in_histogram_value): Likewise.
> 	(gimple_indirect_call_to_profile): Likewise.
> 	(gimple_find_values_to_profile): Likewise.
> 	* value-prof.h (enum hist_type): Likewise.
> 
> libgcc/ChangeLog:
> 
> 2019-06-04  Martin Liska  <mliska@suse.cz>
> 
> 	* Makefile.in: Remove usage of
> 	_gcov_merge_icall_topn.
> 	* libgcov-driver.c (gcov_sort_n_vals): Remove.
> 	(gcov_sort_icall_topn_counter): Likewise.
> 	(gcov_sort_topn_counter_arrays): Likewise.
> 	(dump_one_gcov): Remove call to gcov_sort_topn_counter_arrays.
> 	* libgcov-merge.c (__gcov_merge_icall_topn): Remove.
> 	* libgcov-profiler.c (__gcov_topn_value_profiler_body):
> 	Likewise.
> 	(GCOV_ICALL_COUNTER_CLEAR_THRESHOLD): Remove.
> 	(struct indirect_call_tuple): Remove.
> 	(__gcov_indirect_call_topn_profiler): Remove.
> 	* libgcov-util.c (__gcov_icall_topn_counter_op): Remove.
> 	* libgcov.h (gcov_sort_n_vals): Remove.
> 	(L_gcov_merge_icall_topn): Likewise.
> 	(__gcov_merge_icall_topn): Likewise.
> 	(__gcov_indirect_call_topn_profiler): Likewise.
> ---
>  gcc/doc/invoke.texi       |   3 -
>  gcc/gcov-counter.def      |   3 -
>  gcc/gcov-io.h             |   6 --
>  gcc/params.def            |   8 ---
>  gcc/profile.c             |   1 -
>  gcc/tree-profile.c        |  14 +---
>  gcc/value-prof.c          |  32 +--------
>  gcc/value-prof.h          |   2 -
>  libgcc/Makefile.in        |   5 +-
>  libgcc/libgcov-driver.c   |  80 -----------------------
>  libgcc/libgcov-merge.c    |  62 ------------------
>  libgcc/libgcov-profiler.c | 133 --------------------------------------
>  libgcc/libgcov-util.c     |  19 ------
>  libgcc/libgcov.h          |   7 --
>  14 files changed, 5 insertions(+), 370 deletions(-)
> 

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 91c9bb89651..50e50e39413 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -12140,9 +12140,6 @@ will not try to thread through its block.
>  Maximum number of nested calls to search for control dependencies
>  during uninitialized variable analysis.
>  
> -@item indir-call-topn-profile
> -Track top N target addresses in indirect-call profile.
> -
>  @item max-once-peeled-insns
>  The maximum number of insns of a peeled loop that rolls only once.
>  
> diff --git a/gcc/gcov-counter.def b/gcc/gcov-counter.def
> index 3a0e620987a..b0596c8dc6b 100644
> --- a/gcc/gcov-counter.def
> +++ b/gcc/gcov-counter.def
> @@ -49,6 +49,3 @@ DEF_GCOV_COUNTER(GCOV_COUNTER_IOR, "ior", _ior)
>  
>  /* Time profile collecting first run of a function */
>  DEF_GCOV_COUNTER(GCOV_TIME_PROFILER, "time_profiler", _time_profile)
> -
> -/* Top N value tracking for indirect calls.  */
> -DEF_GCOV_COUNTER(GCOV_COUNTER_ICALL_TOPNV, "indirect_call_topn", _icall_topn)
> diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
> index 9edb2923982..69c9a73dba8 100644
> --- a/gcc/gcov-io.h
> +++ b/gcc/gcov-io.h
> @@ -266,12 +266,6 @@ GCOV_COUNTERS
>  #define GCOV_N_VALUE_COUNTERS \
>    (GCOV_LAST_VALUE_COUNTER - GCOV_FIRST_VALUE_COUNTER + 1)
>  
> -/* The number of hottest callees to be tracked.  */
> -#define GCOV_ICALL_TOPN_VAL  2
> -
> -/* The number of counter entries per icall callsite.  */
> -#define GCOV_ICALL_TOPN_NCOUNTS (1 + GCOV_ICALL_TOPN_VAL * 4)
> -
>  /* 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/params.def b/gcc/params.def
> index 6b7f7eb5bae..b4a4e4a4190 100644
> --- a/gcc/params.def
> +++ b/gcc/params.def
> @@ -992,14 +992,6 @@ DEFPARAM (PARAM_PROFILE_FUNC_INTERNAL_ID,
>  	  "Use internal function id in profile lookup.",
>  	  0, 0, 1)
>  
> -/* When the parameter is 1, track the most frequent N target
> -   addresses in indirect-call profile. This disables
> -   indirect_call_profiler_v3 which tracks single target.  */
> -DEFPARAM (PARAM_INDIR_CALL_TOPN_PROFILE,
> -	  "indir-call-topn-profile",
> -	  "Track top N target addresses in indirect-call profile.",
> -	  0, 0, 1)
> -
>  /* Avoid SLP vectorization of large basic blocks.  */
>  DEFPARAM (PARAM_SLP_MAX_INSNS_IN_BB,
>  	  "slp-max-insns-in-bb",
> diff --git a/gcc/profile.c b/gcc/profile.c
> index a1dba1ac8fb..9aff9ef2b21 100644
> --- a/gcc/profile.c
> +++ b/gcc/profile.c
> @@ -172,7 +172,6 @@ instrument_values (histogram_values values)
>  	  break;
>  
>   	case HIST_TYPE_INDIR_CALL:
> - 	case HIST_TYPE_INDIR_CALL_TOPN:
>   	  gimple_gen_ic_profiler (hist, t, 0);
>    	  break;
>  
> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
> index 9b6f351a3d6..f2cf4047579 100644
> --- a/gcc/tree-profile.c
> +++ b/gcc/tree-profile.c
> @@ -101,11 +101,7 @@ init_ic_make_global_vars (void)
>  
>    ic_tuple_var
>      = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> -		  get_identifier (
> -			  (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
> -			   "__gcov_indirect_call_topn" :
> -			   "__gcov_indirect_call")),
> -		  tuple_type);
> +		  get_identifier ("__gcov_indirect_call"), tuple_type);
>    TREE_PUBLIC (ic_tuple_var) = 1;
>    DECL_ARTIFICIAL (ic_tuple_var) = 1;
>    DECL_INITIAL (ic_tuple_var) = NULL;
> @@ -187,8 +183,6 @@ gimple_init_gcov_profiler (void)
>  					  ptr_type_node,
>  					  NULL_TREE);
>        profiler_fn_name = "__gcov_indirect_call_profiler_v3";
> -      if (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE))
> -	profiler_fn_name = "__gcov_indirect_call_topn_profiler";
>  
>        tree_indirect_call_profiler_fn
>  	      = build_fn_decl (profiler_fn_name, ic_profiler_fn_type);
> @@ -376,12 +370,6 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag, unsigned base)
>    gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>    tree ref_ptr = tree_coverage_counter_addr (tag, base);
>  
> -  if ( (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) &&
> -        tag == GCOV_COUNTER_V_INDIR) ||
> -       (!PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) &&
> -        tag == GCOV_COUNTER_ICALL_TOPNV))
> -    return;
> -
>    ref_ptr = force_gimple_operand_gsi (&gsi, ref_ptr,
>  				      true, NULL_TREE, true, GSI_SAME_STMT);
>  
> diff --git a/gcc/value-prof.c b/gcc/value-prof.c
> index d34d2ab7685..1e14e532070 100644
> --- a/gcc/value-prof.c
> +++ b/gcc/value-prof.c
> @@ -316,22 +316,6 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
>        }
>        fprintf (dump_file, ".\n");
>        break;
> -    case HIST_TYPE_INDIR_CALL_TOPN:
> -      fprintf (dump_file, "Indirect call topn ");
> -      if (hist->hvalue.counters)
> -	{
> -           int i;
> -
> -           fprintf (dump_file, "accu:%" PRId64, hist->hvalue.counters[0]);
> -           for (i = 1; i < (GCOV_ICALL_TOPN_VAL << 2); i += 2)
> -             {
> -               fprintf (dump_file, " target:%" PRId64 " value:%" PRId64,
> -                       (int64_t) hist->hvalue.counters[i],
> -                       (int64_t) hist->hvalue.counters[i+1]);
> -             }
> -        }
> -      fprintf (dump_file, ".\n");
> -      break;
>      case HIST_TYPE_MAX:
>        gcc_unreachable ();
>     }
> @@ -416,10 +400,6 @@ stream_in_histogram_value (struct lto_input_block *ib, gimple *stmt)
>  	  ncounters = 1;
>  	  break;
>  
> -        case HIST_TYPE_INDIR_CALL_TOPN:
> -          ncounters = (GCOV_ICALL_TOPN_VAL << 2) + 1;
> -          break;
> -
>  	case HIST_TYPE_MAX:
>  	  gcc_unreachable ();
>  	}
> @@ -1865,12 +1845,8 @@ gimple_indirect_call_to_profile (gimple *stmt, histogram_values *values)
>  
>    values->reserve (3);
>  
> -  values->quick_push (gimple_alloc_histogram_value (
> -                        cfun,
> -                        PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
> -                          HIST_TYPE_INDIR_CALL_TOPN :
> -                          HIST_TYPE_INDIR_CALL,
> -			stmt, callee));
> +  values->quick_push (gimple_alloc_histogram_value (cfun, HIST_TYPE_INDIR_CALL,
> +						    stmt, callee));
>  
>    return;
>  }
> @@ -1971,10 +1947,6 @@ gimple_find_values_to_profile (histogram_values *values)
>  	  hist->n_counters = 1;
>  	  break;
>  
> -        case HIST_TYPE_INDIR_CALL_TOPN:
> -          hist->n_counters = GCOV_ICALL_TOPN_NCOUNTS;
> -          break;
> -
>  	default:
>  	  gcc_unreachable ();
>  	}
> diff --git a/gcc/value-prof.h b/gcc/value-prof.h
> index 1251fa95e31..a54024b48de 100644
> --- a/gcc/value-prof.h
> +++ b/gcc/value-prof.h
> @@ -33,8 +33,6 @@ enum hist_type
>    HIST_TYPE_AVERAGE,	/* Compute average value (sum of all values).  */
>    HIST_TYPE_IOR,	/* Used to compute expected alignment.  */
>    HIST_TYPE_TIME_PROFILE, /* Used for time profile */
> -  HIST_TYPE_INDIR_CALL_TOPN, /* Tries to identify the top N most frequently
> -                                called functions in indirect call.  */
>    HIST_TYPE_MAX
>  };
>  
> diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
> index ea390a5bbea..fb77881145e 100644
> --- a/libgcc/Makefile.in
> +++ b/libgcc/Makefile.in
> @@ -888,7 +888,7 @@ include $(iterator)
>  # Build libgcov components.
>  
>  LIBGCOV_MERGE = _gcov_merge_add _gcov_merge_single			\
> -	_gcov_merge_ior _gcov_merge_time_profile _gcov_merge_icall_topn
> +	_gcov_merge_ior _gcov_merge_time_profile
>  LIBGCOV_PROFILER = _gcov_interval_profiler				\
>  	_gcov_interval_profiler_atomic					\
>  	_gcov_pow2_profiler						\
> @@ -900,8 +900,7 @@ LIBGCOV_PROFILER = _gcov_interval_profiler				\
>  	_gcov_ior_profiler						\
>  	_gcov_ior_profiler_atomic					\
>  	_gcov_indirect_call_profiler_v3					\
> -	_gcov_time_profiler						\
> -	_gcov_indirect_call_topn_profiler
> +	_gcov_time_profiler
>  LIBGCOV_INTERFACE = _gcov_dump _gcov_flush _gcov_fork			\
>  	_gcov_execl _gcov_execlp					\
>  	_gcov_execle _gcov_execv _gcov_execvp _gcov_execve _gcov_reset
> diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
> index 5dc51df914f..f03868e34bb 100644
> --- a/libgcc/libgcov-driver.c
> +++ b/libgcc/libgcov-driver.c
> @@ -415,84 +415,6 @@ merge_summary (int run_counted, struct gcov_summary *summary,
>      }
>  }
>  
> -/* Sort N entries in VALUE_ARRAY in descending order.
> -   Each entry in VALUE_ARRAY has two values. The sorting
> -   is based on the second value.  */
> -
> -GCOV_LINKAGE  void
> -gcov_sort_n_vals (gcov_type *value_array, int n)
> -{
> -  int j, k;
> -
> -  for (j = 2; j < n; j += 2)
> -    {
> -      gcov_type cur_ent[2];
> -
> -      cur_ent[0] = value_array[j];
> -      cur_ent[1] = value_array[j + 1];
> -      k = j - 2;
> -      while (k >= 0 && value_array[k + 1] < cur_ent[1])
> -        {
> -          value_array[k + 2] = value_array[k];
> -          value_array[k + 3] = value_array[k+1];
> -          k -= 2;
> -        }
> -      value_array[k + 2] = cur_ent[0];
> -      value_array[k + 3] = cur_ent[1];
> -    }
> -}
> -
> -/* Sort the profile counters for all indirect call sites. Counters
> -   for each call site are allocated in array COUNTERS.  */
> -
> -static void
> -gcov_sort_icall_topn_counter (const struct gcov_ctr_info *counters)
> -{
> -  int i;
> -  gcov_type *values;
> -  int n = counters->num;
> -
> -  gcc_assert (!(n % GCOV_ICALL_TOPN_NCOUNTS));
> -  values = counters->values;
> -
> -  for (i = 0; i < n; i += GCOV_ICALL_TOPN_NCOUNTS)
> -    {
> -      gcov_type *value_array = &values[i + 1];
> -      gcov_sort_n_vals (value_array, GCOV_ICALL_TOPN_NCOUNTS - 1);
> -    }
> -}
> -
> -/* Sort topn indirect_call profile counters in GI_PTR.  */
> -
> -static void
> -gcov_sort_topn_counter_arrays (const struct gcov_info *gi_ptr)
> -{
> -  unsigned int i;
> -  int f_ix;
> -  const struct gcov_fn_info *gfi_ptr;
> -  const struct gcov_ctr_info *ci_ptr;
> -
> -  if (!gi_ptr->merge[GCOV_COUNTER_ICALL_TOPNV]) 
> -    return;
> -
> -  for (f_ix = 0; (unsigned)f_ix != gi_ptr->n_functions; f_ix++)
> -    {
> -      gfi_ptr = gi_ptr->functions[f_ix];
> -      ci_ptr = gfi_ptr->ctrs;
> -      for (i = 0; i < GCOV_COUNTERS; i++)
> -        {
> -          if (!gi_ptr->merge[i])
> -            continue;
> -          if (i == GCOV_COUNTER_ICALL_TOPNV)
> -            {
> -              gcov_sort_icall_topn_counter (ci_ptr);
> -              break;
> -            }
> -          ci_ptr++;
> -        }
> -    }
> -}
> -
>  /* Dump the coverage counts for one gcov_info object. We merge with existing
>     counts when possible, to avoid growing the .da files ad infinitum. We use
>     this program's checksum to make sure we only accumulate whole program
> @@ -510,8 +432,6 @@ dump_one_gcov (struct gcov_info *gi_ptr, struct gcov_filename *gf,
>  
>    fn_buffer = 0;
>  
> -  gcov_sort_topn_counter_arrays (gi_ptr);
> -
>    error = gcov_exit_open_gcda_file (gi_ptr, gf);
>    if (error == -1)
>      return;
> diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
> index 046ed5718bb..702a69f8349 100644
> --- a/libgcc/libgcov-merge.c
> +++ b/libgcc/libgcov-merge.c
> @@ -122,66 +122,4 @@ __gcov_merge_single (gcov_type *counters, unsigned n_counters)
>  }
>  #endif /* L_gcov_merge_single */
>  
> -#ifdef L_gcov_merge_icall_topn
> -/* The profile merging function used for merging indirect call counts
> -   This function is given array COUNTERS of N_COUNTERS old counters and it
> -   reads the same number of counters from the gcov file.  */
> -
> -void
> -__gcov_merge_icall_topn (gcov_type *counters, unsigned n_counters)
> -{
> -  unsigned i, j, k, m;
> -
> -  gcc_assert (!(n_counters % GCOV_ICALL_TOPN_NCOUNTS));
> -  for (i = 0; i < n_counters; i += GCOV_ICALL_TOPN_NCOUNTS)
> -    {
> -      gcov_type *value_array = &counters[i + 1];
> -      unsigned tmp_size = 2 * (GCOV_ICALL_TOPN_NCOUNTS - 1);
> -      gcov_type *tmp_array 
> -          = (gcov_type *) alloca (tmp_size * sizeof (gcov_type));
> -
> -      for (j = 0; j < tmp_size; j++)
> -        tmp_array[j] = 0;
> -
> -      for (j = 0; j < GCOV_ICALL_TOPN_NCOUNTS - 1; j += 2)
> -        {
> -          tmp_array[j] = value_array[j];
> -          tmp_array[j + 1] = value_array [j + 1];
> -        }
> -
> -      /* Skip the number_of_eviction entry.  */
> -      gcov_get_counter ();
> -      for (k = 0; k < GCOV_ICALL_TOPN_NCOUNTS - 1; k += 2)
> -        {
> -          int found = 0;
> -          gcov_type global_id = gcov_get_counter_target ();
> -          gcov_type call_count = gcov_get_counter ();
> -          for (m = 0; m < j; m += 2)
> -            {
> -              if (tmp_array[m] == global_id)
> -                {
> -                  found = 1;
> -                  tmp_array[m + 1] += call_count;
> -                  break;
> -                }
> -            }
> -          if (!found)
> -            {
> -              tmp_array[j] = global_id;
> -              tmp_array[j + 1] = call_count;
> -              j += 2;
> -            }
> -        }
> -      /* Now sort the temp array */
> -      gcov_sort_n_vals (tmp_array, j);
> -
> -      /* Now copy back the top half of the temp array */
> -      for (k = 0; k < GCOV_ICALL_TOPN_NCOUNTS - 1; k += 2)
> -        {
> -          value_array[k] = tmp_array[k];
> -          value_array[k + 1] = tmp_array[k + 1];
> -        }
> -    }
> -}
> -#endif /* L_gcov_merge_icall_topn */
>  #endif /* inhibit_libc */
> diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
> index 7116330252b..40f0858a174 100644
> --- a/libgcc/libgcov-profiler.c
> +++ b/libgcc/libgcov-profiler.c
> @@ -163,139 +163,6 @@ __gcov_one_value_profiler_atomic (gcov_type *counters, gcov_type 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
> -   entries are used.
> -   counter[0] --- the accumative count of the number of times one entry in
> -                  in the counters gets evicted/replaced due to limited capacity.
> -                  When this value reaches a threshold, the bottom N values are
> -                  cleared.
> -   counter[1] through counter[2*N] records the top 2*N values collected so far.
> -   Each value is represented by two entries: count[2*i+1] is the ith value, and
> -   count[2*i+2] is the number of times the value is seen.  */
> -
> -static void
> -__gcov_topn_value_profiler_body (gcov_type *counters, gcov_type value)
> -{
> -   unsigned i, found = 0, have_zero_count = 0;
> -   gcov_type *entry;
> -   gcov_type *lfu_entry = &counters[1];
> -   gcov_type *value_array = &counters[1];
> -   gcov_type *num_eviction = &counters[0];
> -   gcov_unsigned_t topn_val = GCOV_ICALL_TOPN_VAL;
> -
> -   /* There are 2*topn_val values tracked, each value takes two slots in the
> -      counter array.  */
> -   for (i = 0; i < (topn_val << 2); i += 2)
> -     {
> -       entry = &value_array[i];
> -       if (entry[0] == value)
> -         {
> -           entry[1]++ ;
> -           found = 1;
> -           break;
> -         }
> -       else if (entry[1] == 0)
> -         {
> -           lfu_entry = entry;
> -           have_zero_count = 1;
> -         }
> -      else if (entry[1] < lfu_entry[1])
> -        lfu_entry = entry;
> -     }
> -
> -   if (found)
> -     return;
> -
> -   /* lfu_entry is either an empty entry or an entry
> -      with lowest count, which will be evicted.  */
> -   lfu_entry[0] = value;
> -   lfu_entry[1] = 1;
> -
> -#define GCOV_ICALL_COUNTER_CLEAR_THRESHOLD 3000
> -
> -   /* Too many evictions -- time to clear bottom entries to
> -      avoid hot values bumping each other out.  */
> -   if (!have_zero_count
> -       && ++*num_eviction >= GCOV_ICALL_COUNTER_CLEAR_THRESHOLD)
> -     {
> -       unsigned i, j;
> -       gcov_type *p, minv;
> -       gcov_type* tmp_cnts
> -           = (gcov_type *)alloca (topn_val * sizeof (gcov_type));
> -
> -       *num_eviction = 0;
> -
> -       for (i = 0; i < topn_val; i++)
> -         tmp_cnts[i] = 0;
> -
> -       /* Find the largest topn_val values from the group of
> -          2*topn_val values and put them into tmp_cnts.  */
> -
> -       for (i = 0; i < 2 * topn_val; i += 2)
> -         {
> -           p = 0;
> -           for (j = 0; j < topn_val; j++)
> -             {
> -               if (!p || tmp_cnts[j] < *p)
> -                  p = &tmp_cnts[j];
> -             }
> -            if (value_array[i + 1] > *p)
> -              *p = value_array[i + 1];
> -         }
> -
> -       minv = tmp_cnts[0];
> -       for (j = 1; j < topn_val; j++)
> -         {
> -           if (tmp_cnts[j] < minv)
> -             minv = tmp_cnts[j];
> -         }
> -       /* Zero out low value entries.  */
> -       for (i = 0; i < 2 * topn_val; i += 2)
> -         {
> -           if (value_array[i + 1] < minv)
> -             {
> -               value_array[i] = 0;
> -               value_array[i + 1] = 0;
> -             }
> -         }
> -     }
> -}
> -
> -/* These two variables are used to actually track caller and callee.  Keep
> -   them in TLS memory so races are not common (they are written to often).
> -   The variables are set directly by GCC instrumented code, so declaration
> -   here must match one in tree-profile.c.  */
> -
> -#if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
> -__thread
> -#endif
> -struct indirect_call_tuple __gcov_indirect_call_topn;
> -
> -#ifdef TARGET_VTABLE_USES_DESCRIPTORS
> -#define VTABLE_USES_DESCRIPTORS 1
> -#else
> -#define VTABLE_USES_DESCRIPTORS 0
> -#endif
> -
> -/* This fucntion is instrumented at function entry to track topn indirect
> -   calls to CUR_FUNC.  */
> - 
> -void
> -__gcov_indirect_call_topn_profiler (gcov_type value, void* cur_func)
> -{
> -  void *callee_func = __gcov_indirect_call_topn.callee;
> -  /* 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 == callee_func
> -      || (VTABLE_USES_DESCRIPTORS && callee_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_v3
>  
>  /* These two variables are used to actually track caller and callee.  Keep
> diff --git a/libgcc/libgcov-util.c b/libgcc/libgcov-util.c
> index 6be229b094a..c794132c172 100644
> --- a/libgcc/libgcov-util.c
> +++ b/libgcc/libgcov-util.c
> @@ -740,25 +740,6 @@ __gcov_single_counter_op (gcov_type *counters, unsigned n_counters,
>      }
>  }
>  
> -/* Performing FN upon indirect-call profile counters.  */
> -
> -static void
> -__gcov_icall_topn_counter_op (gcov_type *counters, unsigned n_counters,
> -                              counter_op_fn fn, void *data1, void *data2)
> -{
> -  unsigned i;
> -
> -  gcc_assert (!(n_counters % GCOV_ICALL_TOPN_NCOUNTS));
> -  for (i = 0; i < n_counters; i += GCOV_ICALL_TOPN_NCOUNTS)
> -    {
> -      unsigned j;
> -      gcov_type *value_array = &counters[i + 1];
> -
> -      for (j = 0; j < GCOV_ICALL_TOPN_NCOUNTS - 1; j += 2)
> -        value_array[j + 1] = fn (value_array[j + 1], data1, data2);
> -    }
> -}
> -
>  /* Scaling the counter value V by multiplying *(float*) DATA1.  */
>  
>  static gcov_type
> diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
> index 993db8fb057..b4f1ec576fc 100644
> --- a/libgcc/libgcov.h
> +++ b/libgcc/libgcov.h
> @@ -102,7 +102,6 @@ typedef unsigned gcov_type_unsigned __attribute__ ((mode (QI)));
>  #define gcov_read_unsigned __gcov_read_unsigned
>  #define gcov_read_counter __gcov_read_counter
>  #define gcov_read_summary __gcov_read_summary
> -#define gcov_sort_n_vals __gcov_sort_n_vals
>  
>  #else /* IN_GCOV_TOOL */
>  /* About the host.  */
> @@ -130,7 +129,6 @@ typedef unsigned gcov_position_t;
>  #define L_gcov_merge_single 1
>  #define L_gcov_merge_ior 1
>  #define L_gcov_merge_time_profile 1
> -#define L_gcov_merge_icall_topn 1
>  
>  extern gcov_type gcov_read_counter_mem ();
>  extern unsigned gcov_get_merge_weight ();
> @@ -267,9 +265,6 @@ extern void __gcov_merge_single (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
>  /* The merge function that just ors the counters together.  */
>  extern void __gcov_merge_ior (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
>  
> -/* The merge function is used for topn indirect call counters.  */
> -extern void __gcov_merge_icall_topn (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
> -
>  /* The profiler functions.  */
>  extern void __gcov_interval_profiler (gcov_type *, gcov_type, int, unsigned);
>  extern void __gcov_interval_profiler_atomic (gcov_type *, gcov_type, int,
> @@ -285,8 +280,6 @@ extern void __gcov_average_profiler (gcov_type *, gcov_type);
>  extern void __gcov_average_profiler_atomic (gcov_type *, gcov_type);
>  extern void __gcov_ior_profiler (gcov_type *, gcov_type);
>  extern void __gcov_ior_profiler_atomic (gcov_type *, gcov_type);
> -extern void __gcov_indirect_call_topn_profiler (gcov_type, void *);
> -extern void gcov_sort_n_vals (gcov_type *, int);
>  
>  #ifndef inhibit_libc
>  /* The wrappers around some library functions..  */

Re: [PATCH 3/4] Dump histograms only if present.

[Jan Hubicka]

> On 6/6/19 2:16 PM, Jan Hubicka wrote:
> > What is the point of having histogram value when there are no counters?
> 
> Because we first create histograms in branch_prob -> gimple_find_values_to_profile and
> later we read profile from file. At that point we know which are empty, but we don't
> remove them.

OK, so it is about the dump not crahsing when you do it in meantime.
That makes sense :)
So patch is OK.

Honza

Re: [PATCH 1/4] Remove indirect call top N counter type.

[Martin Liška]

On 6/6/19 2:52 PM, Jan Hubicka wrote:
> Hi,
> so the only point of removing this is the fact that builds would be
> not reproducible with indir-call-topn-profile?

That's one reason. But the main reason is that the code is dead and not used.
If you take a look at gcc/ipa-profile.c:189, the code is not supporting HIST_TYPE_INDIR_CALL_TOPN.
So it's broken for few years and nobody is using that. Last reason is implementation of:
__gcov_merge_icall_topn. It's over-complicated.

Martin

> I still kind of thing it may be useful to track multiple most common
> values, so I would be in favour of keeping it just updating the documentation
> of indir-call-topn-profile that it is currently incomplete and does not
> lead to reproducible builds and does not handle speculation...
> 
> Honza

Re: [PATCH 1/4] Remove indirect call top N counter type.

[Jan Hubicka]

> On 6/6/19 2:52 PM, Jan Hubicka wrote:
> > Hi,
> > so the only point of removing this is the fact that builds would be
> > not reproducible with indir-call-topn-profile?
> 
> That's one reason. But the main reason is that the code is dead and not used.
> If you take a look at gcc/ipa-profile.c:189, the code is not supporting HIST_TYPE_INDIR_CALL_TOPN.

Yep, i know. It is not that hard to add support for this - we currently
assume that speculative edges have one direct and one indirect call.
We need to add support to have multiple indirect calls which would not
be too hard, but it never got high enough in my TODO list.

Option is to do the mutiple value speuclation the old way dirrect in
VPT.

I was thinking that other possible use of TOPN is the division used in
growing hashtable implementations. Those usually have small set of built
in prime numbers which they use as array size and modulo always use one
of the values. This is however usually not quite visible to compiler
and TOPN counter could work that out.  So we could do automatically more
or less what htab_mod does.

But yep, I have no time to work on this currently, so we may just drop
it and see if we will want to recover it later.
Honza

> So it's broken for few years and nobody is using that. Last reason is implementation of:
> __gcov_merge_icall_topn. It's over-complicated.
> 
> Martin
> 
> > I still kind of thing it may be useful to track multiple most common
> > values, so I would be in favour of keeping it just updating the documentation
> > of indir-call-topn-profile that it is currently incomplete and does not
> > lead to reproducible builds and does not handle speculation...
> > 
> > Honza
> 

Re: [PATCH 2/4] Implement N disk counters for single value and indirect call counters.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 7877 bytes --]

On 6/6/19 2:50 PM, Jan Hubicka wrote:
>>
>> gcc/ChangeLog:
>>
>> 2019-06-04  Martin Liska  <mliska@suse.cz>
>>
>> 	* gcov-io.h (GCOV_DISK_SINGLE_VALUES): New.
>> 	(GCOV_SINGLE_VALUE_COUNTERS): Likewise.
>> 	* ipa-profile.c (ipa_profile_generate_summary):
>> 	Use get_most_common_single_value.
>> 	* tree-profile.c (gimple_init_gcov_profiler):
>> 	Instrument with __gcov_one_value_profiler_v2
>> 	and __gcov_indirect_call_profiler_v4.
>> 	* value-prof.c (dump_histogram_value):
>> 	Print all values for HIST_TYPE_SINGLE_VALUE.
>> 	(stream_in_histogram_value): Set number of
>> 	counters for HIST_TYPE_SINGLE_VALUE.
>> 	(get_most_common_single_value): New.
>> 	(gimple_divmod_fixed_value_transform):
>> 	Use get_most_common_single_value.
>> 	(gimple_ic_transform): Likewise.
>> 	(gimple_stringops_transform): Likewise.
>> 	(gimple_find_values_to_profile): Set number
>> 	of counters for HIST_TYPE_SINGLE_VALUE.
>> 	* value-prof.h (get_most_common_single_value):
>> 	New.
>>
>> libgcc/ChangeLog:
>>
>> 2019-06-04  Martin Liska  <mliska@suse.cz>
>>
>> 	* Makefile.in: Add __gcov_one_value_profiler_v2 and
>> 	__gcov_indirect_call_profiler_v4.
>> 	* libgcov-merge.c (__gcov_merge_single): Change
>> 	function signature.
>> 	(merge_single_value_set): New.
>> 	* libgcov-profiler.c (__gcov_one_value_profiler_body):
>> 	Do not update counters[2].
>> 	(__gcov_one_value_profiler): Remove.
>> 	(__gcov_one_value_profiler_atomic): Rename to ...
>> 	(__gcov_one_value_profiler_v2): ... this.
>> 	(__gcov_indirect_call_profiler_v3): Rename to ...
>> 	(__gcov_indirect_call_profiler_v4): ... this.
>> 	* libgcov.h (__gcov_one_value_profiler): Remove.
>> 	(__gcov_one_value_profiler_atomic): Remove.
>> 	(__gcov_indirect_call_profiler_v3): Remove.
>> 	(__gcov_one_value_profiler_v2): New.
>> 	(__gcov_indirect_call_profiler_v4): New.
>> ---
>>  gcc/gcov-io.h             |   7 +++
>>  gcc/ipa-profile.c         |  13 +++--
>>  gcc/tree-profile.c        |   9 ++-
>>  gcc/value-prof.c          | 120 ++++++++++++++++++++------------------
>>  gcc/value-prof.h          |   2 +
>>  libgcc/Makefile.in        |   5 +-
>>  libgcc/libgcov-merge.c    |  77 ++++++++++++++++--------
>>  libgcc/libgcov-profiler.c |  43 +++-----------
>>  libgcc/libgcov.h          |   5 +-
>>  9 files changed, 147 insertions(+), 134 deletions(-)
>>
> 
>> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
>> index f2cf4047579..008a1271979 100644
>> --- a/gcc/tree-profile.c
>> +++ b/gcc/tree-profile.c
>> @@ -165,10 +165,9 @@ gimple_init_gcov_profiler (void)
>>  	      = build_function_type_list (void_type_node,
>>  					  gcov_type_ptr, gcov_type_node,
>>  					  NULL_TREE);
>> -      fn_name = concat ("__gcov_one_value_profiler", fn_suffix, NULL);
>> -      tree_one_value_profiler_fn = build_fn_decl (fn_name,
>> -						  one_value_profiler_fn_type);
>> -      free (CONST_CAST (char *, fn_name));
>> +      tree_one_value_profiler_fn
>> +	= build_fn_decl ("__gcov_one_value_profiler_v2",
>> +			 one_value_profiler_fn_type);
> 
> Why you no longer need the optional atomic suffix here?

Because I removed hist->hvalue.counters[2] where we stored total number of executions.
It's back again so that _atomic suffix version makes sense again.

>> diff --git a/gcc/value-prof.c b/gcc/value-prof.c
>> index 1e14e532070..e893ca084c9 100644
>> --- a/gcc/value-prof.c
>> +++ b/gcc/value-prof.c
>> @@ -259,15 +259,19 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
>>        break;
>>  
>>      case HIST_TYPE_SINGLE_VALUE:
>> -      fprintf (dump_file, "Single value ");
>> +    case HIST_TYPE_INDIR_CALL:
>> +      fprintf (dump_file, (hist->type == HIST_TYPE_SINGLE_VALUE
>> +			   ? "Single values " : "Indirect call "));
>>        if (hist->hvalue.counters)
>>  	{
>> -	   fprintf (dump_file, "value:%" PRId64
>> -		    " match:%" PRId64
>> -		    " wrong:%" PRId64,
>> -		    (int64_t) hist->hvalue.counters[0],
>> -		    (int64_t) hist->hvalue.counters[1],
>> -		    (int64_t) hist->hvalue.counters[2]);
>> +	  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
>> +	    {
>> +	      fprintf (dump_file, "[%" PRId64 ":%" PRId64 "]",
>> +		       (int64_t) hist->hvalue.counters[2 * i],
>> +		       (int64_t) hist->hvalue.counters[2 * i + 1]);
>> +	      if (i != GCOV_DISK_SINGLE_VALUES - 1)
>> +		fprintf (dump_file, ", ");
>> +	    }
> Unless there are some compelling reasons, I would still include in dump what is
> meaning of the values - not everyone is fluent in that ;)

I improved that.

>> @@ -758,23 +779,19 @@ gimple_divmod_fixed_value_transform (gimple_stmt_iterator *si)
>>    if (!histogram)
>>      return false;
>>  
>> +  if (!get_most_common_single_value (histogram, &val, &count))
>> +    return false;
>> +
>>    value = histogram->hvalue.value;
>> -  val = histogram->hvalue.counters[0];
>> -  count = histogram->hvalue.counters[1];
>> -  all = histogram->hvalue.counters[2];
>> +  all = gimple_bb (stmt)->count.ipa ().to_gcov_type ();
> 
> Here it is safe to use gimple_bb (stmt)->count.ipa ().to_gcov_type () under an
> assumptions that profile is consistent - I am not sure how often one gets
> mismatches with -fprofile-correction at multithreaded programs, maybe it would
> be worth to check before we drop the code logging it to dumps.
> 
> In ipa-profile it is not quite the case since we do CFG optimization and other
> transforms between profile instrumentaiton and IPA passes.  I would remember
> probability within stmt histogram (like we do for speculative edges) that is
> safe WRT BB profile updates.

I put back the total number of executions, now sitting at hist->hvalue.counters[0].

> 
> So in your implementation we store 4 times as many vlaue counters in memory
> during runtime?  Originally I had in mind a variant where in memory everything
> works as before, but in on-disk format one save the section for indirect
> call counters multiple times implementing the update logic.
> This should not be that hard do something:
> 
>  for secno = 1...4
>    for every counter
>      read counter2 from disk
>      if (counter has non-zero exec count)
>        if counter2 is invalid
> 	 clear counter
>        else if counter and counter2 values match
>          increase counter2 hitrate
> 	 clear counter
>        else if counter2 has zero count
> 	 rewrite counter2 by counter
> 	 clear counter
>        else if secno==5
> 	 mark counter2 as invalid
> 	 clear ocunter
>      write counter2 to disk
> 
> So you do not really need to keep duplicates in memory at runtime.
> You will still be able to tell if there was too many counters by checking
> the last copy (instead of first as you do in your implementation)

After we discussed that I decided to live with all counters in memory. Reason is that
in write_one_data we would have to allocate temporary buffers and fill up them here:

   304            tag = gcov_read_unsigned ();
   305            length = gcov_read_unsigned ();
   306            if (tag != GCOV_TAG_FOR_COUNTER (t_ix)
   307                || length != GCOV_TAG_COUNTER_LENGTH (ci_ptr->num))
   308              goto read_mismatch;
   309            (*merge) (ci_ptr->values, ci_ptr->num);
   310            ci_ptr++;

It would be quite nasty and I would like to mitigate libgcov profile crashes.

>> +      value = gcov_get_counter_target ();
>> +      counter = gcov_get_counter ();
>> +
>> +      if (counter == -1)
>> +	{
>> +	  counters[1] = -1;
>> +	  /* We can't return as we need to read all counters.  */
>> +	  continue;
>> +	}
>> +      else if (counter == 0 || counters[1] == -1)
>> +	{
>> +	  /* We can't return as we need to read all counters.  */
>> +	  continue;
>> +	}
> gcov_get_counter scales by gcov_get_merge_weight and thus it will
> not reliably read -1 when you store -1

Ah, you are right. I've fixed that.

I'm sending next version of the patch which I've been testing.

Martin

> 
> Honza
> 


[-- Attachment #2: 0002-Implement-N-disk-counters-for-single-value-and-indir.patch --]
[-- Type: text/x-patch, Size: 22245 bytes --]

From 241d4042c542fb64f38ccb602631d5e7bd689684 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 6 Jun 2019 10:17:13 +0200
Subject: [PATCH 2/4] Implement N disk counters for single value and indirect
 call counters.

gcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* gcov-io.h (GCOV_DISK_SINGLE_VALUES): New.
	(GCOV_SINGLE_VALUE_COUNTERS): Likewise.
	* ipa-profile.c (ipa_profile_generate_summary):
	Use get_most_common_single_value.
	* tree-profile.c (gimple_init_gcov_profiler):
	Instrument with __gcov_one_value_profiler_v2
	and __gcov_indirect_call_profiler_v4.
	* value-prof.c (dump_histogram_value):
	Print all values for HIST_TYPE_SINGLE_VALUE.
	(stream_out_histogram_value): Update assert for
	N values.
	(stream_in_histogram_value): Set number of
	counters for HIST_TYPE_SINGLE_VALUE.
	(get_most_common_single_value): New.
	(gimple_divmod_fixed_value_transform):
	Use get_most_common_single_value.
	(gimple_ic_transform): Likewise.
	(gimple_stringops_transform): Likewise.
	(gimple_find_values_to_profile): Set number
	of counters for HIST_TYPE_SINGLE_VALUE.
	* value-prof.h (get_most_common_single_value):
	New.

libgcc/ChangeLog:

2019-06-04  Martin Liska  <mliska@suse.cz>

	* Makefile.in: Add __gcov_one_value_profiler_v2,
	__gcov_one_value_profiler_v2_atomic and
	__gcov_indirect_call_profiler_v4.
	* libgcov-merge.c (__gcov_merge_single): Change
	function signature.
	(merge_single_value_set): New.
	* libgcov-profiler.c (__gcov_one_value_profiler_body):
	Update functionality.
	(__gcov_one_value_profiler): Remove.
	(__gcov_one_value_profiler_v2): ... this.
	(__gcov_one_value_profiler_atomic): Rename to ...
	(__gcov_one_value_profiler_v2_atomic): this.
	(__gcov_indirect_call_profiler_v3): Rename to ...
	(__gcov_indirect_call_profiler_v4): ... this.
	* libgcov.h (__gcov_one_value_profiler): Remove.
	(__gcov_one_value_profiler_atomic): Remove.
	(__gcov_one_value_profiler_v2_atomic): New.
	(__gcov_indirect_call_profiler_v3): Remove.
	(__gcov_one_value_profiler_v2): New.
	(__gcov_indirect_call_profiler_v4): New.
	(gcov_get_counter_ignore_scaling): New function.
---
 gcc/gcov-io.h             |   7 ++
 gcc/ipa-profile.c         |  12 ++--
 gcc/tree-profile.c        |   6 +-
 gcc/value-prof.c          | 136 ++++++++++++++++++++++----------------
 gcc/value-prof.h          |   4 ++
 libgcc/Makefile.in        |   6 +-
 libgcc/libgcov-merge.c    |  83 ++++++++++++++++-------
 libgcc/libgcov-profiler.c |  33 +++++----
 libgcc/libgcov.h          |  29 +++++++-
 9 files changed, 200 insertions(+), 116 deletions(-)

diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index 69c9a73dba8..0f2905c17ec 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -266,6 +266,13 @@ GCOV_COUNTERS
 #define GCOV_N_VALUE_COUNTERS \
   (GCOV_LAST_VALUE_COUNTER - GCOV_FIRST_VALUE_COUNTER + 1)
 
+/* Number of single value histogram values that live
+   on disk representation.  */
+#define GCOV_DISK_SINGLE_VALUES 4
+
+/* Total number of single value counters.  */
+#define GCOV_SINGLE_VALUE_COUNTERS (2 * GCOV_DISK_SINGLE_VALUES + 1)
+
 /* 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/ipa-profile.c b/gcc/ipa-profile.c
index de9563d808c..c80ea7a9b95 100644
--- a/gcc/ipa-profile.c
+++ b/gcc/ipa-profile.c
@@ -191,17 +191,17 @@ ipa_profile_generate_summary (void)
 		     takes away bad histograms.  */
 		  if (h)
 		    {
-		      /* counter 0 is target, counter 1 is number of execution we called target,
-			 counter 2 is total number of executions.  */
-		      if (h->hvalue.counters[2])
+		      gcov_type val, count, all;
+		      if (get_most_common_single_value (NULL, "indirect call",
+							h, &val, &count, &all))
 			{
 			  struct cgraph_edge * e = node->get_edge (stmt);
 			  if (e && !e->indirect_unknown_callee)
 			    continue;
-			  e->indirect_info->common_target_id
-			    = h->hvalue.counters [0];
+
+			  e->indirect_info->common_target_id = val;
 			  e->indirect_info->common_target_probability
-			    = GCOV_COMPUTE_SCALE (h->hvalue.counters [1], h->hvalue.counters [2]);
+			    = GCOV_COMPUTE_SCALE (count, all);
 			  if (e->indirect_info->common_target_probability > REG_BR_PROB_BASE)
 			    {
 			      if (dump_file)
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index f2cf4047579..5ca4c3e80b6 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -165,10 +165,10 @@ gimple_init_gcov_profiler (void)
 	      = build_function_type_list (void_type_node,
 					  gcov_type_ptr, gcov_type_node,
 					  NULL_TREE);
-      fn_name = concat ("__gcov_one_value_profiler", fn_suffix, NULL);
+      fn_name = concat ("__gcov_one_value_profiler_v2", fn_suffix, NULL);
       tree_one_value_profiler_fn = build_fn_decl (fn_name,
 						  one_value_profiler_fn_type);
-      free (CONST_CAST (char *, fn_name));
+
       TREE_NOTHROW (tree_one_value_profiler_fn) = 1;
       DECL_ATTRIBUTES (tree_one_value_profiler_fn)
 	= tree_cons (get_identifier ("leaf"), NULL,
@@ -182,7 +182,7 @@ gimple_init_gcov_profiler (void)
 					  gcov_type_node,
 					  ptr_type_node,
 					  NULL_TREE);
-      profiler_fn_name = "__gcov_indirect_call_profiler_v3";
+      profiler_fn_name = "__gcov_indirect_call_profiler_v4";
 
       tree_indirect_call_profiler_fn
 	      = build_fn_decl (profiler_fn_name, ic_profiler_fn_type);
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index 1e14e532070..b95bf85270b 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -259,15 +259,22 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
       break;
 
     case HIST_TYPE_SINGLE_VALUE:
-      fprintf (dump_file, "Single value ");
+    case HIST_TYPE_INDIR_CALL:
+      fprintf (dump_file,
+	       (hist->type == HIST_TYPE_SINGLE_VALUE
+		? "Single value counter " : "Indirect call counter"));
       if (hist->hvalue.counters)
 	{
-	   fprintf (dump_file, "value:%" PRId64
-		    " match:%" PRId64
-		    " wrong:%" PRId64,
-		    (int64_t) hist->hvalue.counters[0],
-		    (int64_t) hist->hvalue.counters[1],
-		    (int64_t) hist->hvalue.counters[2]);
+	  fprintf (dump_file, "all: %" PRId64 ", values: ",
+		   (int64_t) hist->hvalue.counters[0]);
+	  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
+	    {
+	      fprintf (dump_file, "[%" PRId64 ":%" PRId64 "]",
+		       (int64_t) hist->hvalue.counters[2 * i + 1],
+		       (int64_t) hist->hvalue.counters[2 * i + 2]);
+	      if (i != GCOV_DISK_SINGLE_VALUES - 1)
+		fprintf (dump_file, ", ");
+	    }
 	}
       fprintf (dump_file, ".\n");
       break;
@@ -294,19 +301,6 @@ dump_histogram_value (FILE *dump_file, histogram_value hist)
       fprintf (dump_file, ".\n");
       break;
 
-    case HIST_TYPE_INDIR_CALL:
-      fprintf (dump_file, "Indirect call ");
-      if (hist->hvalue.counters)
-	{
-	   fprintf (dump_file, "value:%" PRId64
-		    " match:%" PRId64
-		    " all:%" PRId64,
-		    (int64_t) hist->hvalue.counters[0],
-		    (int64_t) hist->hvalue.counters[1],
-		    (int64_t) hist->hvalue.counters[2]);
-	}
-      fprintf (dump_file, ".\n");
-      break;
     case HIST_TYPE_TIME_PROFILE:
       fprintf (dump_file, "Time profile ");
       if (hist->hvalue.counters)
@@ -347,7 +341,7 @@ stream_out_histogram_value (struct output_block *ob, histogram_value hist)
       /* When user uses an unsigned type with a big value, constant converted
 	 to gcov_type (a signed type) can be negative.  */
       gcov_type value = hist->hvalue.counters[i];
-      if (hist->type == HIST_TYPE_SINGLE_VALUE && i == 0)
+      if (hist->type == HIST_TYPE_SINGLE_VALUE && (i > 0 && ((i - 1) % 2) == 0))
 	;
       else
 	gcc_assert (value >= 0);
@@ -392,7 +386,7 @@ stream_in_histogram_value (struct lto_input_block *ib, gimple *stmt)
 
 	case HIST_TYPE_SINGLE_VALUE:
 	case HIST_TYPE_INDIR_CALL:
-	  ncounters = 3;
+	  ncounters = GCOV_SINGLE_VALUE_COUNTERS;
 	  break;
 
 	case HIST_TYPE_IOR:
@@ -729,6 +723,48 @@ gimple_divmod_fixed_value (gassign *stmt, tree value, profile_probability prob,
   return tmp2;
 }
 
+/* Return most common value of SINGLE_VALUE histogram.  If
+   there's a unique value, return true and set VALUE and COUNT
+   arguments.  */
+
+bool
+get_most_common_single_value (gimple *stmt, const char *counter_type,
+			      histogram_value hist,
+			      gcov_type *value, gcov_type *count,
+			      gcov_type *all)
+{
+  if (hist->hvalue.counters[2] == -1)
+    return false;
+
+  *count = 0;
+  *value = 0;
+
+  gcov_type read_all = hist->hvalue.counters[0];
+
+  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
+    {
+      gcov_type v = hist->hvalue.counters[2 * i + 1];
+      gcov_type c = hist->hvalue.counters[2 * i + 2];
+
+      /* Indirect calls can't be vereified.  */
+      if (stmt && check_counter (stmt, counter_type, &c, &read_all,
+				 gimple_bb (stmt)->count))
+	return false;
+
+      *all = read_all;
+
+      if (c > *count)
+	{
+	  *value = v;
+	  *count = c;
+	}
+      else if (c == *count && v > *value)
+	*value = v;
+    }
+
+  return true;
+}
+
 /* Do transform 1) on INSN if applicable.  */
 
 static bool
@@ -758,23 +794,19 @@ gimple_divmod_fixed_value_transform (gimple_stmt_iterator *si)
   if (!histogram)
     return false;
 
+  if (!get_most_common_single_value (stmt, "divmod", histogram, &val, &count,
+				     &all))
+    return false;
+
   value = histogram->hvalue.value;
-  val = histogram->hvalue.counters[0];
-  count = histogram->hvalue.counters[1];
-  all = histogram->hvalue.counters[2];
   gimple_remove_histogram_value (cfun, stmt, histogram);
 
-  /* We require that count is at least half of all; this means
-     that for the transformation to fire the value must be constant
-     at least 50% of time (and 75% gives the guarantee of usage).  */
+  /* We require that count is at least half of all.  */
   if (simple_cst_equal (gimple_assign_rhs2 (stmt), value) != 1
       || 2 * count < all
       || optimize_bb_for_size_p (gimple_bb (stmt)))
     return false;
 
-  if (check_counter (stmt, "value", &count, &all, gimple_bb (stmt)->count))
-    return false;
-
   /* Compute probability of taking the optimal path.  */
   if (all > 0)
     prob = profile_probability::probability_in_gcov_type (count, all);
@@ -1401,7 +1433,7 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
 {
   gcall *stmt;
   histogram_value histogram;
-  gcov_type val, count, all, bb_all;
+  gcov_type val, count, all;
   struct cgraph_node *direct_call;
 
   stmt = dyn_cast <gcall *> (gsi_stmt (*gsi));
@@ -1418,21 +1450,9 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
   if (!histogram)
     return false;
 
-  val = histogram->hvalue.counters [0];
-  count = histogram->hvalue.counters [1];
-  all = histogram->hvalue.counters [2];
-
-  bb_all = gimple_bb (stmt)->count.ipa ().to_gcov_type ();
-  /* The order of CHECK_COUNTER calls is important -
-     since check_counter can correct the third parameter
-     and we want to make count <= all <= bb_all. */
-  if (check_counter (stmt, "ic", &all, &bb_all, gimple_bb (stmt)->count)
-      || check_counter (stmt, "ic", &count, &all,
-		        profile_count::from_gcov_type (all)))
-    {
-      gimple_remove_histogram_value (cfun, stmt, histogram);
-      return false;
-    }
+  if (!get_most_common_single_value (NULL, "indirect call", histogram, &val,
+				     &count, &all))
+    return false;
 
   if (4 * count <= 3 * all)
     return false;
@@ -1644,19 +1664,19 @@ gimple_stringops_transform (gimple_stmt_iterator *gsi)
   if (TREE_CODE (blck_size) == INTEGER_CST)
     return false;
 
-  histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_SINGLE_VALUE);
+  histogram = gimple_histogram_value_of_type (cfun, stmt,
+					      HIST_TYPE_SINGLE_VALUE);
   if (!histogram)
     return false;
 
-  val = histogram->hvalue.counters[0];
-  count = histogram->hvalue.counters[1];
-  all = histogram->hvalue.counters[2];
+  if (!get_most_common_single_value (stmt, "stringops", histogram, &val,
+				     &count, &all))
+    return false;
+
   gimple_remove_histogram_value (cfun, stmt, histogram);
 
-  /* We require that count is at least half of all; this means
-     that for the transformation to fire the value must be constant
-     at least 80% of time.  */
-  if ((6 * count / 5) < all || optimize_bb_for_size_p (gimple_bb (stmt)))
+  /* We require that count is at least half of all.  */
+  if (2 * count < all || optimize_bb_for_size_p (gimple_bb (stmt)))
     return false;
   if (check_counter (stmt, "value", &count, &all, gimple_bb (stmt)->count))
     return false;
@@ -1928,11 +1948,11 @@ gimple_find_values_to_profile (histogram_values *values)
 	  break;
 
 	case HIST_TYPE_SINGLE_VALUE:
-	  hist->n_counters = 3;
+	  hist->n_counters = GCOV_SINGLE_VALUE_COUNTERS;
 	  break;
 
- 	case HIST_TYPE_INDIR_CALL:
- 	  hist->n_counters = 3;
+	case HIST_TYPE_INDIR_CALL:
+	  hist->n_counters = GCOV_SINGLE_VALUE_COUNTERS;
 	  break;
 
         case HIST_TYPE_TIME_PROFILE:
diff --git a/gcc/value-prof.h b/gcc/value-prof.h
index a54024b48de..25b03f7591a 100644
--- a/gcc/value-prof.h
+++ b/gcc/value-prof.h
@@ -90,6 +90,10 @@ void free_histograms (function *);
 void stringop_block_profile (gimple *, unsigned int *, HOST_WIDE_INT *);
 gcall *gimple_ic (gcall *, struct cgraph_node *, profile_probability);
 bool check_ic_target (gcall *, struct cgraph_node *);
+bool get_most_common_single_value (gimple *stmt, const char *counter_type,
+				   histogram_value hist,
+				   gcov_type *value, gcov_type *count,
+				   gcov_type *all);
 
 
 /* In tree-profile.c.  */
diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index fb77881145e..33b83809cfc 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -893,13 +893,13 @@ LIBGCOV_PROFILER = _gcov_interval_profiler				\
 	_gcov_interval_profiler_atomic					\
 	_gcov_pow2_profiler						\
 	_gcov_pow2_profiler_atomic					\
-	_gcov_one_value_profiler					\
-	_gcov_one_value_profiler_atomic					\
+	_gcov_one_value_profiler_v2					\
+	_gcov_one_value_profiler_v2_atomic					\
 	_gcov_average_profiler						\
 	_gcov_average_profiler_atomic					\
 	_gcov_ior_profiler						\
 	_gcov_ior_profiler_atomic					\
-	_gcov_indirect_call_profiler_v3					\
+	_gcov_indirect_call_profiler_v4					\
 	_gcov_time_profiler
 LIBGCOV_INTERFACE = _gcov_dump _gcov_flush _gcov_fork			\
 	_gcov_execl _gcov_execlp					\
diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
index 702a69f8349..42416341b4d 100644
--- a/libgcc/libgcov-merge.c
+++ b/libgcc/libgcov-merge.c
@@ -34,8 +34,9 @@ void __gcov_merge_add (gcov_type *counters  __attribute__ ((unused)),
 #endif
 
 #ifdef L_gcov_merge_single
-void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)),
-                          unsigned n_counters __attribute__ ((unused))) {}
+void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
+{
+}
 #endif
 
 #else
@@ -85,40 +86,72 @@ __gcov_merge_time_profile (gcov_type *counters, unsigned n_counters)
 #endif /* L_gcov_merge_time_profile */
 
 #ifdef L_gcov_merge_single
+
+static void
+merge_single_value_set (gcov_type *counters)
+{
+  unsigned j;
+  gcov_type value, counter;
+
+  /* First value is number of total executions of the profiler.  */
+  gcov_type all = gcov_get_counter_ignore_scaling (-1);
+  counters[0] += all;
+  ++counters;
+
+  for (unsigned i = 0; i < GCOV_DISK_SINGLE_VALUES; i++)
+    {
+      value = gcov_get_counter_target ();
+      counter = gcov_get_counter_ignore_scaling (-1);
+
+      if (counter == -1)
+	{
+	  counters[1] = -1;
+	  /* We can't return as we need to read all counters.  */
+	  continue;
+	}
+      else if (counter == 0 || counters[1] == -1)
+	{
+	  /* We can't return as we need to read all counters.  */
+	  continue;
+	}
+
+      for (j = 0; j < GCOV_DISK_SINGLE_VALUES; j++)
+	{
+	  if (counters[2 * j] == value)
+	    {
+	      counters[2 * j + 1] += counter;
+	      break;
+	    }
+	  else if (counters[2 * j + 1] == 0)
+	    {
+	      counters[2 * j] = value;
+	      counters[2 * j + 1] = counter;
+	      break;
+	    }
+	}
+
+      /* We haven't found a free slot for the value, mark overflow.  */
+      if (j == GCOV_DISK_SINGLE_VALUES)
+	counters[1] = -1;
+    }
+}
+
 /* The profile merging function for choosing the most common value.
    It is given an array COUNTERS of N_COUNTERS old counters and it
    reads the same number of counters from the gcov file.  The counters
-   are split into 3-tuples where the members of the tuple have
+   are split into pairs where the members of the tuple have
    meanings:
 
    -- the stored candidate on the most common value of the measured entity
    -- counter
-   -- total number of evaluations of the value  */
+   */
 void
 __gcov_merge_single (gcov_type *counters, unsigned n_counters)
 {
-  unsigned i, n_measures;
-  gcov_type value, counter, all;
+  gcc_assert (!(n_counters % GCOV_SINGLE_VALUE_COUNTERS));
 
-  gcc_assert (!(n_counters % 3));
-  n_measures = n_counters / 3;
-  for (i = 0; i < n_measures; i++, counters += 3)
-    {
-      value = gcov_get_counter_target ();
-      counter = gcov_get_counter ();
-      all = gcov_get_counter ();
-
-      if (counters[0] == value)
-        counters[1] += counter;
-      else if (counter > counters[1])
-        {
-          counters[0] = value;
-          counters[1] = counter - counters[1];
-        }
-      else
-        counters[1] -= counter;
-      counters[2] += all;
-    }
+  for (unsigned i = 0; i < (n_counters / GCOV_SINGLE_VALUE_COUNTERS); i++)
+    merge_single_value_set (counters + (i * GCOV_SINGLE_VALUE_COUNTERS));
 }
 #endif /* L_gcov_merge_single */
 
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 40f0858a174..9ba65b90df3 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -112,40 +112,37 @@ __gcov_pow2_profiler_atomic (gcov_type *counters, gcov_type value)
    COUNTERS[1] is decremented.  Otherwise COUNTERS[1] is set to one and
    VALUE is stored to COUNTERS[0].  This algorithm guarantees that if this
    function is called more than 50% of the time with one value, this value
-   will be in COUNTERS[0] in the end.
-
-   In any case, COUNTERS[2] is incremented.  If USE_ATOMIC is set to 1,
-   COUNTERS[2] is updated with an atomic instruction.  */
+   will be in COUNTERS[0] in the end.  */
 
 static inline void
 __gcov_one_value_profiler_body (gcov_type *counters, gcov_type value,
 				int use_atomic)
 {
-  if (value == counters[0])
-    counters[1]++;
-  else if (counters[1] == 0)
+  if (value == counters[1])
+    counters[2]++;
+  else if (counters[2] == 0)
     {
-      counters[1] = 1;
-      counters[0] = value;
+      counters[2] = 1;
+      counters[1] = value;
     }
   else
-    counters[1]--;
+    counters[2]--;
 
   if (use_atomic)
-    __atomic_fetch_add (&counters[2], 1, __ATOMIC_RELAXED);
+    __atomic_fetch_add (&counters[0], 1, __ATOMIC_RELAXED);
   else
-    counters[2]++;
+    counters[0]++;
 }
 
-#ifdef L_gcov_one_value_profiler
+#ifdef L_gcov_one_value_profiler_v2
 void
-__gcov_one_value_profiler (gcov_type *counters, gcov_type value)
+__gcov_one_value_profiler_v2 (gcov_type *counters, gcov_type value)
 {
   __gcov_one_value_profiler_body (counters, value, 0);
 }
 #endif
 
-#if defined(L_gcov_one_value_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
+#if defined(L_gcov_one_value_profiler_v2_atomic) && GCOV_SUPPORTS_ATOMIC
 
 /* Update one value profilers (COUNTERS) for a given VALUE.
 
@@ -157,13 +154,13 @@ __gcov_one_value_profiler (gcov_type *counters, gcov_type value)
    https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00024.html.  */
 
 void
-__gcov_one_value_profiler_atomic (gcov_type *counters, gcov_type value)
+__gcov_one_value_profiler_v2_atomic (gcov_type *counters, gcov_type value)
 {
   __gcov_one_value_profiler_body (counters, value, 1);
 }
 #endif
 
-#ifdef L_gcov_indirect_call_profiler_v3
+#ifdef L_gcov_indirect_call_profiler_v4
 
 /* These two variables are used to actually track caller and callee.  Keep
    them in TLS memory so races are not common (they are written to often).
@@ -185,7 +182,7 @@ struct indirect_call_tuple __gcov_indirect_call;
 
 /* Tries to determine the most common value among its inputs. */
 void
-__gcov_indirect_call_profiler_v3 (gcov_type value, void* cur_func)
+__gcov_indirect_call_profiler_v4 (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
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index b4f1ec576fc..144b4817a37 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -271,9 +271,9 @@ extern void __gcov_interval_profiler_atomic (gcov_type *, gcov_type, int,
 					     unsigned);
 extern void __gcov_pow2_profiler (gcov_type *, gcov_type);
 extern void __gcov_pow2_profiler_atomic (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_v3 (gcov_type, void *);
+extern void __gcov_one_value_profiler_v2 (gcov_type *, gcov_type);
+extern void __gcov_one_value_profiler_v2_atomic (gcov_type *, gcov_type);
+extern void __gcov_indirect_call_profiler_v4 (gcov_type, void *);
 extern void __gcov_time_profiler (gcov_type *);
 extern void __gcov_time_profiler_atomic (gcov_type *);
 extern void __gcov_average_profiler (gcov_type *, gcov_type);
@@ -324,6 +324,29 @@ gcov_get_counter (void)
 #endif
 }
 
+/* Similar function as gcov_get_counter(), but do not scale
+   when read value is equal to IGNORE_SCALING.  */
+
+static inline gcov_type
+gcov_get_counter_ignore_scaling (gcov_type ignore_scaling)
+{
+#ifndef IN_GCOV_TOOL
+  /* This version is for reading count values in libgcov runtime:
+     we read from gcda files.  */
+
+  return gcov_read_counter ();
+#else
+  /* This version is for gcov-tool. We read the value from memory and
+     multiply it by the merge weight.  */
+
+  gcov_type v = gcov_read_counter_mem ();
+  if (v != ignore_scaling)
+    v *= gcov_get_merge_weight ();
+
+  return v;
+#endif
+}
+
 /* Similar function as gcov_get_counter(), but handles target address
    counters.  */
 
-- 
2.21.0

Re: [PATCH 2/4] Implement N disk counters for single value and indirect call counters.

[Jan Hubicka]

> 
> Because I removed hist->hvalue.counters[2] where we stored total number of executions.
> It's back again so that _atomic suffix version makes sense again.

OK and we do not care about race conditions on the other two counters?
> 
> After we discussed that I decided to live with all counters in memory. Reason is that
> in write_one_data we would have to allocate temporary buffers and fill up them here:
> 
>    304            tag = gcov_read_unsigned ();
>    305            length = gcov_read_unsigned ();
>    306            if (tag != GCOV_TAG_FOR_COUNTER (t_ix)
>    307                || length != GCOV_TAG_COUNTER_LENGTH (ci_ptr->num))
>    308              goto read_mismatch;
>    309            (*merge) (ci_ptr->values, ci_ptr->num);
>    310            ci_ptr++;
> 
> It would be quite nasty and I would like to mitigate libgcov profile crashes.

Hmm, I see, if code was organized to directly write the output, it would
be easier.  I guess it is not overly important.

Patch is OK
Honza

Re: [PATCH 2/4] Implement N disk counters for single value and indirect call counters.

[Martin Liška]

On 6/7/19 3:58 PM, Jan Hubicka wrote:
>>
>> Because I removed hist->hvalue.counters[2] where we stored total number of executions.
>> It's back again so that _atomic suffix version makes sense again.
> 
> OK and we do not care about race conditions on the other two counters?

We do care, but one can't probably implement that without a locking of the critical section:

   117  static inline void
   118  __gcov_one_value_profiler_body (gcov_type *counters, gcov_type value,
   119                                  int use_atomic)
   120  {
   121    if (value == counters[1])
   122      counters[2]++;
   123    else if (counters[2] == 0)
   124      {
   125        counters[2] = 1;
   126        counters[1] = value;
   127      }
   128    else
   129      counters[2]--;

Here you operate with 2 values which can change during a parallel execution.

>>
>> After we discussed that I decided to live with all counters in memory. Reason is that
>> in write_one_data we would have to allocate temporary buffers and fill up them here:
>>
>>    304            tag = gcov_read_unsigned ();
>>    305            length = gcov_read_unsigned ();
>>    306            if (tag != GCOV_TAG_FOR_COUNTER (t_ix)
>>    307                || length != GCOV_TAG_COUNTER_LENGTH (ci_ptr->num))
>>    308              goto read_mismatch;
>>    309            (*merge) (ci_ptr->values, ci_ptr->num);
>>    310            ci_ptr++;
>>
>> It would be quite nasty and I would like to mitigate libgcov profile crashes.
> 
> Hmm, I see, if code was organized to directly write the output, it would
> be easier.  I guess it is not overly important.
> 
> Patch is OK

Thanks for the approval.

Martin

> Honza
> 

Fix build with inhibit_libc (was Re: [PATCH 2/4] Implement N disk counters for single value and indirect call counters.)

[Jakub Jelinek]

On Fri, Jun 07, 2019 at 11:10:59AM +0200, Martin Liška wrote:
> libgcc/ChangeLog:
> 
> 2019-06-04  Martin Liska  <mliska@suse.cz>
> 
> 	* Makefile.in: Add __gcov_one_value_profiler_v2,
> 	__gcov_one_value_profiler_v2_atomic and
> 	__gcov_indirect_call_profiler_v4.
> 	* libgcov-merge.c (__gcov_merge_single): Change
> 	function signature.

> diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
> index 702a69f8349..42416341b4d 100644
> --- a/libgcc/libgcov-merge.c
> +++ b/libgcc/libgcov-merge.c
> @@ -34,8 +34,9 @@ void __gcov_merge_add (gcov_type *counters  __attribute__ ((unused)),
>  #endif
>  
>  #ifdef L_gcov_merge_single
> -void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)),
> -                          unsigned n_counters __attribute__ ((unused))) {}
> +void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
> +{
> +}
>  #endif
>  
>  #else

This change broke build without target libc.

../../../../libgcc/libgcov-merge.c:37:6: error: conflicting types for ‘__gcov_merge_single’
   37 | void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
      |      ^~~~~~~~~~~~~~~~~~~
In file included from ../../../../libgcc/libgcov-merge.c:26:
../../../../libgcc/libgcov.h:263:13: note: previous declaration of ‘__gcov_merge_single’ was here
  263 | extern void __gcov_merge_single (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
      |             ^~~~~~~~~~~~~~~~~~~

It is unclear why it has been changed, when the callers haven't been
adjusted, nor the prototype.

So, I'd like to revert this hunk.  Tested with x86_64-linux -> nvptx-none
cross build, ok for trunk?

2019-06-10  Jakub Jelinek  <jakub@redhat.com>

	* libgcov-merge.c (__gcov_merge_single): Revert previous change.

--- libgcc/libgcov-merge.c.jj	2019-06-10 19:39:29.291363550 +0200
+++ libgcc/libgcov-merge.c	2019-06-10 19:58:36.731524778 +0200
@@ -34,9 +34,8 @@ void __gcov_merge_add (gcov_type *counte
 #endif
 
 #ifdef L_gcov_merge_single
-void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
-{
-}
+void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)),
+			  unsigned n_counters __attribute__ ((unused))) {}
 #endif
 
 #else


	Jakub

Re: Fix build with inhibit_libc (was Re: [PATCH 2/4] Implement N disk counters for single value and indirect call counters.)

[Jakub Jelinek]

On Mon, Jun 10, 2019 at 08:02:26PM +0200, Jakub Jelinek wrote:
> This change broke build without target libc.
> 
> ../../../../libgcc/libgcov-merge.c:37:6: error: conflicting types for ‘__gcov_merge_single’
>    37 | void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
>       |      ^~~~~~~~~~~~~~~~~~~
> In file included from ../../../../libgcc/libgcov-merge.c:26:
> ../../../../libgcc/libgcov.h:263:13: note: previous declaration of ‘__gcov_merge_single’ was here
>   263 | extern void __gcov_merge_single (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
>       |             ^~~~~~~~~~~~~~~~~~~
> 
> It is unclear why it has been changed, when the callers haven't been
> adjusted, nor the prototype.
> 
> So, I'd like to revert this hunk.  Tested with x86_64-linux -> nvptx-none
> cross build, ok for trunk?

Additionally successfully bootstrapped/regtested on x86_64-linux.

> 2019-06-10  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* libgcov-merge.c (__gcov_merge_single): Revert previous change.
> 
> --- libgcc/libgcov-merge.c.jj	2019-06-10 19:39:29.291363550 +0200
> +++ libgcc/libgcov-merge.c	2019-06-10 19:58:36.731524778 +0200
> @@ -34,9 +34,8 @@ void __gcov_merge_add (gcov_type *counte
>  #endif
>  
>  #ifdef L_gcov_merge_single
> -void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
> -{
> -}
> +void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)),
> +			  unsigned n_counters __attribute__ ((unused))) {}
>  #endif
>  
>  #else

	Jakub

Re: Fix build with inhibit_libc (was Re: [PATCH 2/4] Implement N disk counters for single value and indirect call counters.)

[Martin Liška]

On 6/10/19 11:53 PM, Jakub Jelinek wrote:
> On Mon, Jun 10, 2019 at 08:02:26PM +0200, Jakub Jelinek wrote:
>> This change broke build without target libc.
>>
>> ../../../../libgcc/libgcov-merge.c:37:6: error: conflicting types for ‘__gcov_merge_single’
>>    37 | void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
>>       |      ^~~~~~~~~~~~~~~~~~~
>> In file included from ../../../../libgcc/libgcov-merge.c:26:
>> ../../../../libgcc/libgcov.h:263:13: note: previous declaration of ‘__gcov_merge_single’ was here
>>   263 | extern void __gcov_merge_single (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
>>       |             ^~~~~~~~~~~~~~~~~~~
>>
>> It is unclear why it has been changed, when the callers haven't been
>> adjusted, nor the prototype.
>>
>> So, I'd like to revert this hunk.  Tested with x86_64-linux -> nvptx-none
>> cross build, ok for trunk?
> 
> Additionally successfully bootstrapped/regtested on x86_64-linux.

Thank you for the patch. It's correct, I really accidentally changed the signature.

Martin

> 
>> 2019-06-10  Jakub Jelinek  <jakub@redhat.com>
>>
>> 	* libgcov-merge.c (__gcov_merge_single): Revert previous change.
>>
>> --- libgcc/libgcov-merge.c.jj	2019-06-10 19:39:29.291363550 +0200
>> +++ libgcc/libgcov-merge.c	2019-06-10 19:58:36.731524778 +0200
>> @@ -34,9 +34,8 @@ void __gcov_merge_add (gcov_type *counte
>>  #endif
>>  
>>  #ifdef L_gcov_merge_single
>> -void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)))
>> -{
>> -}
>> +void __gcov_merge_single (gcov_type *counters  __attribute__ ((unused)),
>> +			  unsigned n_counters __attribute__ ((unused))) {}
>>  #endif
>>  
>>  #else
> 
> 	Jakub
>