Re: [PATCH 2/3] Make early return predictor more precise.

[Martin Liška]

On 06/09/2017 04:08 PM, Jan Hubicka wrote:
>> gcc/ChangeLog:
>>
>> 2017-05-26  Martin Liska  <mli...@suse.cz>
>>
>>      PR tree-optimization/79489
>>      * gimplify.c (maybe_add_early_return_predict_stmt): New
>>      function.
>>      (gimplify_return_expr): Call the function.
>>      * predict.c (tree_estimate_probability_bb): Remove handling
>>      of early return.
>>      * predict.def: Update comment about early return predictor.
>>      * gimple-predict.h (is_gimple_predict): New function.
>>      * tree-inline.c (remap_gimple_stmt): Do not copy early return
>>      predictors during inlining.
>>      * predict.def: Change default value of early return to 66.
> 
> Thanks for working on this.
> Doing tail recursion early is quite useful.  Can't we make the pass to
> skip predict statements in analysis similar was as debug statements are
> skipped?

Hi.

Yes, this was easy to fix, skipping here helps.

>> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
>> index f3ec404ef09..3f3813cb062 100644
>> --- a/gcc/tree-inline.c
>> +++ b/gcc/tree-inline.c
>> @@ -1629,6 +1629,13 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id)
>>        gimple_seq_add_stmt (&stmts, copy);
>>        return stmts;
>>      }
>> +      if (is_gimple_predict (stmt))
>> +    {
>> +      /* Do not copy early return predictor that does not make sense
>> +         after inlining.  */
>> +      if (gimple_predict_predictor (stmt) == PRED_TREE_EARLY_RETURN)
>> +        return stmts;
>> +    }
> 
> I am also not quite sure about this one.  The code was still structured in a 
> way
> there was early return in the inlined function, so we may still assume that 
> the heuristic works for it?

Ok, you're right that we can preserve the predictor. However, let's consider 
following test-case:

static
int baz(int a)
{
  if (a == 1)
    return 1;

  return 0;
}

  
static
int bar(int a)
{
  if (a == 1)
    return baz(a);

  return 0;
}
  
static
int foo(int a)
{
  if (a == 1)
    return bar(a);

  return 12;
}

int main(int argc, char **argv)
{
  return foo(argc);
}

There after einline we have:

main (int argc, char * * argv)
{
  int D.1832;
  int _3;
  int _4;

  <bb 2> [100.00%]:
  if (argc_2(D) == 1)
    goto <bb 3>; [37.13%]
  else
    goto <bb 4>; [62.87%]

  <bb 3> [37.13%]:
  // predicted unlikely by early return (on trees) predictor.
  // predicted unlikely by early return (on trees) predictor.
  // predicted unlikely by early return (on trees) predictor.

  <bb 4> [100.00%]:
  # _3 = PHI <12(2), 1(3)>
  _5 = _3;
  _4 = _5;
  return _4;

}

I'm thinking what's the best place to merge all the predictor
statements?

Thanks,
Martin

> 
> Where did you found this case?
> Honza
>>  
>>        /* Create a new deep copy of the statement.  */
>>        copy = gimple_copy (stmt);
>> -- 
>> 2.13.0
>>