Re: [patch gimplifier]: Make sure TRUTH_NOT_EXPR has boolean_type_node type and argument

[Richard Guenther <richard.guenther@gmail.com>]

On Sat, May 14, 2011 at 6:14 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> 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.
>
> 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);

We can't use a test for BOOLEAN_TYPE as the middle-end considers
a INTEGER_TYPE with same precision/signedness as compatible
and thus may propagate a variable of INTEGER_TYPE there.  I don't
understand why promoting bools to boolean_type_node during
gimplification does not work (it might be slightly undesirable as it
might introduce conversions from one boolean type to another).

So, to relax the above check to allow any of the boolean types
we'd need a way to enumerate them.  Or make conversions to/from
BOOLEAN_TYPEs not useless.

Kai - your testing should have catched the Ada testsuite fail, please
inverstigate why your setup didn't catch it.

Richard.