Re: [patch] Fix PR lto/50492

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Oct 7, 2011 at 1:37 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Hm, but canonicalize_cond_expr_cond is supposed to produce a
>> tree that is suitable for a condition in a GIMPLE_COND, or a
>> COND_EXPR. ?So the issue is that it is used for a tcc_comparison
>> on a assignment RHS?
>
> It is called on (Ada_Boolean_Type) iftmp.xxx, which comes from the RHS of
>
> ?Ada_Boolean_Var = (Integer_Var != 0);
>
>> Wouldn't the "proper" fix then be to verify that the result from
>> forward_propagate_into_comparison_1 in forwprop is a suitable
>> replacement of the assign rhs in forward_propagate_into_comparison?
>
> Not clear whether it is the proper fix, as you explicitly pass the type in the
> call to forward_propagate_into_comparison_1 just above:
>
> ?tmp = forward_propagate_into_comparison_1 (stmt,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? gimple_assign_rhs_code (stmt),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TREE_TYPE
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (gimple_assign_lhs (stmt)),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? rhs1, rhs2);
>
> and then to combine_cond_expr_cond, so you would expect that both functions
> return a tree with the specified type. ?But this would probably work, yes.

Yeah, it uses the type to do

  t = fold_binary_loc (gimple_location (stmt), code, type, op0, op1);

and using the "original type" is surely the best idea for that.  It could
also simply use boolean_type_node nowadays, not sure if that
would cause more fails on the type checking that is needed in
forward_propagate_into_comparison.  I suppose at most for Ada,
so maybe you can check that.  If it doesn't make a difference
then stripping the type argument from the call chain and using
boolean_type_node in combine_cond_expr_cond would be a good
cleanup.

But the original suggested patch is ok if it bootstraps & tests if
you just want to go forward.

Thanks,
Richard.

> --
> Eric Botcazou
>