Hi Honza,
Jan Hubicka wrote:
But if you have something like
a=__builtin_expect (b?1:0,0)
and you produce
a=b?predict_expr not_taken, 0,0
...
if (a)
unlikely path
We need to check how it goes down to gimple.
It seems as if something doesn't work in that case – at least I do not
understand the failure in gfortran.dg/elemental_optional_args_5.f03.
With the patch one has in gfc_conv_procedure_call:
+ gfc_init_block (&err_block);
+ gfc_add_expr_to_block (&err_block,
+ build_predict_expr (PRED_ZERO, NOT_TAKEN));
+ gfc_add_expr_to_block (&err_block,
+ fold_convert (TREE_TYPE (parmse.expr),
+ null_pointer_node));
tmp = fold_build2_loc (input_location, EQ_EXPR, boolean_type_node,
descriptor_data,
fold_convert (TREE_TYPE (descriptor_data),
null_pointer_node));
parmse.expr
= fold_build3_loc (input_location, COND_EXPR,
+ TREE_TYPE (parmse.expr), tmp,
+ gfc_finish_block (&err_block), parmse.expr);
And for some reasons that will fail with -O0/-Os - it works with -O1 or
when one removes the PREDICT.
Any idea what goes wrong here? I thought predictions can only produce
inefficient code but not wrong code.
So in short if we want the predictions to work reliably, we need to be sure
we produce them in a way backend can reliably consume them.
I fully concur.
2) if there are the other cases (i.e. fortran language allows the failure
flag to be stored into user variable and handled by user later),
I will add internal use only argument to predict_expr and extend its
handling.
Again it won't be 100% reliable, since one conditional can then be handled
by multiple predict_exprs that leads to generally unsolvable problem on
how to combine probabilities.
But hopefully those cases will mostly be simple - i.e. user will have
if (failure_flag)
make horrible death in controlled way
statement somewhre after the allcation.
We are smart enough to work out simple variants like
if (failure_flag1 || failure_flag2)
make horrible death in controlled way
I think that's the case for, e.g.
ALLOCATE(variable, stat=status)
where gives status /= 0 in case an error occurs.
3) make sure the runtime library calls are correctly annotated with
noreturn/cold
flags that may help back end to work out the cases it failed otherwise.
I think that's already the case - and is the most important case.
+/* Branch leading to an overflow are extremely unlikely. */
+DEF_PREDICTOR (PRED_OVERFLOW, "overflow", PROB_VERY_LIKELY,
+ PRED_FLAG_FIRST_MATCH)
I would even gor for PROB_ALWAYS here.
Fine with me - I was only fearing that it would regard it as always and
remove the NOT_TAKEN path. But if it keeps it, using PROB_ALWAYS makes
sense.
I would go for a bit more dscriptive names, like "fortran alloc overflow"/"fortran
zero size alloc" etc.
Those names will hopefuly be more obvious for middle end developer like myself.
Okay. When the issue with elemental_optional_args_5.f03 is understood, I
will update the patch.
Tobias
