On Fri, Oct 7, 2011 at 1:37 PM, Eric Botcazou <ebotca...@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 >
