This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC][PATCH] Clean up of histogram allocation and release.


Hi.

Attempt of the patch is to remove all histograms right after
"profile_estimate" pass. Then nobody should use them. That
will simplify code we'll not need verification and currently
we leaked some histograms till the end of compilation.

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

Ready to be installed?
Martin

gcc/ChangeLog:

2018-07-31  Martin Liska  <mliska@suse.cz>

	* auto-profile.c (afdo_indirect_call): Do not remove
        histograms.
	* cfgexpand.c (pass_expand::execute): Likewise.
	* cgraph.c (release_function_body): Likewise.
	* gimple-iterator.c (gsi_replace): Likewise.
	(gsi_remove): Likewise.
	* ipa-profile.c (ipa_profile_generate_summary):
        Release all histograms.
	* tree-cfg.c (verify_gimple_in_cfg): Do not
        verify histograms.
	(move_block_to_fn): Do not remove histograms.
	* value-prof.c (gimple_remove_histogram_value):
        Remove.
	(gimple_remove_stmt_histograms): Likewise.
	(visit_hist): Likewise.
	(verify_histograms): Likewise.
	(gimple_divmod_fixed_value_transform): Do not
        remove histograms.
	(gimple_mod_pow2_value_transform): Likewise.
	(gimple_mod_subtract_transform): Likewise.
	(gimple_ic_transform): Likewise.
	(gimple_stringops_transform): Likewise.
	(stringop_block_profile): Likewise.
	(gimple_find_values_to_profile): Do not produce
        extra dump file output.
	* value-prof.h: Remove declarations.

gcc/testsuite/ChangeLog:

2018-07-31  Martin Liska  <mliska@suse.cz>

	* gcc.dg/tree-prof/val-prof-6.c: Do not scan histograms
        in optimized dump, it's already removed in that pass.
---
 gcc/auto-profile.c                          |   1 -
 gcc/cfgexpand.c                             |   1 -
 gcc/cgraph.c                                |   2 -
 gcc/gimple-iterator.c                       |   2 -
 gcc/ipa-profile.c                           |   6 +-
 gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c |   6 +-
 gcc/tree-cfg.c                              |   2 -
 gcc/value-prof.c                            | 127 +-------------------
 gcc/value-prof.h                            |   3 -
 9 files changed, 11 insertions(+), 139 deletions(-)


diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c
index 197fa10e08c..c3b5083f034 100644
--- a/gcc/auto-profile.c
+++ b/gcc/auto-profile.c
@@ -1063,7 +1063,6 @@ afdo_indirect_call (gimple_stmt_iterator *gsi, const icall_target_map &map,
       = indirect_edge->make_speculative (direct_call,
 					 profile_count::uninitialized ());
   new_edge->redirect_call_stmt_to_callee ();
-  gimple_remove_histogram_value (cfun, stmt, hist);
   inline_call (new_edge, true, NULL, NULL, false);
 }
 
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index d6e3c382085..1dda8e29f99 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -6397,7 +6397,6 @@ pass_expand::execute (function *fun)
   /* Expansion is used by optimization passes too, set maybe_hot_insn_p
      conservatively to true until they are all profile aware.  */
   delete lab_rtx_for_bb;
-  free_histograms (fun);
 
   construct_exit_block ();
   insn_locations_finalize ();
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index d19f1aacab8..c88029c4631 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1728,8 +1728,6 @@ release_function_body (tree decl)
 	  clear_edges (fn);
 	  fn->cfg = NULL;
 	}
-      if (fn->value_histograms)
-	free_histograms (fn);
       gimple_set_body (decl, NULL);
       /* Struct function hangs a lot of data that would leak if we didn't
          removed all pointers to it.   */
diff --git a/gcc/gimple-iterator.c b/gcc/gimple-iterator.c
index c0131f3654c..61011f37360 100644
--- a/gcc/gimple-iterator.c
+++ b/gcc/gimple-iterator.c
@@ -445,7 +445,6 @@ gsi_replace (gimple_stmt_iterator *gsi, gimple *stmt, bool update_eh_info)
 
   /* Free all the data flow information for ORIG_STMT.  */
   gimple_set_bb (orig_stmt, NULL);
-  gimple_remove_stmt_histograms (cfun, orig_stmt);
   delink_stmt_imm_use (orig_stmt);
 
   gsi_set_stmt (gsi, stmt);
@@ -573,7 +572,6 @@ gsi_remove (gimple_stmt_iterator *i, bool remove_permanently)
 	   close.  */
 	cfun->debug_marker_count--;
       require_eh_edge_purge = remove_stmt_from_eh_lp (stmt);
-      gimple_remove_stmt_histograms (cfun, stmt);
     }
 
   /* Update the iterator and re-wire the links in I->SEQ.  */
diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c
index f921d1bb6f4..4aa73917274 100644
--- a/gcc/ipa-profile.c
+++ b/gcc/ipa-profile.c
@@ -216,8 +216,6 @@ ipa_profile_generate_summary (void)
 			      e->indirect_info->common_target_probability = REG_BR_PROB_BASE;
 			    }
 			}
-		      gimple_remove_histogram_value (DECL_STRUCT_FUNCTION (node->decl),
-						      stmt, h);
 		    }
 		}
 	      time += estimate_num_insns (stmt, &eni_time_weights);
@@ -228,6 +226,10 @@ ipa_profile_generate_summary (void)
 			       time, size);
 	}
   histogram.qsort (cmp_counts);
+
+  /* Remove all histograms for all functions.  */
+  FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
+    free_histograms (DECL_STRUCT_FUNCTION (node->decl));
 }
 
 /* Serialize the ipa info for lto.  */
diff --git a/gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c b/gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c
index 597efa6ad70..db87af9f696 100644
--- a/gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c
+++ b/gcc/testsuite/gcc.dg/tree-prof/val-prof-6.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fdump-ipa-profile-details" } */
 char a[1000];
 char b[1000];
 int size=1000;
@@ -16,5 +16,5 @@ main()
   return 0;
 }
 /* autofdo does not do value profiling so far */
-/* { dg-final-use-not-autofdo { scan-tree-dump "Average value sum:499500" "optimized"} } */
-/* { dg-final-use-not-autofdo { scan-tree-dump "IOR value" "optimized"} } */
+/* { dg-final-use-not-autofdo { scan-tree-dump "Average value sum:499500" "profile"} } */
+/* { dg-final-use-not-autofdo { scan-tree-dump "IOR value" "profile"} } */
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 14d66b7a728..da6fd558437 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -5424,7 +5424,6 @@ verify_gimple_in_cfg (struct function *fn, bool verify_nothrow)
   if (err || eh_error_found)
     internal_error ("verify_gimple failed");
 
-  verify_histograms ();
   timevar_pop (TV_TREE_STMT_VERIFY);
 }
 
@@ -7106,7 +7105,6 @@ move_block_to_fn (struct function *dest_cfun, basic_block bb,
       remove_stmt_from_eh_lp_fn (cfun, stmt);
 
       gimple_duplicate_stmt_histograms (dest_cfun, stmt, cfun, stmt);
-      gimple_remove_stmt_histograms (cfun, stmt);
 
       /* We cannot leave any operands allocated from the operand caches of
 	 the current function.  */
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index a7c4be7a7d8..416eea18ae1 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -183,29 +183,6 @@ gimple_add_histogram_value (struct function *fun, gimple *stmt,
   hist->fun = fun;
 }
 
-/* Remove histogram HIST from STMT's histogram list.  */
-
-void
-gimple_remove_histogram_value (struct function *fun, gimple *stmt,
-			       histogram_value hist)
-{
-  histogram_value hist2 = gimple_histogram_value (fun, stmt);
-  if (hist == hist2)
-    {
-      set_histogram_value (fun, stmt, hist->hvalue.next);
-    }
-  else
-    {
-      while (hist2->hvalue.next != hist)
-	hist2 = hist2->hvalue.next;
-      hist2->hvalue.next = hist->hvalue.next;
-    }
-  free (hist->hvalue.counters);
-  if (flag_checking)
-    memset (hist, 0xab, sizeof (*hist));
-  free (hist);
-}
-
 /* Lookup histogram of type TYPE in the STMT.  */
 
 histogram_value
@@ -446,16 +423,6 @@ dump_histograms_for_stmt (struct function *fun, FILE *dump_file, gimple *stmt)
     dump_histogram_value (dump_file, hist);
 }
 
-/* Remove all histograms associated with STMT.  */
-
-void
-gimple_remove_stmt_histograms (struct function *fun, gimple *stmt)
-{
-  histogram_value val;
-  while ((val = gimple_histogram_value (fun, stmt)) != NULL)
-    gimple_remove_histogram_value (fun, stmt, val);
-}
-
 /* Duplicate all histograms associates with OSTMT to STMT.  */
 
 void
@@ -492,66 +459,6 @@ gimple_move_stmt_histograms (struct function *fun, gimple *stmt, gimple *ostmt)
     }
 }
 
-static bool error_found = false;
-
-/* Helper function for verify_histograms.  For each histogram reachable via htab
-   walk verify that it was reached via statement walk.  */
-
-static int
-visit_hist (void **slot, void *data)
-{
-  hash_set<histogram_value> *visited = (hash_set<histogram_value> *) data;
-  histogram_value hist = *(histogram_value *) slot;
-
-  if (!visited->contains (hist)
-      && hist->type != HIST_TYPE_TIME_PROFILE)
-    {
-      error ("dead histogram");
-      dump_histogram_value (stderr, hist);
-      debug_gimple_stmt (hist->hvalue.stmt);
-      error_found = true;
-    }
-  return 1;
-}
-
-/* Verify sanity of the histograms.  */
-
-DEBUG_FUNCTION void
-verify_histograms (void)
-{
-  basic_block bb;
-  gimple_stmt_iterator gsi;
-  histogram_value hist;
-
-  error_found = false;
-  hash_set<histogram_value> visited_hists;
-  FOR_EACH_BB_FN (bb, cfun)
-    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-      {
-	gimple *stmt = gsi_stmt (gsi);
-
-	for (hist = gimple_histogram_value (cfun, stmt); hist;
-	     hist = hist->hvalue.next)
-	  {
-	    if (hist->hvalue.stmt != stmt)
-	      {
-		error ("Histogram value statement does not correspond to "
-		       "the statement it is associated with");
-		debug_gimple_stmt (stmt);
-		dump_histogram_value (stderr, hist);
-		error_found = true;
-	      }
-            visited_hists.add (hist);
-	  }
-      }
-  if (VALUE_HISTOGRAMS (cfun))
-    htab_traverse (VALUE_HISTOGRAMS (cfun), visit_hist, &visited_hists);
-  if (error_found)
-    internal_error ("verify_histograms failed");
-}
-
-/* Helper function for verify_histograms.  For each histogram reachable via htab
-   walk verify that it was reached via statement walk.  */
 
 static int
 free_hist (void **slot, void *data ATTRIBUTE_UNUSED)
@@ -787,7 +694,6 @@ gimple_divmod_fixed_value_transform (gimple_stmt_iterator *si)
   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
@@ -948,8 +854,6 @@ gimple_mod_pow2_value_transform (gimple_stmt_iterator *si)
   wrong_values = histogram->hvalue.counters[0];
   count = histogram->hvalue.counters[1];
 
-  gimple_remove_histogram_value (cfun, stmt, histogram);
-
   /* We require that we hit a power of 2 at least half of all evaluations.  */
   if (simple_cst_equal (gimple_assign_rhs2 (stmt), value) != 1
       || count < wrong_values
@@ -1126,10 +1030,7 @@ gimple_mod_subtract_transform (gimple_stmt_iterator *si)
 
   /* 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);
-      return false;
-    }
+    return false;
 
   if (flag_profile_correction && count1 + count2 > all)
       all = count1 + count2;
@@ -1149,7 +1050,6 @@ gimple_mod_subtract_transform (gimple_stmt_iterator *si)
       || optimize_bb_for_size_p (gimple_bb (stmt)))
     return false;
 
-  gimple_remove_histogram_value (cfun, stmt, histogram);
   if (dump_file)
     {
       fprintf (dump_file, "Mod subtract transformation on insn ");
@@ -1464,10 +1364,7 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
   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;
-    }
+    return false;
 
   if (4 * count <= 3 * all)
     return false;
@@ -1499,7 +1396,6 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
 	  fprintf (dump_file, " transformation skipped because of type mismatch");
 	  print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
 	}
-      gimple_remove_histogram_value (cfun, stmt, histogram);
       return false;
     }
 
@@ -1686,7 +1582,6 @@ gimple_stringops_transform (gimple_stmt_iterator *gsi)
   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
@@ -1763,10 +1658,7 @@ stringop_block_profile (gimple *stmt, unsigned int *expected_align,
   if (!histogram)
     *expected_size = -1;
   else if (!histogram->hvalue.counters[1])
-    {
-      *expected_size = -1;
-      gimple_remove_histogram_value (cfun, stmt, histogram);
-    }
+    *expected_size = -1;
   else
     {
       gcov_type size;
@@ -1778,7 +1670,6 @@ stringop_block_profile (gimple *stmt, unsigned int *expected_align,
       if (size > INT_MAX)
 	size = INT_MAX;
       *expected_size = size;
-      gimple_remove_histogram_value (cfun, stmt, histogram);
     }
 
   histogram = gimple_histogram_value_of_type (cfun, stmt, HIST_TYPE_IOR);
@@ -1786,10 +1677,7 @@ stringop_block_profile (gimple *stmt, unsigned int *expected_align,
   if (!histogram)
     *expected_align = 0;
   else if (!histogram->hvalue.counters[0])
-    {
-      gimple_remove_histogram_value (cfun, stmt, histogram);
-      *expected_align = 0;
-    }
+    *expected_align = 0;
   else
     {
       gcov_type count;
@@ -1801,7 +1689,6 @@ stringop_block_profile (gimple *stmt, unsigned int *expected_align,
 	     && (alignment <= UINT_MAX / 2 / BITS_PER_UNIT))
 	alignment <<= 1;
       *expected_align = alignment * BITS_PER_UNIT;
-      gimple_remove_histogram_value (cfun, stmt, histogram);
     }
 }
 
@@ -1995,11 +1882,5 @@ gimple_find_values_to_profile (histogram_values *values)
 	default:
 	  gcc_unreachable ();
 	}
-      if (dump_file && hist->hvalue.stmt != NULL)
-        {
-	  fprintf (dump_file, "Stmt ");
-          print_gimple_stmt (dump_file, hist->hvalue.stmt, 0, TDF_SLIM);
-	  dump_histogram_value (dump_file, hist);
-        }
     }
 }
diff --git a/gcc/value-prof.h b/gcc/value-prof.h
index d0b8cda181f..f644f47e230 100644
--- a/gcc/value-prof.h
+++ b/gcc/value-prof.h
@@ -82,12 +82,9 @@ histogram_value gimple_histogram_value_of_type (struct function *, gimple *,
 						enum hist_type);
 void gimple_add_histogram_value (struct function *, gimple *, histogram_value);
 void dump_histograms_for_stmt (struct function *, FILE *, gimple *);
-void gimple_remove_histogram_value (struct function *, gimple *, histogram_value);
-void gimple_remove_stmt_histograms (struct function *, gimple *);
 void gimple_duplicate_stmt_histograms (struct function *, gimple *,
 				       struct function *, gimple *);
 void gimple_move_stmt_histograms (struct function *, gimple *, gimple *);
-void verify_histograms (void);
 void free_histograms (function *);
 void stringop_block_profile (gimple *, unsigned int *, HOST_WIDE_INT *);
 gcall *gimple_ic (gcall *, struct cgraph_node *, profile_probability);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]