* [PATCH] Handle OBJ_TYPE_REF in FRE @ 2015-12-03 10:46 Richard Biener 2015-12-03 17:40 ` Jan Hubicka 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2015-12-03 10:46 UTC (permalink / raw) To: gcc-patches; +Cc: Jan Hubicka The following patch handles CSEing OBJ_TYPE_REF which was omitted because it is a GENERIC expression even on GIMPLE (for whatever reason...). Rather than changing this now the following patch simply treats it properly as such. 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?). Thanks, Richard. 2015-12-03 Richard Biener <rguenther@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" } } */ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Handle OBJ_TYPE_REF in FRE 2015-12-03 10:46 [PATCH] Handle OBJ_TYPE_REF in FRE Richard Biener @ 2015-12-03 17:40 ` Jan Hubicka 2015-12-03 20:46 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Jan Hubicka @ 2015-12-03 17:40 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, Jan Hubicka > > 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? 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. Or do you just substitute the operands of OBJ_TYPE_REF? > > 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. > > Thanks, > Richard. > > 2015-12-03 Richard Biener <rguenther@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" } } */ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Handle OBJ_TYPE_REF in FRE 2015-12-03 17:40 ` Jan Hubicka @ 2015-12-03 20:46 ` Richard Biener 2015-12-03 22:52 ` Jan Hubicka 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2015-12-03 20:46 UTC (permalink / raw) To: Jan Hubicka; +Cc: gcc-patches On December 3, 2015 6:40:07 PM GMT+01:00, Jan Hubicka <hubicka@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 <rguenther@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" } } >*/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Handle OBJ_TYPE_REF in FRE 2015-12-03 20:46 ` Richard Biener @ 2015-12-03 22:52 ` Jan Hubicka 2015-12-04 9:02 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Jan Hubicka @ 2015-12-03 22:52 UTC (permalink / raw) To: Richard Biener; +Cc: Jan Hubicka, gcc-patches > >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 <rguenther@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" } } > >*/ > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Handle OBJ_TYPE_REF in FRE 2015-12-03 22:52 ` Jan Hubicka @ 2015-12-04 9:02 ` Richard Biener 2015-12-04 9:31 ` Jan Hubicka 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2015-12-04 9:02 UTC (permalink / raw) To: Jan Hubicka; +Cc: gcc-patches On Thu, 3 Dec 2015, Jan Hubicka wrote: > > >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. Indeed we don't do code hoisting yet. Maybe one could trick PPRE into doing it. Note that for OBJ_TYPE_REFs in calls you probably should better use gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway (well, fntype will be the method-type, not pointer-to-method-type). Not sure if you need OBJ_TYPE_REFs type in non-call contexts? > > >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... See above. > > > > >> 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! For the above reasons I'm defering all this to stage1. Below is the patch that actually passed bootstrap & regtest on x86_64-unknown-linux-gnu, just in case you want to play with it. It doesn't do the propagation into calls yet though, the following does (untested) Index: gcc/tree-ssa-pre.c =================================================================== --- gcc/tree-ssa-pre.c (revision 231244) +++ gcc/tree-ssa-pre.c (working copy) @@ -4334,6 +4334,22 @@ eliminate_dom_walker::before_dom_childre maybe_remove_unused_call_args (cfun, call_stmt); gimple_set_modified (stmt, true); } + + else + { + /* Lookup the OBJ_TYPE_REF. */ + tree sprime + = vn_nary_op_lookup_pieces (3, OBJ_TYPE_REF, + TREE_TYPE (fn), + &TREE_OPERAND (fn, 0), NULL); + if (sprime) + sprime = eliminate_avail (sprime); + if (sprime) + { + gimple_call_set_fn (call_stmt, sprime); + gimple_set_modified (stmt, true); + } + } } } but it ICEs because we decided (tree-cfg.c, verify_gimple_call): if (fn && (!POINTER_TYPE_P (TREE_TYPE (fn)) || (TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != FUNCTION_TYPE && TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != METHOD_TYPE))) { error ("non-function in gimple call"); return true; } and in useless_type_conversion_p: /* Do not lose casts to function pointer types. */ if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE) && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE)) return false; probably from the times we didn't have gimple_call_fntype. So if I paper over the ICE (in the verifier) then the libreoffice testcase gets optimized to <bb 2>: _3 = this_2(D)->D.2399.D.2325._vptr.B; _4 = *_3; PROF_6 = OBJ_TYPE_REF(_4;(struct WindowListenerMultiplexer)this_2(D)->0); if (PROF_6 == acquire) goto <bb 3>; else goto <bb 4>; <bb 3>: PROF_6 (this_2(D)); goto <bb 5>; <bb 4>: PROF_6 (this_2(D)); by FRE2 and either VRP or DOM will propagate the equivalency to <bb 2>: _3 = this_2(D)->D.2399.D.2325._vptr.B; _4 = *_3; PROF_6 = OBJ_TYPE_REF(_4;(struct WindowListenerMultiplexer)this_2(D)->0); if (PROF_6 == acquire) goto <bb 3>; else goto <bb 4>; <bb 3>: WindowListenerMultiplexer::acquire (this_2(D)); goto <bb 5>; <bb 4>: PROF_6 (this_2(D)); Richard. 2015-12-03 Richard Biener <rguenther@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. * gimple-fold.c (gimple_build): Likewise. * gimple.h (gimple_expr_type): 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" } } */ Index: gcc/gimple-fold.c =================================================================== --- gcc/gimple-fold.c (revision 231221) +++ gcc/gimple-fold.c (working copy) @@ -6038,7 +6038,8 @@ gimple_build (gimple_seq *seq, location_ else res = create_tmp_reg (type); gimple *stmt; - if (code == BIT_FIELD_REF) + if (code == BIT_FIELD_REF + || code == OBJ_TYPE_REF) stmt = gimple_build_assign (res, code, build3 (code, type, op0, op1, op2)); else Index: gcc/gimple.h =================================================================== --- gcc/gimple.h (revision 231221) +++ gcc/gimple.h (working copy) @@ -6079,7 +6079,9 @@ gimple_expr_type (const gimple *stmt) } else if (code == GIMPLE_ASSIGN) { - if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR) + enum tree_code rcode = gimple_assign_rhs_code (stmt); + if (rcode == POINTER_PLUS_EXPR + || rcode == OBJ_TYPE_REF) return TREE_TYPE (gimple_assign_rhs1 (stmt)); else /* As fallback use the type of the LHS. */ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Handle OBJ_TYPE_REF in FRE 2015-12-04 9:02 ` Richard Biener @ 2015-12-04 9:31 ` Jan Hubicka 2015-12-04 9:40 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Jan Hubicka @ 2015-12-04 9:31 UTC (permalink / raw) To: Richard Biener; +Cc: Jan Hubicka, gcc-patches > Indeed we don't do code hoisting yet. Maybe one could trick PPRE > into doing it. > > Note that for OBJ_TYPE_REFs in calls you probably should better use > gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway > (well, fntype will be the method-type, not pointer-to-method-type). > > Not sure if you need OBJ_TYPE_REFs type in non-call contexts? Well, to optimize speculative call sequences if (funptr == thismethod) inlined this method body else funptr (); Here you want to devirtualize the conditional, not the call in order to get the inlined method unconditonally. In general I think OBJ_TYPE_REF is misplaced - it should be on vtable load instead of the call/conditional. It is a property of the vtable lookup. Then it would work for method pointers too. > > if (fn > && (!POINTER_TYPE_P (TREE_TYPE (fn)) > || (TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != FUNCTION_TYPE > && TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != METHOD_TYPE))) > { > error ("non-function in gimple call"); > return true; > } > > and in useless_type_conversion_p: > > /* Do not lose casts to function pointer types. */ > if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE > || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE) > && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE > || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE)) > return false; Yeah, this does not make much sense to me anymore. Something to track next stage1. > > probably from the times we didn't have gimple_call_fntype. So if I > paper over the ICE (in the verifier) then the libreoffice testcase > gets optimized to > > <bb 2>: > _3 = this_2(D)->D.2399.D.2325._vptr.B; > _4 = *_3; > PROF_6 = OBJ_TYPE_REF(_4;(struct > WindowListenerMultiplexer)this_2(D)->0); > if (PROF_6 == acquire) > goto <bb 3>; > else > goto <bb 4>; > > <bb 3>: > PROF_6 (this_2(D)); > goto <bb 5>; > > <bb 4>: > PROF_6 (this_2(D)); > > by FRE2 and either VRP or DOM will propagate the equivalency to > > <bb 2>: > _3 = this_2(D)->D.2399.D.2325._vptr.B; > _4 = *_3; > PROF_6 = OBJ_TYPE_REF(_4;(struct > WindowListenerMultiplexer)this_2(D)->0); > if (PROF_6 == acquire) > goto <bb 3>; > else > goto <bb 4>; > > <bb 3>: > WindowListenerMultiplexer::acquire (this_2(D)); > goto <bb 5>; > > <bb 4>: > PROF_6 (this_2(D)); > > Richard. LGTM. Honza > > 2015-12-03 Richard Biener <rguenther@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. > * gimple-fold.c (gimple_build): Likewise. > * gimple.h (gimple_expr_type): 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" } } */ > Index: gcc/gimple-fold.c > =================================================================== > --- gcc/gimple-fold.c (revision 231221) > +++ gcc/gimple-fold.c (working copy) > @@ -6038,7 +6038,8 @@ gimple_build (gimple_seq *seq, location_ > else > res = create_tmp_reg (type); > gimple *stmt; > - if (code == BIT_FIELD_REF) > + if (code == BIT_FIELD_REF > + || code == OBJ_TYPE_REF) > stmt = gimple_build_assign (res, code, > build3 (code, type, op0, op1, op2)); > else > Index: gcc/gimple.h > =================================================================== > --- gcc/gimple.h (revision 231221) > +++ gcc/gimple.h (working copy) > @@ -6079,7 +6079,9 @@ gimple_expr_type (const gimple *stmt) > } > else if (code == GIMPLE_ASSIGN) > { > - if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR) > + enum tree_code rcode = gimple_assign_rhs_code (stmt); > + if (rcode == POINTER_PLUS_EXPR > + || rcode == OBJ_TYPE_REF) > return TREE_TYPE (gimple_assign_rhs1 (stmt)); > else > /* As fallback use the type of the LHS. */ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Handle OBJ_TYPE_REF in FRE 2015-12-04 9:31 ` Jan Hubicka @ 2015-12-04 9:40 ` Richard Biener 2015-12-04 16:59 ` Jan Hubicka 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2015-12-04 9:40 UTC (permalink / raw) To: Jan Hubicka; +Cc: gcc-patches On Fri, 4 Dec 2015, Jan Hubicka wrote: > > Indeed we don't do code hoisting yet. Maybe one could trick PPRE > > into doing it. > > > > Note that for OBJ_TYPE_REFs in calls you probably should better use > > gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway > > (well, fntype will be the method-type, not pointer-to-method-type). > > > > Not sure if you need OBJ_TYPE_REFs type in non-call contexts? > > Well, to optimize speculative call sequences > > if (funptr == thismethod) > inlined this method body > else > funptr (); > > Here you want to devirtualize the conditional, not the call in order > to get the inlined method unconditonally. > > In general I think OBJ_TYPE_REF is misplaced - it should be on vtable load > instead of the call/conditional. It is a property of the vtable lookup. > Then it would work for method pointers too. Even better. Make it a tcc_reference tree then. > > > > if (fn > > && (!POINTER_TYPE_P (TREE_TYPE (fn)) > > || (TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != FUNCTION_TYPE > > && TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != METHOD_TYPE))) > > { > > error ("non-function in gimple call"); > > return true; > > } > > > > and in useless_type_conversion_p: > > > > /* Do not lose casts to function pointer types. */ > > if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE > > || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE) > > && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE > > || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE)) > > return false; > > Yeah, this does not make much sense to me anymore. Something to track next > stage1. Btw, might be necessary for targets with function descriptors - not sure though. Richard. > > > > probably from the times we didn't have gimple_call_fntype. So if I > > paper over the ICE (in the verifier) then the libreoffice testcase > > gets optimized to > > > > <bb 2>: > > _3 = this_2(D)->D.2399.D.2325._vptr.B; > > _4 = *_3; > > PROF_6 = OBJ_TYPE_REF(_4;(struct > > WindowListenerMultiplexer)this_2(D)->0); > > if (PROF_6 == acquire) > > goto <bb 3>; > > else > > goto <bb 4>; > > > > <bb 3>: > > PROF_6 (this_2(D)); > > goto <bb 5>; > > > > <bb 4>: > > PROF_6 (this_2(D)); > > > > by FRE2 and either VRP or DOM will propagate the equivalency to > > > > <bb 2>: > > _3 = this_2(D)->D.2399.D.2325._vptr.B; > > _4 = *_3; > > PROF_6 = OBJ_TYPE_REF(_4;(struct > > WindowListenerMultiplexer)this_2(D)->0); > > if (PROF_6 == acquire) > > goto <bb 3>; > > else > > goto <bb 4>; > > > > <bb 3>: > > WindowListenerMultiplexer::acquire (this_2(D)); > > goto <bb 5>; > > > > <bb 4>: > > PROF_6 (this_2(D)); > > > > Richard. > > LGTM. > Honza > > > > 2015-12-03 Richard Biener <rguenther@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. > > * gimple-fold.c (gimple_build): Likewise. > > * gimple.h (gimple_expr_type): 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" } } */ > > Index: gcc/gimple-fold.c > > =================================================================== > > --- gcc/gimple-fold.c (revision 231221) > > +++ gcc/gimple-fold.c (working copy) > > @@ -6038,7 +6038,8 @@ gimple_build (gimple_seq *seq, location_ > > else > > res = create_tmp_reg (type); > > gimple *stmt; > > - if (code == BIT_FIELD_REF) > > + if (code == BIT_FIELD_REF > > + || code == OBJ_TYPE_REF) > > stmt = gimple_build_assign (res, code, > > build3 (code, type, op0, op1, op2)); > > else > > Index: gcc/gimple.h > > =================================================================== > > --- gcc/gimple.h (revision 231221) > > +++ gcc/gimple.h (working copy) > > @@ -6079,7 +6079,9 @@ gimple_expr_type (const gimple *stmt) > > } > > else if (code == GIMPLE_ASSIGN) > > { > > - if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR) > > + enum tree_code rcode = gimple_assign_rhs_code (stmt); > > + if (rcode == POINTER_PLUS_EXPR > > + || rcode == OBJ_TYPE_REF) > > return TREE_TYPE (gimple_assign_rhs1 (stmt)); > > else > > /* As fallback use the type of the LHS. */ > > -- Richard Biener <rguenther@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Handle OBJ_TYPE_REF in FRE 2015-12-04 9:40 ` Richard Biener @ 2015-12-04 16:59 ` Jan Hubicka 0 siblings, 0 replies; 8+ messages in thread From: Jan Hubicka @ 2015-12-04 16:59 UTC (permalink / raw) To: Richard Biener; +Cc: Jan Hubicka, gcc-patches > On Fri, 4 Dec 2015, Jan Hubicka wrote: > > > > Indeed we don't do code hoisting yet. Maybe one could trick PPRE > > > into doing it. > > > > > > Note that for OBJ_TYPE_REFs in calls you probably should better use > > > gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway > > > (well, fntype will be the method-type, not pointer-to-method-type). > > > > > > Not sure if you need OBJ_TYPE_REFs type in non-call contexts? > > > > Well, to optimize speculative call sequences > > > > if (funptr == thismethod) > > inlined this method body > > else > > funptr (); > > > > Here you want to devirtualize the conditional, not the call in order > > to get the inlined method unconditonally. > > > > In general I think OBJ_TYPE_REF is misplaced - it should be on vtable load > > instead of the call/conditional. It is a property of the vtable lookup. > > Then it would work for method pointers too. > > Even better. Make it a tcc_reference tree then. makes sense to me - it is indeed more similar to MEM_REF than to an expression. I will look into that next stage1. Honza ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-12-04 16:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-03 10:46 [PATCH] Handle OBJ_TYPE_REF in FRE Richard Biener 2015-12-03 17:40 ` Jan Hubicka 2015-12-03 20:46 ` Richard Biener 2015-12-03 22:52 ` Jan Hubicka 2015-12-04 9:02 ` Richard Biener 2015-12-04 9:31 ` Jan Hubicka 2015-12-04 9:40 ` Richard Biener 2015-12-04 16:59 ` Jan Hubicka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).