On Sat, Oct 24, 2015 at 7:31 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
> Hello,
> this patch adds OBJ_TYPE_REF that is the only code handled by ipa-icf-gimple
> but not bu operand_equal_p. I tried to make the following testcase:
>
> /* { dg-do compile } */
> /* { dg-options "-O2 -fdump-tree-pre" } */
> struct foo {
> virtual int test ();
> };
>
> int ret (struct foo *f, int v)
> {
> return v ? f->test() : f->test();
> }
> /* { dg-final { scan-tree-dump-not "if " "pre"} } */
>
> But we fail to tailmerge the code:
> <bb 2>:
> if (v_3(D) != 0)
> goto <bb 3>;
> else
> goto <bb 4>;
>
> <bb 3>:
> _6 = f_5(D)->_vptr.foo;
> _7 = *_6;
> _9 = OBJ_TYPE_REF(_7;(struct foo)f_5(D)->0) (f_5(D)); [tail call]
> goto <bb 5>;
>
> <bb 4>:
> _10 = f_5(D)->_vptr.foo;
> _11 = *_10;
> _13 = OBJ_TYPE_REF(_11;(struct foo)f_5(D)->0) (f_5(D)); [tail call]
>
> This is because the OBJ_TYPE_REF are not equal without unifing _7 with _11
> and _6 with _10 that we don't do, because we don't have head merging.
>
> We ought to be able to do so once tail merge is witched to ipa-icf-gimple
> and ipa-icf-gimple to operand_equal_p. Otherwsie i think we are hitting to
> limitation of tree-ssa-sccvn that may want similar matching somewhere.
>
> Once I have bit of time I will work on removing of OBJ_TYPE_REF wrappers
> produced by objective-C in gimplification and then all the
> virtual_method_call_p calls can go. The naive approach of simply not building
> them does not work, perhaps we want to have separate OBJC tree code, since
> they
> do not really represent virtual calls after all, but I got stuck on adding new
> tree codes to objc becuase handling of tree.def between C/C++/obj-C/obj-C++
> FEs is very convoluted.
>
> Bootstrapped/regtested x86_64-linx, OK?
>
> Honza
>
>
> * fold-const.c (operand_equal_p): Handle OBJ_TYPE_REF
>
> Index: fold-const.c
> ===================================================================
> --- fold-const.c (revision 229278)
> +++ fold-const.c (working copy)
> @@ -3085,6 +3082,27 @@ operand_equal_p (const_tree arg0, const_
> case DOT_PROD_EXPR:
> return OP_SAME (0) && OP_SAME (1) && OP_SAME (2);
>
> + case OBJ_TYPE_REF:
> + /* OBJ_TYPE_REF is just a wrapper around OBJ_TYP_REF_EXPR. */
> + if (!operand_equal_p (OBJ_TYPE_REF_EXPR (arg0),
> + OBJ_TYPE_REF_EXPR (arg1), flags))
> + return false;
> +
> + /* 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?
> + /* 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 ()
> + && 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?
> + && operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
> + OBJ_TYPE_REF_OBJECT (arg1),
> + flags));
> default:
> return 0;
> }
