2011/5/14 Eric Botcazou <ebotca...@adacore.com>:
>> Those issues should be fixed by the attached patch, which relaxes
>> strictness of logical operations in tree-cfg.c file.
>
> Thanks.
>
>> 2011-05-14 Kai Tietz
>>
>> * tree-cfg.c (verify_gimple_assign_unary): Don't enforce
>> boolean_type_node
>> compatible lhs/rhs types for logical expression.
>> (verify_gimple_assign_binary): Likewise.
>
> - /* We allow only boolean typed or compatible argument and result. */
> - if (!useless_type_conversion_p (boolean_type_node, rhs1_type)
> - || !useless_type_conversion_p (boolean_type_node, rhs2_type)
> - || !useless_type_conversion_p (boolean_type_node, lhs_type))
> + /* The gimplify code ensures that just boolean_type_node types
> + are present, but ADA and FORTRAN using artificial gimple generation
> + code which is producing non-gimplify compatible trees. So we need
> + to allow here any kind of integral typed argument and result. */
> + if (!INTEGRAL_TYPE_P (rhs1_type)
> + || !INTEGRAL_TYPE_P (rhs2_type)
> + || !INTEGRAL_TYPE_P (lhs_type))
>
> What does "artificial gimple generation code" mean exactly? The only thing
> different in Ada is the definition of boolean_type_node which isn't compatible
> with the C definition (its precision is 8 instead of 1). That's all.
Well, I mean by artificial here, that gimplification is done via
gimplify_expr API. As FE and ME have here different assumptions. The
ME uses internally most boolean_type_node and IMHO it should be the
variant used there. As this conversation to a single boolean_type
(with recast to result FE's boolean type on demand) has some
advantages on optimization passes. Additionally it simplifies logic
in passes on types. For example there are some expressions, which are
in general unexpected in ME as they are transformed in gimplification
(like TRUTH_ANDIF/ORIF_EXPR). By adding tree manual, you might cause
the same issue as for the logical-expression showing up now.
> For Ada, you can just do:
>
> Index: tree-cfg.c
> ===================================================================
> --- tree-cfg.c (revision 173756)
> +++ tree-cfg.c (working copy)
> @@ -3350,7 +3350,7 @@ verify_gimple_assign_unary (gimple stmt)
> return false;
>
> case TRUTH_NOT_EXPR:
> - if (!useless_type_conversion_p (boolean_type_node, rhs1_type))
> + if (TREE_CODE (rhs1_type) != BOOLEAN_TYPE)
> {
> error ("invalid types in truth not");
> debug_generic_expr (lhs_type);
> @@ -3558,10 +3558,10 @@ do_pointer_plus_expr_check:
> case TRUTH_OR_EXPR:
> case TRUTH_XOR_EXPR:
> {
> - /* We allow only boolean typed or compatible argument and result. */
> - if (!useless_type_conversion_p (boolean_type_node, rhs1_type)
> - || !useless_type_conversion_p (boolean_type_node, rhs2_type)
> - || !useless_type_conversion_p (boolean_type_node, lhs_type))
> + /* We allow only boolean-typed argument and result. */
> + if (TREE_CODE (rhs1_type) != BOOLEAN_TYPE
> + || TREE_CODE (rhs2_type) != BOOLEAN_TYPE
> + || TREE_CODE (lhs_type) != BOOLEAN_TYPE)
> {
> error ("type mismatch in binary truth expression");
> debug_generic_expr (lhs_type);
>
> --
> Eric Botcazou
>
Well, this patch might be an alternative, but I see here potential
issues in such none-gimplified expressions for comparision and logical
not, which not necessariily have BOOLEAN_TYPE. See here the code for
fold_truth_not (and some other places) in fold-const. So I think, as
long as we have here external gimplication it is more save to check
just for integral-kind.
At the moment I am discussing with Tobias Burnus about wrapping
fold_build calls for logical operations (COND_EXPR (conditional
operand); TRUTH..., and possibly comparison too) , so that the
operation is done on boolean_type_node and the result gets re-casted
back to FE's logical boolean type. This would allow us to keep in
tree-cfg the check for one-bit operations for truth-expressions.
The general advantage here is that later passes (like reassoc - by
moving TRUTH-AND/OR/XOR to binary variant BIT_AND/OR/XOR) would show
effects and inner operands can be matched. Also by oiperating on
one-bit types here allows us to assume even that TRUTH_NOT_EXPR can be
transformed to BIT_NOT_EXPR (or XOR by 1).
But well, I would like to know Richard's opinion here.
Regards,
Kai
