[PATCH 3/9] operand_equal_p: add support for OBJ_TYPE_REF.

[Richard Biener]

On Thu, Aug 15, 2019 at 4:57 PM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> > On Tue, Aug 6, 2019 at 5:43 PM Martin Liska <mliska@suse.cz> wrote:
> > >
> > >
> > > gcc/ChangeLog:
> >
> > +       /* Virtual table call.  */
> > +       case OBJ_TYPE_REF:
> > +         {
> > +           if (!operand_equal_p (OBJ_TYPE_REF_EXPR (arg0),
> > +                                 OBJ_TYPE_REF_EXPR (arg1), flags))
> > +             return false;
> > +           if (virtual_method_call_p (arg0))
> > +             {
> > +               if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg0))
> > +                   != tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg1)))
> > +                 return false;
> > +               if (!types_same_for_odr (obj_type_ref_class (arg0),
> > +                                        obj_type_ref_class (arg1)))
> > +                 return false;
> > +               if (!operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
> > +                                     OBJ_TYPE_REF_OBJECT (arg1), flags))
> > +                 return false;
> >
> > this all gets deep into the devirt machinery, including looking at
> > ODR type hashes.  So I'm not sure if we really want to handle
> > it this "optimistic" in operand_equal_p and completely ignore
> > other operands when !virtual_method_call_p?  That is, why
> > not compare OBJ_TYPE_REF_TOKEN/OBJECT always at least?
>
> For !virtual_method_call_p we do not use OBJ_TYPE_REF at all yet obj-C frontend
> produce it.  I think we should remove them somewhere during gimplification.
> We can definitly turn "optimistic" to "pesimistic" and return false here.
>
> Otherwise the checks makes sense to me - it the tests above passes devirt
> machinery ought to give same results.
> >
> > Do we then have cases where the OBJ_TYPE_REF is actually
> > distinct according to the remaining check?
>
> I am not sure what you mean here?

When we compare OBJ_TYPE_REF_TOKEN and OBJ_TYPE_REF_OBJECT
and they are equal, are there cases where types_same_for_odr returns false?

I guess we went over this some time ago when talking about value-numbering
of them.  I realize the devirt machinery use the class type to get at the
virtual table but since we later expand simply OBJ_TPE_REF_EXPR _that_
already has the result of the load from the vitual table.

So - do we need to compare obj_type_ref_class at all?  The worst thing
that could happen is that devirt no longer can devirtualize after the merging
but it should never result in "wrong" devirtualization?  So, do we really
want to inhibit ICF of two equal functions that could be eventually
devirtualized both but into different direct calls?  Can't this sort of thing
happen anyway when you'd factor in IPA-CP and two identical functions
with indirect calls to a function argument but called with different constant
args?  And should IPA-CP then not consider cloning after ICF merging
(not sure how ordering works here).

Richard.

> Honza
> >
> > +             }
> >
> >
> > > 2019-07-24  Martin Liska  <mliska@suse.cz>
> > >
> > >         * fold-const.c (operand_equal_p): Support OBJ_TYPE_REF.
> > >         * tree.c (add_expr): Hash parts of OBJ_TYPE_REF.
> > > ---
> > >  gcc/fold-const.c | 21 +++++++++++++++++++++
> > >  gcc/tree.c       |  9 +++++++++
> > >  2 files changed, 30 insertions(+)
> > >