On Fri, Nov 10, 2017 at 3:54 PM, Andrew MacLeod <amacl...@redhat.com> wrote:
> On 11/10/2017 09:03 AM, Richard Biener wrote:
>>
>> On Fri, Nov 10, 2017 at 2:49 PM, Andrew MacLeod <amacl...@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
>
>
