Types of operands in a gimple equality operation

[Andrew MacLeod]

On 11/10/2017 09:03 AM, Richard Biener wrote:
> On Fri, Nov 10, 2017 at 2:49 PM, Andrew MacLeod <amacleod@redhat.com> wrote:
>> Before I open a PR, I want to confirm my beliefs.
>>
>>
>> Is it not true that both operations of a gimple operation such as == or !=
>> must satisfy types_compatible_p (op1_type, op2_type)  ?  Even when one is a
>> constant?
>>
>> given :
>>
>>    _10 = _2 != 0
>>
>> so the generic node for the 0 needs to be a type compatible with _2?
>>
>> I ask because I tripped over a fortran test where that is not true. It is
>> comparing a function pointer of some sort with a (void *)0, and the
>> types_compatible_p check fails.
>>
>> I hacked the compiler to check when building a gimple assign to verify that
>> the types are compatible.  It succeeds and entire bootstrap cycle, which
>> leads me to believe my assertion is true. For some reason I thought there
>> was gimple verification that would catch things like this.. apparently not?
>>
>> Index: gimple.c
>> ===================================================================
>> *** gimple.c    (revision 254327)
>> --- gimple.c    (working copy)
>> *************** gimple_build_assign_1 (tree lhs, enum tr
>> *** 423,428 ****
>> --- 423,430 ----
>>        {
>>          gcc_assert (num_ops > 2);
>>          gimple_assign_set_rhs2 (p, op2);
>> +       if (subcode == EQ_EXPR || subcode == NE_EXPR)
>> +     gcc_assert (types_compatible_p (TREE_TYPE (op1), TREE_TYPE (op2)));
>>        }
>>
>>
>> and when I run it on this small program:
>>
>>    interface
>>      integer function foo ()
>>      end function
>>      integer function baz ()
>>      end function
>>    end interface
>>    procedure(foo), pointer :: ptr
>>    ptr => baz
>>    if (.not.associated (ptr, baz)) call abort
>> end
>>
>> I get a trap on this statement:
>>
>>   if (.not.associated (ptr, baz)) call abort
>>
>> internal compiler error: in gimple_build_assign_1, at gimple.c:443
>>
>> The IL is comparing
>>       ptr == 0B
>>
>> and I see:
>> Type op1 : 0x7fd8e312df18 -> integer(kind=4) (*<T561>) (void)
>> Type op2 : 0x7fd8e2fa10a8 -> void *
>>
>> These 2 types fail the types_compatible_p test.
>>
>> So is this a bug like I think it is?
> Always look at tree-cfg.c:verify_gimple_*
>
> Quoting:
>
> static bool
> verify_gimple_comparison (tree type, tree op0, tree op1, enum tree_code code)
> {
> ...
>    /* For comparisons we do not have the operations type as the
>       effective type the comparison is carried out in.  Instead
>       we require that either the first operand is trivially
>       convertible into the second, or the other way around.
>       Because we special-case pointers to void we allow
>       comparisons of pointers with the same mode as well.  */
>    if (!useless_type_conversion_p (op0_type, op1_type)
>        && !useless_type_conversion_p (op1_type, op0_type)
>        && (!POINTER_TYPE_P (op0_type)
>            || !POINTER_TYPE_P (op1_type)
>            || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>      {
>        error ("mismatching comparison operand types");
>        debug_generic_expr (op0_type);
>        debug_generic_expr (op1_type);
>        return true;
>      }
>
> this is exactly because we have that "wart"
>
> bool
> useless_type_conversion_p (tree outer_type, tree inner_type)
> {
>    /* Do the following before stripping toplevel qualifiers.  */
>    if (POINTER_TYPE_P (inner_type)
>        && POINTER_TYPE_P (outer_type))
>      {
> ...
>        /* Do not lose casts to function pointer types.  */
>        if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
>             || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
>            && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
>                 || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
>          return false;
>      }
>
> which is IIRC because of targets with function descriptors (details are lost
> on me, but I remember repeatedly trying to get rid of this special case).
>
Huh, that bites.  Im surprised we don't just make those places produce a 
cast, or just introduce an explicit cast of the (void *)0 during the 
expression building process.

Of course, I'm sure its not that simple :-P Nothing ever is.

OKeydoke.  I'll leave it as is an allow the wart to pass my code as well 
for the moment.

Thanks
Adnrew