Types of operands in a gimple equality operation
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) (*) (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
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) (*) (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 (*) (pthread_key_t *, void (*) (void *)) Type op2 : 0x7fa299dca738 -> void * const Andrew
Re: Types of operands in a gimple equality operation
On Fri, Nov 10, 2017 at 2:49 PM, Andrew MacLeod 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) (*) (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
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) (*) (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
On 11/10/2017 09:03 AM, Richard Biener wrote: On Fri, Nov 10, 2017 at 2:49 PM, Andrew MacLeod 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) (*) (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