diff mbox

Fix direction of polymorphic call predictor

Message ID 20170106161151.GA21169@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Jan. 6, 2017, 4:11 p.m. UTC 
Hi,
this is next patch to tweak call predictors.  It fixes bug in the direction of
predictor (like normal calls, polymorphic calls are usually not taken when
there is if guariding them) and feeds in correct data from Martin's table.

Bootstrapped/regtested x86_64-linux, comitted.
I will fix the indirect call predictor next after periodic testers pick up the
change.

Honza

Comments

Jakub Jelinek Jan. 6, 2017, 4:14 p.m. UTC | #1 
On Fri, Jan 06, 2017 at 05:11:51PM +0100, Jan Hubicka wrote:
> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 244166)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,10 @@
> +2017-01-01  Jan Hubicka  <hubicka@ucw.cz>
> +
> +	PR middle-end/77484
> +	* predict.def (PRED_POLYMORPHIC_CALL): Set to 58

Missing . at the end.  More importantly, you say 58 here, while

> --- predict.def	(revision 244166)
> +++ predict.def	(working copy)
> @@ -122,7 +122,7 @@ DEF_PREDICTOR (PRED_CALL, "call", HITRAT
>     less reliable for indirect calls and polymorphic calls.  For spec2k6
>     the predictio nis slightly in the direction of taking the call.  */
>  DEF_PREDICTOR (PRED_INDIR_CALL, "indirect call", HITRATE (51), 0)
> -DEF_PREDICTOR (PRED_POLYMORPHIC_CALL, "polymorphic call", HITRATE (58), 0)
> +DEF_PREDICTOR (PRED_POLYMORPHIC_CALL, "polymorphic call", HITRATE (59), 0)
>  
>  /* Recursive calls are usually not taken or the function will recurse
>     indefinitely.  */

you've actually changed it from 58 to 59.  So is the predict.def change
intent and ChangeLog just stale, something else?

	Jakub
Jan Hubicka Jan. 6, 2017, 4:16 p.m. UTC | #2 
> On Fri, Jan 06, 2017 at 05:11:51PM +0100, Jan Hubicka wrote:
> > Index: ChangeLog
> > ===================================================================
> > --- ChangeLog	(revision 244166)
> > +++ ChangeLog	(working copy)
> > @@ -1,3 +1,10 @@
> > +2017-01-01  Jan Hubicka  <hubicka@ucw.cz>
> > +
> > +	PR middle-end/77484
> > +	* predict.def (PRED_POLYMORPHIC_CALL): Set to 58
> 
> Missing . at the end.  More importantly, you say 58 here, while
> 
> > --- predict.def	(revision 244166)
> > +++ predict.def	(working copy)
> > @@ -122,7 +122,7 @@ DEF_PREDICTOR (PRED_CALL, "call", HITRAT
> >     less reliable for indirect calls and polymorphic calls.  For spec2k6
> >     the predictio nis slightly in the direction of taking the call.  */
> >  DEF_PREDICTOR (PRED_INDIR_CALL, "indirect call", HITRATE (51), 0)
> > -DEF_PREDICTOR (PRED_POLYMORPHIC_CALL, "polymorphic call", HITRATE (58), 0)
> > +DEF_PREDICTOR (PRED_POLYMORPHIC_CALL, "polymorphic call", HITRATE (59), 0)
> >  
> >  /* Recursive calls are usually not taken or the function will recurse
> >     indefinitely.  */
> 
> you've actually changed it from 58 to 59.  So is the predict.def change
> intent and ChangeLog just stale, something else?

Ah, sorry. I will update the changelog entry.  It probably does not really mater
if there is 58 or 59, but it is better to stay consistent with experimental data where
we can.

Honza
> 
> 	Jakub
diff mbox

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 244166)
+++ ChangeLog	(working copy)
@@ -1,3 +1,10 @@ 
+2017-01-01  Jan Hubicka  <hubicka@ucw.cz>
+
+	PR middle-end/77484
+	* predict.def (PRED_POLYMORPHIC_CALL): Set to 58
+	* predict.c (tree_estimate_probability_bb): Reverse direction of
+	polymorphic call predictor.
+
 2017-01-06  David Malcolm  <dmalcolm@redhat.com>
 
 	* passes.c (execute_one_pass): Split out pass-skipping logic
Index: predict.c
===================================================================
--- predict.c	(revision 244166)
+++ predict.c	(working copy)
@@ -2789,7 +2789,7 @@  tree_estimate_probability_bb (basic_bloc
 		  if (gimple_call_fndecl (stmt))
 		    predict_edge_def (e, PRED_CALL, NOT_TAKEN);
 		  else if (virtual_method_call_p (gimple_call_fn (stmt)))
-		    predict_edge_def (e, PRED_POLYMORPHIC_CALL, TAKEN);
+		    predict_edge_def (e, PRED_POLYMORPHIC_CALL, NOT_TAKEN);
 		  else
 		    predict_edge_def (e, PRED_INDIR_CALL, TAKEN);
 		  break;
Index: predict.def
===================================================================
--- predict.def	(revision 244166)
+++ predict.def	(working copy)
@@ -122,7 +122,7 @@  DEF_PREDICTOR (PRED_CALL, "call", HITRAT
    less reliable for indirect calls and polymorphic calls.  For spec2k6
    the predictio nis slightly in the direction of taking the call.  */
 DEF_PREDICTOR (PRED_INDIR_CALL, "indirect call", HITRATE (51), 0)
-DEF_PREDICTOR (PRED_POLYMORPHIC_CALL, "polymorphic call", HITRATE (58), 0)
+DEF_PREDICTOR (PRED_POLYMORPHIC_CALL, "polymorphic call", HITRATE (59), 0)
 
 /* Recursive calls are usually not taken or the function will recurse
    indefinitely.  */