This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 06/09/2017 04:08 PM, Jan Hubicka wrote:
>> gcc/ChangeLog:
>>
>> 2017-05-26  Martin Liska  <mliska@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
>>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]