Re: Handle OBJ_TYPE_REF in operand_equal_p

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Oct 26, 2015 at 3:34 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > +         /* Objective-C frontend produce ill formed OBJ_TYPE_REF which
>> > +            probably should be dropped before reaching middle-end.  */
>> > +         if (!virtual_method_call_p (arg0) || !virtual_method_call_p (arg1))
>> > +           return false;
>>
>> So what kind of brokeness is this?
>
> As the comment says, Objective-C uses OBJ_TYPE_REF for its own way. It also
> represent kind of polymorphic calls, but not in a way middle-end would understand
> because Objective-C types have no TYPE_BINFO and ODR information, so they can't
> be used for devirtualization in any way.
>
> /* Possibly rewrite a function CALL into an OBJ_TYPE_REF expression.  This
>    needs to be done if we are calling a function through a cast.  */
>
> tree
> objc_rewrite_function_call (tree function, tree first_param)
> {
>   if (TREE_CODE (function) == NOP_EXPR
>       && TREE_CODE (TREE_OPERAND (function, 0)) == ADDR_EXPR
>       && TREE_CODE (TREE_OPERAND (TREE_OPERAND (function, 0), 0))
>          == FUNCTION_DECL)
>     {
>       function = build3 (OBJ_TYPE_REF, TREE_TYPE (function),
>                          TREE_OPERAND (function, 0),
>                          first_param, size_zero_node);
>     }
>
>   return function;
> }
>
> This thing simply stays in GIMPLE and serves no purpose.  I tried to remove
> it at one point, but run into some regressions.  I plan to return to this.
> Have no idea how this was intended to work.
> Some bits was added by:
>  https://gcc.gnu.org/ml/gcc-patches/2005-04/txt00100.txt
> Code in build_objc_method_call seems predating changelogs.

I see.  But the above should be harmless unless you throw it onto the ODR
predicates?

>>
>> > +         /* Match the type information stored in the wrapper.  */
>> > +
>> > +         flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF);
>> > +         return (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg0))
>> > +                 == tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg1))
>>
>> Err, please use tree_int_cst_eq ()
>
> OK updated in my local copy and, I will update other places too, this was
> directly copied from ipa-icf-gimple.
>>
>> > +                 && types_same_for_odr (obj_type_ref_class (arg0),
>> > +                                        obj_type_ref_class (arg1))
>>
>> Why do you need this check?  The args of OBJ_TYPE_REF should be fully
>> specifying the object, no?
>
> This is the type of THIS pointer that is extracted from pointer type of
> OBJ_TYPE_REF.  Because we otherwise consider different pointers compatible,
> I think we need to check it here.

Hum, this is fold-const.c and thus GENERIC.  GENERIC doesn't consider pointer
types compatible in general.  So I think it is not needed.  As you say elsewhere
the caller is responsible for restricting typeof the trees it asks for equality.

>  It basically compares
>
> TREE_TYPE (TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (TREE_TYPE (obj_type_ref)))))
>
> I supose it can also do
>
> TYPE_METHOD_BASETYPE (TREE_TYPE (TREE_TYPE (obj_type_ref)))
>
> will look into that independently.  In any case we need to compare it.

Not on GENERIC IMHO and thus not in operand_equal_p.

Richard.

>>
>> > +                 && operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
>> > +                                     OBJ_TYPE_REF_OBJECT (arg1),
>> > +                                     flags));
>> >         default:
>> >           return 0;
>> >         }