> >may lead to wrong code.
>
> Can you try generating a testcase?
> Because with equal vptr and voffset I can't see how that can happen unless
> some pass extracts information from the pointer types without sanity checking
> with the pointers and offsets.
I am not sure I can get a wrong code with current mainline, because for now you
only substitute for the lookup done for speculative devirt and if we wrongly
predict the thing to be __builtin_unreachable, we dispatch to usual virtual
call. Once you get movement on calls it will be easier to do.
OBJ_TYPE_REF is a wrapper around OBJ_TYPE_EXPR adding three extra parameters:
- OBJ_TYPE_REF_OBJECT
- OBJ_TYPE_REF_TOKEN
- obj_type_ref_class which is computed from TREE_TYPE (obj_type_ref) itself.
While two OBJ_TYPE_REFS with equivalent OBJ_TYPE_EXPR are kind of same
expressions, they are optimized differently (just as if they was in different
alias set). For that reason you need to match the type of obj_type_ref_class
because that one is not matched by usless_type_conversion (it is a pointer to
method of corresponding class type we are looking up)
The following testcase:
struct foo {virtual void bar(void) __attribute__ ((const));};
struct foobar {virtual void bar(void) __attribute__ ((const));};
void
dojob(void *ptr, int t)
{
if (t)
((struct foo*)ptr)->bar();
else
((struct foobar*)ptr)->bar();
}
produces
void dojob(void*, int) (void * ptr, int t)
{
int (*__vtbl_ptr_type) () * _5;
int (*__vtbl_ptr_type) () _6;
int (*__vtbl_ptr_type) () * _8;
int (*__vtbl_ptr_type) () _9;
<bb 2>:
if (t_2(D) != 0)
goto <bb 3>;
else
goto <bb 4>;
<bb 3>:
_5 = MEM[(struct foo *)ptr_4(D)]._vptr.foo;
_6 = *_5;
OBJ_TYPE_REF(_6;(struct foo)ptr_4(D)->0) (ptr_4(D));
goto <bb 5>;
<bb 4>:
_8 = MEM[(struct foobar *)ptr_4(D)]._vptr.foobar;
_9 = *_8;
OBJ_TYPE_REF(_9;(struct foobar)ptr_4(D)->0) (ptr_4(D));
<bb 5>:
return;
}
Now I would need to get some code movement done to get _5 and _6
moved and unified with _8 and _9 that we currently don't do.
Still would feel safer if the equivalence predicate also checked
that the type is the same.
> >Or do you just substitute the operands of OBJ_TYPE_REF?
>
> No, I value number them. But yes, the type issue also crossed my mind.
> Meanwhile testing revealed that I need to adjust gimple_expr_type to preserve
> the type of the obj-type-ref, otherwise the devirt machinery ICEs (receiving
> void *). That's also a reason we can't make obj-type-ref a ternary RHS.
Yep, type of OBJ_TYPE_REF matters...
>
> >> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> >>
> >> Note that this does not (yet) substitute OBJ_TYPE_REFs in calls
> >> with SSA names that have the same value - not sure if that would
> >> be desired generally (does the devirt machinery cope with that?).
> >
> >This should work fine.
>
> OK. So with that substituting the direct call later should work as well.
Great!
>
> Richard.
Honza
>
> >>
> >> Thanks,
> >> Richard.
> >>
> >> 2015-12-03 Richard Biener <[email protected]>
> >>
> >> PR tree-optimization/64812
> >> * tree-ssa-sccvn.c (vn_get_stmt_kind): Handle OBJ_TYPE_REF.
> >> (vn_nary_length_from_stmt): Likewise.
> >> (init_vn_nary_op_from_stmt): Likewise.
> >> * gimple-match-head.c (maybe_build_generic_op): Likewise.
> >> * gimple-pretty-print.c (dump_unary_rhs): Likewise.
> >>
> >> * g++.dg/tree-ssa/ssa-fre-1.C: New testcase.
> >>
> >> Index: gcc/tree-ssa-sccvn.c
> >> ===================================================================
> >> *** gcc/tree-ssa-sccvn.c (revision 231221)
> >> --- gcc/tree-ssa-sccvn.c (working copy)
> >> *************** vn_get_stmt_kind (gimple *stmt)
> >> *** 460,465 ****
> >> --- 460,467 ----
> >> ? VN_CONSTANT : VN_REFERENCE);
> >> else if (code == CONSTRUCTOR)
> >> return VN_NARY;
> >> + else if (code == OBJ_TYPE_REF)
> >> + return VN_NARY;
> >> return VN_NONE;
> >> }
> >> default:
> >> *************** vn_nary_length_from_stmt (gimple *stmt)
> >> *** 2479,2484 ****
> >> --- 2481,2487 ----
> >> return 1;
> >>
> >> case BIT_FIELD_REF:
> >> + case OBJ_TYPE_REF:
> >> return 3;
> >>
> >> case CONSTRUCTOR:
> >> *************** init_vn_nary_op_from_stmt (vn_nary_op_t
> >> *** 2508,2513 ****
> >> --- 2511,2517 ----
> >> break;
> >>
> >> case BIT_FIELD_REF:
> >> + case OBJ_TYPE_REF:
> >> vno->length = 3;
> >> vno->op[0] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
> >> vno->op[1] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
> >> Index: gcc/gimple-match-head.c
> >> ===================================================================
> >> *** gcc/gimple-match-head.c (revision 231221)
> >> --- gcc/gimple-match-head.c (working copy)
> >> *************** maybe_build_generic_op (enum tree_code c
> >> *** 243,248 ****
> >> --- 243,249 ----
> >> *op0 = build1 (code, type, *op0);
> >> break;
> >> case BIT_FIELD_REF:
> >> + case OBJ_TYPE_REF:
> >> *op0 = build3 (code, type, *op0, op1, op2);
> >> break;
> >> default:;
> >> Index: gcc/gimple-pretty-print.c
> >> ===================================================================
> >> *** gcc/gimple-pretty-print.c (revision 231221)
> >> --- gcc/gimple-pretty-print.c (working copy)
> >> *************** dump_unary_rhs (pretty_printer *buffer,
> >> *** 302,308 ****
> >> || TREE_CODE_CLASS (rhs_code) == tcc_reference
> >> || rhs_code == SSA_NAME
> >> || rhs_code == ADDR_EXPR
> >> ! || rhs_code == CONSTRUCTOR)
> >> {
> >> dump_generic_node (buffer, rhs, spc, flags, false);
> >> break;
> >> --- 302,309 ----
> >> || TREE_CODE_CLASS (rhs_code) == tcc_reference
> >> || rhs_code == SSA_NAME
> >> || rhs_code == ADDR_EXPR
> >> ! || rhs_code == CONSTRUCTOR
> >> ! || rhs_code == OBJ_TYPE_REF)
> >> {
> >> dump_generic_node (buffer, rhs, spc, flags, false);
> >> break;
> >> Index: gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C
> >> ===================================================================
> >> *** gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C (revision 0)
> >> --- gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C (working copy)
> >> ***************
> >> *** 0 ****
> >> --- 1,44 ----
> >> + /* { dg-do compile } */
> >> + /* { dg-options "-O2 -fdump-tree-fre2" } */
> >> +
> >> + template <class T> class A
> >> + {
> >> + T *p;
> >> +
> >> + public:
> >> + A (T *p1) : p (p1) { p->acquire (); }
> >> + };
> >> +
> >> + class B
> >> + {
> >> + public:
> >> + virtual void acquire ();
> >> + };
> >> + class D : public B
> >> + {
> >> + };
> >> + class F : B
> >> + {
> >> + int mrContext;
> >> + };
> >> + class WindowListenerMultiplexer : F, public D
> >> + {
> >> + void acquire () { acquire (); }
> >> + };
> >> + class C
> >> + {
> >> + void createPeer () throw ();
> >> + WindowListenerMultiplexer maWindowListeners;
> >> + };
> >> + class FmXGridPeer
> >> + {
> >> + public:
> >> + void addWindowListener (A<D>);
> >> + } a;
> >> + void
> >> + C::createPeer () throw ()
> >> + {
> >> + a.addWindowListener (&maWindowListeners);
> >> + }
> >> +
> >> + /* { dg-final { scan-tree-dump-times "= OBJ_TYPE_REF" 1 "fre2" } }
> >*/
>
