Strip only selected predictors after early tree passes (PR tree-optimization/85799).

Message ID 72e5339d-c62a-3390-acc0-e1fbbf7d0fa0@suse.cz
State New
Headers show
Series
  • Strip only selected predictors after early tree passes (PR tree-optimization/85799).
Related show

Commit Message

Martin Liška Aug. 9, 2018, 10:14 a.m.
Hi.

This patch adds an argument to the strip predictor pass. Early
version strips only selected hints that are not reliable when
inlining happens. The rest is striped in late tree passes.

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

Ready to be installed?
Martin

gcc/ChangeLog:

2018-08-09  Martin Liska  <mliska@suse.cz>

        PR tree-optimization/85799
	* passes.def: Add new pass_strip_predict_hints_early.
	* predict.c (make_pass_profile): Add
        pass_data_strip_predict_hints_early and
        new pass pass_strip_predict_hints_early.
	(strip_predictor_early): New function.
	(pass_strip_predict_hints::execute): Call strip_predict_hints.
	(strip_predict_hints): New.
	(class pass_strip_predict_hints_early): New.
	(make_pass_strip_predict_hints_early): Likewise.
	* predict.def: Fix documentation.
	* tree-pass.h (make_pass_strip_predict_hints_early): New.

gcc/testsuite/ChangeLog:

2018-08-09  Martin Liska  <mliska@suse.cz>

	* gcc.dg/pr85799.c: New test.
---
 gcc/passes.def                 |   2 +-
 gcc/predict.c                  | 138 ++++++++++++++++++++++++---------
 gcc/predict.def                |   2 +-
 gcc/testsuite/gcc.dg/pr85799.c |  19 +++++
 gcc/tree-pass.h                |   1 +
 5 files changed, 123 insertions(+), 39 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr85799.c

Comments

Jan Hubicka Aug. 9, 2018, 12:48 p.m. | #1
> Hi.
> 
> This patch adds an argument to the strip predictor pass. Early
> version strips only selected hints that are not reliable when
> inlining happens. The rest is striped in late tree passes.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-08-09  Martin Liska  <mliska@suse.cz>
> 
>         PR tree-optimization/85799
> 	* passes.def: Add new pass_strip_predict_hints_early.
> 	* predict.c (make_pass_profile): Add
>         pass_data_strip_predict_hints_early and
>         new pass pass_strip_predict_hints_early.
> 	(strip_predictor_early): New function.
> 	(pass_strip_predict_hints::execute): Call strip_predict_hints.
> 	(strip_predict_hints): New.
> 	(class pass_strip_predict_hints_early): New.
> 	(make_pass_strip_predict_hints_early): Likewise.
> 	* predict.def: Fix documentation.
> 	* tree-pass.h (make_pass_strip_predict_hints_early): New.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-08-09  Martin Liska  <mliska@suse.cz>

OK
>  	  NEXT_PASS (pass_split_functions);
> -	  NEXT_PASS (pass_strip_predict_hints);
> +	  NEXT_PASS (pass_strip_predict_hints_early);

Perhaps we want to add bool parameter
 NEXT_PASS (pass_strip_predict_hints, true /* early_p */);
> +/* Return true when PRED predictor should be removed after early
> +   tree passes.  */

Please add bit of reasoning why some stuff is left for late (i.e. we want it
to survive inlining so we don't get into this trap again)

OK with this change.

Honza

Patch

diff --git a/gcc/passes.def b/gcc/passes.def
index 2a8fbc2efbe..3b1c15db1e1 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -98,7 +98,7 @@  along with GCC; see the file COPYING3.  If not see
 	     early optimizations again.  It is thus good idea to do this
 	      late.  */
 	  NEXT_PASS (pass_split_functions);
-	  NEXT_PASS (pass_strip_predict_hints);
+	  NEXT_PASS (pass_strip_predict_hints_early);
       POP_INSERT_PASSES ()
       NEXT_PASS (pass_release_ssa_names);
       NEXT_PASS (pass_rebuild_cgraph_edges);
diff --git a/gcc/predict.c b/gcc/predict.c
index 96ae10f1319..a37c3845423 100644
--- a/gcc/predict.c
+++ b/gcc/predict.c
@@ -3878,44 +3878,27 @@  pass_profile::execute (function *fun)
 
 } // anon namespace
 
-gimple_opt_pass *
-make_pass_profile (gcc::context *ctxt)
-{
-  return new pass_profile (ctxt);
-}
-
-namespace {
-
-const pass_data pass_data_strip_predict_hints =
-{
-  GIMPLE_PASS, /* type */
-  "*strip_predict_hints", /* name */
-  OPTGROUP_NONE, /* optinfo_flags */
-  TV_BRANCH_PROB, /* tv_id */
-  PROP_cfg, /* properties_required */
-  0, /* properties_provided */
-  0, /* properties_destroyed */
-  0, /* todo_flags_start */
-  0, /* todo_flags_finish */
-};
+/* Return true when PRED predictor should be removed after early
+   tree passes.  */
 
-class pass_strip_predict_hints : public gimple_opt_pass
+static bool
+strip_predictor_early (enum br_predictor pred)
 {
-public:
-  pass_strip_predict_hints (gcc::context *ctxt)
-    : gimple_opt_pass (pass_data_strip_predict_hints, ctxt)
-  {}
-
-  /* opt_pass methods: */
-  opt_pass * clone () { return new pass_strip_predict_hints (m_ctxt); }
-  virtual unsigned int execute (function *);
-
-}; // class pass_strip_predict_hints
+  switch (pred)
+    {
+    case PRED_TREE_EARLY_RETURN:
+      return true;
+    default:
+      return false;
+    }
+}
 
 /* Get rid of all builtin_expect calls and GIMPLE_PREDICT statements
-   we no longer need.  */
+   we no longer need.  EARLY is set to true when called from early
+   optimizations.  */
+
 unsigned int
-pass_strip_predict_hints::execute (function *fun)
+strip_predict_hints (function *fun, bool early)
 {
   basic_block bb;
   gimple *ass_stmt;
@@ -3931,15 +3914,20 @@  pass_strip_predict_hints::execute (function *fun)
 
 	  if (gimple_code (stmt) == GIMPLE_PREDICT)
 	    {
-	      gsi_remove (&bi, true);
-	      changed = true;
-	      continue;
+	      if (!early
+		  || strip_predictor_early (gimple_predict_predictor (stmt)))
+		{
+		  gsi_remove (&bi, true);
+		  changed = true;
+		  continue;
+		}
 	    }
 	  else if (is_gimple_call (stmt))
 	    {
 	      tree fndecl = gimple_call_fndecl (stmt);
 
-	      if ((fndecl
+	      if ((!early
+		   && fndecl
 		   && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
 		   && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_EXPECT
 		   && gimple_call_num_args (stmt) == 2)
@@ -3967,6 +3955,43 @@  pass_strip_predict_hints::execute (function *fun)
   return changed ? TODO_cleanup_cfg : 0;
 }
 
+gimple_opt_pass *
+make_pass_profile (gcc::context *ctxt)
+{
+  return new pass_profile (ctxt);
+}
+
+namespace {
+
+const pass_data pass_data_strip_predict_hints =
+{
+  GIMPLE_PASS, /* type */
+  "*strip_predict_hints", /* name */
+  OPTGROUP_NONE, /* optinfo_flags */
+  TV_BRANCH_PROB, /* tv_id */
+  PROP_cfg, /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  0, /* todo_flags_finish */
+};
+
+class pass_strip_predict_hints : public gimple_opt_pass
+{
+public:
+  pass_strip_predict_hints (gcc::context *ctxt)
+    : gimple_opt_pass (pass_data_strip_predict_hints, ctxt)
+  {}
+
+  /* opt_pass methods: */
+  opt_pass * clone () { return new pass_strip_predict_hints (m_ctxt); }
+  virtual unsigned int execute (function *fun)
+  {
+    return strip_predict_hints (fun, false);
+  }
+
+}; // class pass_strip_predict_hints
+
 } // anon namespace
 
 gimple_opt_pass *
@@ -3975,6 +4000,45 @@  make_pass_strip_predict_hints (gcc::context *ctxt)
   return new pass_strip_predict_hints (ctxt);
 }
 
+namespace {
+
+const pass_data pass_data_strip_predict_hints_early =
+{
+  GIMPLE_PASS, /* type */
+  "*estrip_predict_hints", /* name */
+  OPTGROUP_NONE, /* optinfo_flags */
+  TV_BRANCH_PROB, /* tv_id */
+  PROP_cfg, /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  0, /* todo_flags_finish */
+};
+
+class pass_strip_predict_hints_early : public gimple_opt_pass
+{
+public:
+  pass_strip_predict_hints_early (gcc::context *ctxt)
+    : gimple_opt_pass (pass_data_strip_predict_hints, ctxt)
+  {}
+
+  /* opt_pass methods: */
+  opt_pass * clone () { return new pass_strip_predict_hints_early (m_ctxt); }
+  virtual unsigned int execute (function *fun)
+  {
+    return strip_predict_hints (fun, true);
+  }
+
+}; // class pass_strip_predict_hints_early
+
+} // anon namespace
+
+gimple_opt_pass *
+make_pass_strip_predict_hints_early (gcc::context *ctxt)
+{
+  return new pass_strip_predict_hints_early (ctxt);
+}
+
 /* Rebuild function frequencies.  Passes are in general expected to
    maintain profile by hand, however in some cases this is not possible:
    for example when inlining several functions with loops freuqencies might run
diff --git a/gcc/predict.def b/gcc/predict.def
index 76e6590cc96..c0709aa6473 100644
--- a/gcc/predict.def
+++ b/gcc/predict.def
@@ -22,7 +22,7 @@  along with GCC; see the file COPYING3.  If not see
      DEF_PREDICTOR (ENUM, NAME, HITRATE)
 
    This macro will be called once for each predictor.  The ENUM will
-   be of type `enum predictor', and will enumerate all supported
+   be of type `enum br_predictor', and will enumerate all supported
    predictors.  The order of DEF_PREDICTOR calls is important, as
    in the first match combining heuristics, the predictor appearing
    first in this file will win.
diff --git a/gcc/testsuite/gcc.dg/pr85799.c b/gcc/testsuite/gcc.dg/pr85799.c
new file mode 100644
index 00000000000..0e937857e29
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr85799.c
@@ -0,0 +1,19 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+void unlikely();
+void likely();
+
+inline int expect_false(int b) {
+    return __builtin_expect(b, 0);
+}
+
+void inline_func_hint(int b) {
+    if (expect_false(b)) {
+        unlikely();
+    } else {
+        likely();
+    }
+}
+
+/* { dg-final { scan-tree-dump "_builtin_expect heuristics of edge" "profile_estimate"} } */
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index af15adc8e0c..6d738f27997 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -405,6 +405,7 @@  extern gimple_opt_pass *make_pass_pre (gcc::context *ctxt);
 extern unsigned int tail_merge_optimize (unsigned int);
 extern gimple_opt_pass *make_pass_profile (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_strip_predict_hints (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_strip_predict_hints_early (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_lower_complex_O0 (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_lower_complex (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_lower_switch (gcc::context *ctxt);