> These predictors are in my opinion not reliable and thus I decided to remove
> them:
>
> 1) PRED_NEGATIVE_RETURN: probability is ~51%
> 2) PRED_RECURSIVE_CALL: there are 2 dominant edges that influence value to
> 63%;
> w/o these edges it goes down to 52%
> 3) PRED_POLYMORPHIC_CALL: having very low coverage, probability is ~51%
> 4) PRED_INDIR_CALL: likewise
>
> Question here is whether we want to remove them, or to predict them with a
> 'ignored'
> flag? Doing that, we can measure statistics of the predictor in the future?
I believe that recursive call was introudced to help exchange2 benchmark. I
think it does
make sense globally because function simply can not contain hot self recursive
call and thus
I would not care about low benchmark coverage.
For polymorphic/indir call I think they are going to grow in importance in
future
(especialy first one) and thus I would like to keep them tracked. If you simply
set probablility to PROB_EVEN, they won't affect branch prediction outcome and
we still will get data on how common they are.
For PRED_NAGATIVE_RETURN, can you take a look why it changed from 98 to 51%?
The idea is that negative values are often used to report error codes and that
seems
reasonable. Perhaps it can be made more specific so it remains working ofr
spec2k16?
Honza
>
> Martin
> >From afbc86cb72eab37bcf6325954d0bf306b301f76e Mon Sep 17 00:00:00 2001
> From: marxin <mli...@suse.cz>
> Date: Thu, 28 Dec 2017 10:23:48 +0100
> Subject: [PATCH 4/5] Remove predictors that are unrealiable.
>
> gcc/ChangeLog:
>
> 2017-12-28 Martin Liska <mli...@suse.cz>
>
> * predict.c (return_prediction): Do not predict
> PRED_NEGATIVE_RETURN.
> (tree_bb_level_predictions): Do not predict PRED_RECURSIVE_CALL.
> (tree_estimate_probability_bb): Do not predict
> PRED_POLYMORPHIC_CALL and PRED_INDIR_CALL.
> * predict.def (PRED_INDIR_CALL): Remove unused predictors.
> (PRED_POLYMORPHIC_CALL): Likewise.
> (PRED_RECURSIVE_CALL): Likewise.
> (PRED_NEGATIVE_RETURN): Likewise.
> ---
> gcc/predict.c | 17 ++---------------
> gcc/predict.def | 13 -------------
> 2 files changed, 2 insertions(+), 28 deletions(-)
>
> diff --git a/gcc/predict.c b/gcc/predict.c
> index 51fd14205c2..f53724792e9 100644
> --- a/gcc/predict.c
> +++ b/gcc/predict.c
> @@ -2632,14 +2632,6 @@ return_prediction (tree val, enum prediction
> *prediction)
> }
> else if (INTEGRAL_TYPE_P (TREE_TYPE (val)))
> {
> - /* Negative return values are often used to indicate
> - errors. */
> - if (TREE_CODE (val) == INTEGER_CST
> - && tree_int_cst_sgn (val) < 0)
> - {
> - *prediction = NOT_TAKEN;
> - return PRED_NEGATIVE_RETURN;
> - }
> /* Constant return values seems to be commonly taken.
> Zero/one often represent booleans so exclude them from the
> heuristics. */
> @@ -2820,9 +2812,6 @@ tree_bb_level_predictions (void)
> DECL_ATTRIBUTES (decl)))
> predict_paths_leading_to (bb, PRED_COLD_FUNCTION,
> NOT_TAKEN);
> - if (decl && recursive_call_p (current_function_decl, decl))
> - predict_paths_leading_to (bb, PRED_RECURSIVE_CALL,
> - NOT_TAKEN);
> }
> else if (gimple_code (stmt) == GIMPLE_PREDICT)
> {
> @@ -2880,12 +2869,10 @@ tree_estimate_probability_bb (basic_block bb, bool
> local_only)
> something exceptional. */
> && gimple_has_side_effects (stmt))
> {
> + /* Consider just normal function calls, skip indirect and
> + polymorphic calls as these tend to be unreliable. */
> 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, NOT_TAKEN);
> - else
> - predict_edge_def (e, PRED_INDIR_CALL, TAKEN);
> break;
> }
> }
> diff --git a/gcc/predict.def b/gcc/predict.def
> index fe72080d5bd..7291650d237 100644
> --- a/gcc/predict.def
> +++ b/gcc/predict.def
> @@ -118,16 +118,6 @@ DEF_PREDICTOR (PRED_TREE_FPOPCODE, "fp_opcode (on
> trees)", HITRATE (90), 0)
> /* Branch guarding call is probably taken. */
> DEF_PREDICTOR (PRED_CALL, "call", HITRATE (67), 0)
>
> -/* PRED_CALL is not very reliable predictor and it turns out to be even
> - 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 (86), 0)
> -DEF_PREDICTOR (PRED_POLYMORPHIC_CALL, "polymorphic call", HITRATE (59), 0)
> -
> -/* Recursive calls are usually not taken or the function will recurse
> - indefinitely. */
> -DEF_PREDICTOR (PRED_RECURSIVE_CALL, "recursive call", HITRATE (75), 0)
> -
> /* Branch causing function to terminate is probably not taken. */
> DEF_PREDICTOR (PRED_TREE_EARLY_RETURN, "early return (on trees)", HITRATE
> (66),
> 0)
> @@ -138,9 +128,6 @@ DEF_PREDICTOR (PRED_GOTO, "goto", HITRATE (66), 0)
> /* Branch ending with return constant is probably not taken. */
> DEF_PREDICTOR (PRED_CONST_RETURN, "const return", HITRATE (65), 0)
>
> -/* Branch ending with return negative constant is probably not taken. */
> -DEF_PREDICTOR (PRED_NEGATIVE_RETURN, "negative return", HITRATE (98), 0)
> -
> /* Branch ending with return; is probably not taken */
> DEF_PREDICTOR (PRED_NULL_RETURN, "null return", HITRATE (71), 0)
>
> --
> 2.14.3
>
