Types of operands in a gimple equality operation

Types of operands in a gimple equality operation

[Andrew MacLeod]

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?
Andrew

Re: Types of operands in a gimple equality operation

[Andrew MacLeod]

On 11/10/2017 08:49 AM, Andrew MacLeod wrote:
>
> 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?
> Andrew

Interesting, in a scratch build I do see it in other places, and its 
always with a (void *)0.   especially in the __gcov stuff.

Maybe we special case this, or pointers in general, for some reason?

Makefile:915: recipe for target '_gcov_flush.o' failed
make[5]: *** [_gcov_flush.o] Error 1
make[5]: *** Waiting for unfinished jobs....

./gthr-default.h: In function ‘__gthread_active_p’:
./gthr-default.h:251:31: internal compiler error: in 
gimple_build_assign_1, at gimple.c:459
    return __gthread_active_ptr != 0;
           ~~~~~~~~~~~~~~~~~~~~~^~~~

  Incompatible types (in)equality check!
__gthrw___pthread_key_create != 0B
Type op1 : 0x7fa299dca5e8 -> int (*<T8e7>) (pthread_key_t *, void 
(*<Te2>) (void *))
Type op2 : 0x7fa299dca738 -> void * const

Andrew

Re: Types of operands in a gimple equality operation

[Richard Biener]

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).

Richard.


> Andrew

Re: Types of operands in a gimple equality operation

[Jakub Jelinek]

On Fri, Nov 10, 2017 at 08:59:41AM -0500, Andrew MacLeod wrote:
> On 11/10/2017 08:49 AM, Andrew MacLeod wrote:
> > 
> > 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?
> > Andrew
> 
> Interesting, in a scratch build I do see it in other places, and its always
> with a (void *)0.   especially in the __gcov stuff.
> 
> Maybe we special case this, or pointers in general, for some reason?

Please see verify_gimple_comparison, void pointers are special case and
valid.

	Jakub

Re: 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

Re: Types of operands in a gimple equality operation

[Richard Biener]

On Fri, Nov 10, 2017 at 3:54 PM, Andrew MacLeod <amacleod@redhat.com> wrote:
> 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.

I'm quite sure we could relax the above now given we have gimple_call_fntype
to preserve the original function type.  But I don't remember what broke and
exactly why on those pesky fn descriptor targets.  These days I only know
about IA64.

Richard.

> 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
>
>

Re: Types of operands in a gimple equality operation

[Jeff Law]

On 11/13/2017 01:30 AM, Richard Biener wrote:

>> 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.
> 
> I'm quite sure we could relax the above now given we have gimple_call_fntype
> to preserve the original function type.  But I don't remember what broke and
> exactly why on those pesky fn descriptor targets.  These days I only know
> about IA64.
It could well have been a PA-ism at the time.  I can recall looking at
some of this stuff, but none of the details.

jeff

Re: Types of operands in a gimple equality operation

[David Edelsohn]

On Mon, Nov 13, 2017 at 9:51 AM, Jeff Law <law@redhat.com> wrote:
> On 11/13/2017 01:30 AM, Richard Biener wrote:
>
>>> 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.
>>
>> I'm quite sure we could relax the above now given we have gimple_call_fntype
>> to preserve the original function type.  But I don't remember what broke and
>> exactly why on those pesky fn descriptor targets.  These days I only know
>> about IA64.
> It could well have been a PA-ism at the time.  I can recall looking at
> some of this stuff, but none of the details.

AIX and PPC64 BE Linux also use function descriptors.

- David