On December 3, 2015 6:40:07 PM GMT+01:00, Jan Hubicka <hubi...@ucw.cz> wrote: >> >> The following patch handles CSEing OBJ_TYPE_REF which was omitted >> because it is a GENERIC expression even on GIMPLE (for whatever > >Why it is generic? It is part of gimple grammar :) > >> reason...). Rather than changing this now the following patch >> simply treats it properly as such. > >Thanks for working on this! Will this do code motion, too?
It will do PRE, so "yes". >I think you may want to compare the ODR type of obj_type_ref_class >otherwise two otherwise equivalent OBJ_TYPE_REFs may lead to different >optimizations later. I suppose we can have code of form > >if (test) > OBJ_TYPE_REF1 > ... >else > OBJ_TYPE_REF2 > .. >where each invoke method of different class type but would otherwise >match as equivalent for tree-ssa-sccvn becuase we ignore pointed-to >types. >so doing > >OBJ_TYPE_REF1 >if (test) > ... >else > ... > >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. >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. >> 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. Richard. >> >> Thanks, >> Richard. >> >> 2015-12-03 Richard Biener <rguent...@suse.de> >> >> 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" } } >*/