On 01/11/2018 11:39 AM, Jan Hubicka wrote:
>> 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.
Yes it probably was. However according to numbers I have it does not influence
the jumpy
benchmarks ;)
>
> 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.
Sure, let's make then PROB_EVEN.
>
> 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?
Yes, there's a huge difference in between CPU 2006 and 2017. Former has 63% w/
dominant edges,
and later one only 11%. It's caused by these 2 benchmarks with a high coverage:
500.perlbench_r: regexec.c.065i.profile:
negative return heuristics of edge 1368->1370: 2.0% exec 2477714850 hit
2429863555 (98.1%)
and 523.xalancbmk_r:
build/build_peak_gcc7-m64.0000/NameDatatypeValidator.cpp.065i.profile:
negative return heuristics of edge 3->4: 2.0% exec 1221735072 hit 1221522453
(100.0%)
Ideas what to do with the predictor for GCC 8 release?
Martin
>
> 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
>>
>
